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

Question

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.

Answer 1

For newbies like myself researching this in the future:

The important part of the error is actually here:

return result | ------ returning this value requires that *self is borrowed for '1

It's not that you're calling find within the loop, but that you're returning a reference that was assigned within the loop. This also applies when trying to return a mutable reference from a vector. Seems to just be a limitation of the current borrow checker.

The simplest solution is to instead find the key inside the loop, then index and return outside the loop.

Two caveats:

• You will need to handle the possibility that your loop returned an invalid key (or unwrap).
• There is one extra index into the collection - might impact performance if calling many times - I just consider this a trade-off for memory safety

In this specific example find doesn't actually need to return anything - it is just ensuring the value is cached. Not sure why find would ever return None here but I'll assume there is some case. After the value is cached and you get a Some back, you can just index into the cache inside of the it fn.

Rust Playground

use std::collections::HashMap;

#[derive(Debug)]
struct Calculation {
    _value: Option<i32>,
}

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

impl Struct {
    fn cache(&mut self, key: &String) -> Option<()> {
        if *key == String::from("1") {
            return None;
        }
        match self.items.get(key) {
            None => {
                let calculation = Calculation { _value: None };
                self.items.insert(key.clone(), Box::new(calculation));
                // Some heavy calculation here
                Some(())
            }
            Some(_) => Some(()), // Some heavy calculation before returning
        }
    }

    fn it(&mut self) -> Option<&Box<Calculation>> {
        let mut result: Option<String> = None;
        for key in vec!["1", "2", "3"] {
            if let Some(_) = self.cache(&String::from(key)) {
                result = Some(String::from(key));
                break;
            }
        }
        if let Some(key) = result {
            return Some(self.items.get(&key).unwrap());
        }
        None
    }
}

fn main() {
    let mut s = Struct {
        items: HashMap::new(),
    };
    println!("{:?}", s.it()); // Some(Calculation { value: None })
    println!("{:?}", s.items); // {"2": Calculation { value: None }}
}

Answer 2

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

Answer 3

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.

Answer 4

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