rustownership

How to fix ".. was mutably borrowed here in the previous iteration of the loop" in Rust?


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
    }
}

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.


Solution

  • 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.