"Cannot borrow as mutable because already borrowed by immutable", even though immutable borrow has gone out of scope?

Hi all,
Recently I ran into a strange compiler warning that says that I'm not allowed to borrow a variable as mutable because it was borrowed immutable earlier... however, that earlier borrow was done in an inner block, and therefore should've gone out of scope. Any ideas? My code is here - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8f21704f592957887a436452d742a494.

Also, I'm aware that using a reference of a 3-tuple of references is a really awkward way to work with a HashMap. However, I couldn't find any better way to do it - I can't do a reference to a tuple of values, because the function activate doesn't own those values... If anyone has any ideas, it would be appreciated.

Couldn't you turn the tuple into a struct?

1 Like

I think your issue is that HashMap::get yields an Option<&V> whose lifetime is tied to that of the key and the hashmap. So you have a value which can’t outlive the key as far as I understand, but the key’s lifetime is tied to that of the references obtained from the references to self.

I wonder if you could use keys that have Rc pointers instead of bare references? Also, a reference to a thing with a reference inside feels fraught. Maybe consider using usizes to point at locations in the stack. Alternatively if you can make your keys Copy then this is easier.

Finally, hashmap.get(...).unwrap() is probably better written hashmap[...], and you can use stack.last() to get an option with a reference to its last element if any (not that that really helps here).

1 Like

Borrows do not "go out of scope"; they persist until they are no longer used.

I'm not sure why exactly your code won't compile but I noticed some weird things about it that might mean it's not doing what you expect. Here's a lightly edited version of activate that has the same problem:

pub fn activate(&mut self, input: &ContextType) {
    // I'm just going to put this in a variable so I can more easily talk about the following line
    let key = (
        &self.dat,
        match self.stack.len() {
            0 => None,
            _ => Some(&self.stack[self.stack.len() - 1]),
        },
        input,
    );
    let (stack_action, val_action) = (&self.table.get(&key)).unwrap();
    match stack_action {
        StackAction::Pop => {
            self.stack.pop();
        }
        StackAction::Push(x) => self.stack.push(x.clone()),
        _ => {}
    };
    self.dat = val_action.clone();
}

The weirdness is concentrated in this line:

let (stack_action, val_action) = (&self.table.get(&key)).unwrap();

First, HashMap::get returns an Option<&V>. Then you take a & reference to it, so you have &Option<&V> -- this in itself is weird because there's not really a good reason for it. The next thing you do is call .unwrap(), which dereferences the & to make a copy of the Option which it then unwraps to &V. This is a little more weird because you generally can't call .unwrap() on a &Option<_>. The only reason it works is because Option<&V> is Copy, but that doesn't seem relevant to what you're doing -- you could have just written self.table.get(&key).unwrap(). Maybe you were trying to express something that got a little lost in translation.

Then, &V is actually &(StackAction<T>, Option<T>). You assign this to (stack_action, val_action). This compiles because of "pattern ergonomics" which is non-obvious but what it does is make stack_action a &StackAction<T> and val_action an &Option<T>, i.e. they reference the values that are still owned by self.table, instead of being moved out. Is this what you meant to do or did you expect stack_action and val_action to be owned values?

2 Likes

Here’s a lightly edited version which builds:

This clones the value from the stack rather than referencing it, so that the stack can be mutated later without risk of invalidating the reference.

I think it's worth mentioning, that what they did is appropriate, if they did that on purpose to refactor later for proper error handling where you can just scan your source code for .unwrap(. :slightly_smiling_face:

Yeah, I suppose so - that would certainly be a lot cleaner than a tuple. However, now it's still a struct of references, which doesn't get around the lifetime issue that another poster mentioned.

Yes, I did want stack_action and val_action to be owned values. I just made a let (stack_action, val-action) = {let (a, b) = my old self.table.get code; ((*a).clone(), (*b).clone())}, and it worked perfectly. That was exactly the problem, thanks so much!

1 Like

I see, makes sense. I chose to go with cloning val_action and stack_action instead, though, because it requires less total clones. Thanks for the help though!