19

I have to iterate on keys, find the value in HashMap by key, possibly do some heavy computation in the found struct as a value (lazy => mutate the struct) and cached return it in Rust.

I'm getting the following error message:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/main.rs:25:26
   |
23 |     fn it(&mut self) -> Option<&Box<Calculation>> {
   |           - let's call the lifetime of this reference `'1`
24 |         for key in vec!["1","2","3"] {
25 |             let result = self.find(&key.to_owned());
   |                          ^^^^ `*self` was mutably borrowed here in the previous iteration of the loop
...
28 |                 return result
   |                        ------ returning this value requires that `*self` is borrowed for `'1`

Here is the code in playground.

use std::collections::HashMap;

struct Calculation {
    value: Option<i32>
}

struct Struct {
    items: HashMap<String, Box<Calculation>> // cache
}

impl Struct {
    fn find(&mut self, key: &String) -> Option<&Box<Calculation>> {
        None // find, create, and/or calculate items
    }

    fn it(&mut self) -> Option<&Box<Calculation>> {
        for key in vec!["1","2","3"] {
            let result = self.find(&key.to_owned());
            if result.is_some() {
                return result
            }
        }
        None
    }
}
  • I can't avoid the loop as I have to check multiple keys
  • I have to make it mutable (self and the structure) as the possible calculation changes it

Any suggestion on how to change the design (as Rust forces to think in a bit different way that makes sense) or work around it?

PS. There are some other issues with the code, but let's split the problems and solve this one first.

4ntoine
  • 19,816
  • 21
  • 96
  • 220
  • Also relevant: [Why is it discouraged to accept a reference to a String (&String), Vec (&Vec), or Box (&Box) as a function argument?](https://stackoverflow.com/q/40006219/3650362) – trent Feb 12 '21 at 15:26
  • 2
    Help but still can't understand why it should not compile. Ok self is being borrowed till the function call self.find(...) ends, and even the lifetime of `result` ends before the next iteration. So "why" and how is "was mutably borrowed here in the **previous iteration** of the loop" is an error and what if it wasn't error, how can we wrongly use it. Even a brief explanation will help, this error confuses me :' ) – AdityaG15 Jan 27 '22 at 18:47
  • This is a limitation of the current borrow checker. Polonius accepts it. – Chayim Friedman Mar 06 '23 at 09:24
  • Polonius is the next version of the borrow checker, https://github.com/rust-lang/polonius – Benjamin Smus Jun 26 '23 at 17:30

3 Answers3

6

You can't do caching with exclusive access. You can't treat Rust references like general-purpose pointers (BTW: &String and &Box<T> are double indirection, and very unidiomatic in Rust. Use &str or &T for temporary borrows).

&mut self means not just mutable, but exclusive and mutable, so your cache supports returning only one item, because the reference it returns has to keep self "locked" for as long as it exists.

You need to convince the borrow checker that the thing that find returns won't suddenly disappear next time you call it. Currently there's no such guarantee, because the interface doesn't stop you from calling e.g. items.clear() (borrow checker checks what function's interface allows, not what function actually does).

You can do that either by using Rc, or using a crate that implements a memory pool/arena.

struct Struct {
   items: HashMap<String, Rc<Calculation>>,
}

fn find(&mut self, key: &str) -> Rc<Calculation> 

This way if you clone the Rc, it will live for as long as it needs, independently of the cache.

You can also make it nicer with interior mutability.

struct Struct {
   items: RefCell<HashMap<…
}

This will allow your memoizing find method to use a shared borrow instead of an exclusive one:

fn find(&self, key: &str) -> …

which is much easier to work with for the callers of the method.

Kornel
  • 97,764
  • 37
  • 219
  • 309
  • In order to modify Calculation (`Rc::get_mut()`) Rc strong_count have to be = `0`, but it can be used somewhere. Basically this locks mutation until it's read or hold somewhere. So just using of `RC` does not help if i understand correctly (instead of compilation issue we have the same runtime issue) – 4ntoine Feb 15 '21 at 14:37
  • If you need shared mutability (that causes cached entries to be mutated too), then use `Rc>`. – Kornel Feb 18 '21 at 10:41
5

Probably not the cleanest way to do that, but it compiles. The idea is not to store the value found in a temporary result, to avoid aliasing: if you store the result, self is kept borrowed.

impl Struct {

    fn find(&mut self, key: &String) -> Option<&Box<Calculation>> {
        None
    }

    fn it(&mut self) -> Option<&Box<Calculation>> {
        for key in vec!["1","2","3"] {
            if self.find(&key.to_owned()).is_some() {
                return self.find(&key.to_owned());
            }
        }
        None
    }
}
Bromind
  • 1,065
  • 8
  • 23
  • that seems to be not good for performance: `find` is called twice. There should be a "proper way" – 4ntoine Feb 12 '21 at 10:21
  • 2
    I agree with you, it could be better. On the other hand, for your specific case, `find` is not that expensive, since it is only a lookup in a hashtable, and you know that the value exists. If I understand correctly your intentions, the heavy calculation is done once anyway, so maybe this double call is quite small compared to that. I'll try to think of a better way to do it. – Bromind Feb 12 '21 at 10:38
0

I had similar issues, and I found a workaround by turning the for loop into a fold, which convinced the compiler that self was not mutably borrowed twice.

It works without using interior mutability or duplicated function call; the only downside is that if the result was found early, it will not short-circuit but continue iterating until the end.

Before:

for key in vec!["1","2","3"] {
    let result = self.find(&key.to_owned());
      if result.is_some() {
          return result
      }
}

After:

vec!["1", "2,", "3"]
    .iter()
    .fold(None, |result, key| match result {
        Some(result) => Some(result),
        None => self.find(&key.to_string())
    })

Working playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=92bc73e4bac556ce163e0790c7d3f154

Wong Jia Hau
  • 2,639
  • 2
  • 18
  • 30