Avoid calling both HashMap::get() and HashMap::get_mut() when borrowing 'self'

Question

I'm learning rust and I find myself writing lots and lots of code that disappoints the borrow checker. I've recently ran into this quirk that I can't figure out a way around.

To set the scene: I would like to (1) fetch a struct from a HashMap, (2) compute some metadata about the fetched struct, then (3) modify the fetched struct. Simple! See a simplified example below:

struct Entity {
    pub metadata: i64
}
struct Engine {
    pub entities: HashMap<i32, Box<Entity>>
}
impl Engine {
    fn immutable_calculate_metadata(&self, new_metadata: i64) -> i64 {
        // Compute a bunch of stuff that requires reading "self"
        42 + new_metadata
    }
    pub fn add_entity(&mut self, key: i32, metadata: i64) {
        self.entities.insert(key, Box::new(Entity{metadata}));
        let neighbor_key = &(key - 1);
        if let Some(neighbor) = self.entities.get_mut(neighbor_key) {
            let metadata = self.immutable_calculate_metadata(metadata);
            // I *want* to write this line, but it breaks:
            // neighbor.metadata = metadata;
            // So I have to re-fetch the same key.
            self.entities.get_mut(neighbor_key).unwrap().metadata = metadata;
        }
    }
}

(rust playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cf53e4af2d41add3765578e619c75e5e )

I don't know how I can remove the redundant fetch in the self.entities.get_mut(neighbor_key).unwrap().metadata = metadata; line. Any help here would be greatly appreciated. Ideally someone who knows what they're doing (read: you) could enumerate the options I have here to avoid this.

Of course, if I try to do this intuitively (see the commented out line // neighbor.metadata = metadata;) I get the following error:

error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
   |
18 |         if let Some(neighbor) = self.entities.get_mut(neighbor_key) {
   |                                 ----------------------------------- mutable borrow occurs here
19 |             let metadata = self.immutable_calculate_metadata(metadata);
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here
20 |             // I *want* to write this line, but it breaks:
21 |             neighbor.metadata = metadata;
   |             ---------------------------- mutable borrow later used here

I don't exactly understand why self.entities.get_mut(neighbor_key) is considered a "mutable borrow of *self". It seems to me we're borrowing an Entity or a HashMap but that's not the focus of this question.

Answer 1

I don't exactly understand why self.entities.get_mut(neighbor_key) is considered a "mutable borrow of *self".

Since entities is a part of self, and any HashMap entry is a part of the hash map, mutating even a single entry in entities counts as mutating self.

Due to how Rust's borrow checker works (and like the error message you received suggests), you won't be allowed to borrow self as immutable while also borrowing it as mutable. The reason for this constraint is exactly due to the fact that mutating a single entry means mutation to the whole self, and therefore violating the immutability contract with immutable_calculate_metadata.

... enumerate the options I have here to avoid this.

RECOMMENDED (probably what you want):

1. Consider converting immutable_calculate_metadata into an associated function (remove &self from signature). Does it actually need &self, or perhaps it would be better to only give it (by value) the specific parameters it needs from the Engine struct? More context about your actual use case would be helpful here.

fn immutable_calculate_metadata(new_metadata: i64) -> i64 {
    // Doesn't require reading "self" anymore
    42 + new_metadata
}
pub fn add_entity(&mut self, key: i32, metadata: i64) {
    self.entities.insert(key, Box::new(Entity { metadata }));
    let neighbor_key = &(key - 1);
    if let Some(neighbor) = self.entities.get_mut(neighbor_key) {
        let metadata = Engine::immutable_calculate_metadata(metadata);
        // No breakage
        neighbor.metadata = metadata;
    }
}

LESS RECOMMENDED (it would solve your problem, but probably not in the way you intended)

2. Leave almost everything as it is, with the small difference of calculating the metadata before the if let scope. Of course this solution would have performance implications in the case of large and heavy calculations, but in some cases it might actually be more performant then re-fetching the key twice.

fn immutable_calculate_metadata(&self, new_metadata: i64) -> i64 {
    // Compute a bunch of stuff that requires reading "self"
    42 + new_metadata
}
pub fn add_entity(&mut self, key: i32, metadata: i64) {
    self.entities.insert(key, Box::new(Entity { metadata }));
    let neighbor_key = &(key - 1);
    let metadata = self.immutable_calculate_metadata(metadata);
    if let Some(neighbor) = self.entities.get_mut(neighbor_key) {
        // No breakage
        neighbor.metadata = metadata;
    }
}

NOT RECOMMENDED (this one is NOT a good solution, as it just blindly satisfies the borrow checker, disregarding the implications. I'm mentioning it only for the sake of completeness)

3. Clone the entire entities HashMap. The borrow checker will be satisfied, but this will most likely heavily affect performance.

fn immutable_calculate_metadata(&self, new_metadata: i64) -> i64 {
    // Compute a bunch of stuff that requires reading "self"
    42 + new_metadata
}
pub fn add_entity(&mut self, key: i32, metadata: i64) {
    self.entities.insert(key, Box::new(Entity { metadata }));
    let neighbor_key = &(key - 1);
    let mut entities = self.entities.to_owned();
    if let Some(neighbor) = entities.get_mut(neighbor_key) {
        let metadata = self.immutable_calculate_metadata(metadata);
        // No breakage
        neighbor.metadata = metadata;
    }
    self.entities = entities;
}