Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fight-compiler - alternative approaches? #342

Open
nneonneo opened this issue Dec 21, 2022 · 1 comment
Open

fight-compiler - alternative approaches? #342

nneonneo opened this issue Dec 21, 2022 · 1 comment

Comments

@nneonneo
Copy link

The single "fight-compiler" exercise borrows the list for iteration, then attempts to borrow the structure mutably in each iteration, which naturally fails.

One of the reasons why this fails is that do_something receives a reference to the entire structure, meaning that it could (in principle) mutate the list while the iteration is ongoing in run. Obviously, Rust forbids this.

However, the proposed solution (https://github.com/sunface/rust-by-practice/blob/master/solutions/fight-compiler/borrowing.md) involves refactoring the iteration to a regular indexed loop. This doesn't actually solve the underlying (potential) safety hazard: do_something could still mutate the list, which could cause the caller to panic unexpectedly if e.g. the list was truncated by some other implementation of do_something. It can be argued that the API is broken here, because it does not appropriately restrict implementers from doing unexpected things.

There are a few ways this can be solved (IMHO) more cleanly, although they involve changing the API of do_something:

  • Don't give do_something access to &mut self; instead, give it a &mut i32 reference so it is only capable of mutating a and not list. So, pub fn do_something(a: &mut i32, n: i32) { *a = n; }
  • Convert do_something to a closure. Closures get special treatment because the compiler can directly see what they borrow. So, for example, self.list.iter().for_each(|i| { self.a = *i; }); works. In fact, we can even do something like self.list.iter().for_each(|i| { self.a = self.list[(7 - *i) as usize]; });, borrowing list immutably and a mutably, which (AFAIK) you simply cannot do with a separate function.
@u9g
Copy link

u9g commented Aug 10, 2023

Couldn't you also just clone the array before iterating it? I feel like the rules are a little too loose. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants