2

I have a struct with two methods. The first method returns an iterator over some field. The second method calls the first one and subsequently tries to modify that field (Playground):

struct Container {
    vec: Vec<f64>,
}

impl Container {
    fn large_positions<'a>(&'a self) -> impl Iterator<Item = usize> + 'a {
        self.vec
            .iter()
            .enumerate()
            .filter_map(|(i, &f)| if f > 3.14 { Some(i) } else { None })
    }

    fn negate_first_large_entry(&mut self) {
        if let Some(i) = self.large_positions().next() {
            self.vec[i] *= -1.0;
        }
    }
}

The borrow checker complains:

error[E0502]: cannot borrow `self.vec` as mutable because it is also borrowed as immutable
  --> src/lib.rs:15:13
   |
14 |         if let Some(i) = self.large_positions().next() {
   |                          ----------------------
   |                          |
   |                          immutable borrow occurs here
   |                          a temporary with access to the immutable borrow is created here ...
15 |             self.vec[i] *= -1.0;
   |             ^^^^^^^^ mutable borrow occurs here
16 |         }
17 |     }
   |     - ... and the immutable borrow might be used here, when that temporary is dropped and runs the destructor for type `impl Iterator`

If I manually inline large_positions, the code compiles so the destructor thing is not a real issue.

Why does the borrow checker know that there is no conflict in the inlined version? And how can I explain this to the borrow checker in the non-inlined version?

Update: The following slightly different version of the second function exhibits the same behavior: (Playground)

fn negate_first_large_entry_below_100(&mut self) {
    for i in self.large_positions() {
        if self.vec[i] < 100.0 {
            self.vec[i] *= -1.0;
            return;
        }
    }
}

Here the return statement causes the borrow checker to understand that the borrow of self.vec can be released as soon as the if branch is entered. But again this only works if large_positions is inlined.

Ideally I would like to modify the signature of large_positions in such a way that the borrow checker is still able to perform this clever "early releasing".

wonce
  • 1,893
  • 12
  • 18
  • It looks like your question might be answered by the answers of [Why does refactoring by extracting a method trigger a borrow checker error?](https://stackoverflow.com/q/57017747/155423). If not, please **[edit]** your question to explain the differences. Otherwise, we can mark this question as already answered. – Shepmaster Dec 08 '21 at 19:24
  • Specifically, what if `self.vec[i] *= -1.0` caused the vector to reallocate, invalidating any references? Then using the iterator would cause memory unsafety. The compiler is preventing you from introducing vulnerabilities. – Shepmaster Dec 08 '21 at 19:26
  • @Shepmaster Isn't that question centered about borrowing two distinct fields of `self`? Here there is only one field. – wonce Dec 08 '21 at 19:26
  • 1
    As a practical solution, you can do `let v = self.large_positions().next();` to show that the iterator is no longer needed after advancing it once. – Shepmaster Dec 08 '21 at 19:29
  • @Shepmaster Thanks. In the original code this "practical solution" would be less elegant than here, so I'm still interested in understanding the underlying issue. – wonce Dec 08 '21 at 19:43

2 Answers2

2

You can extrat the iterator into a variable that you can later drop in order to get a &mut afterwards:

    fn negate_first_large_entry(&mut self) {
        let mut iter = self.large_positions();
        if let Some(i) = iter.next() {
            drop(iter);
            self.vec[i] *= -1.0;
        }
    }

Playground

For the second method you just need to use a while loop instead of a for loop to perform the same trick:

    fn negate_first_large_entry_below_100(&mut self) {
        let mut iter = self.large_positions();
        while let Some(i) = iter.next() {
            if self.vec[i] < 100.0 {
                drop(iter);
                self.vec[i] *= -1.0;
                return;
            }  
        }
    }

Playground

Netwave
  • 40,134
  • 6
  • 50
  • 93
  • Very interesting. I've never seen `drop` used to help the borrow-checker. Do you think this could also work in `negate_first_large_entry_below_100` (see question update)? – wonce Dec 09 '21 at 14:09
  • @wonce of course it would. Check update :) – Netwave Dec 09 '21 at 14:33
0

(I believe to have now gained a superficial understanding of the underlying problem, so I will formulate this as an answer. Comments are welcome.)

As the error message hints and the answer by Netwave demonstrates, the issue is related to dropping iter. Upon dropping iter, the borrow of self.vec is freed. If this happens before self.vec is modified, then the code borrow-checks successfully.

In the non-inlined version, the drop of iter happens when iter goes out of scope. The compiler is not able to move this forward, because it is not aware whether iter has any user-defined Drop implementation.

In contrast, in the manually inlined version of the code, the compile is able to perform this drop immediately after the last time iter is used. I presume this optimization is possible because the compiler knows here that no custom code is run upon dropping iter (not sure how I could verify this).

A proper solution to the problem would thus be to extend the return type of large_positions, indicating that the returned value may be dropped before it leaves scope. If I understand this proposal correctly, this is currently impossible because such a "TrivialDrop" trait does not exist in Rust.

wonce
  • 1,893
  • 12
  • 18