Drop a immutable borrow to make a mutable borrow

Question

I am still learning Rust and when trying to implement Dikjstra as part of a training project, I encountered this peculiar catch. First I define a HashMap:

let mut dist: HashMap<Node, usize> = HashMap::new();

And later:

let state = State { node: next_node.clone(), cost: cost + 1 };
let current_dist = dist.get(&state.node);
if (current_dist == None) || (state.cost < *current_dist.unwrap()) {
    dist.insert(state.node.clone(), state.cost);
    heap.push(state);
}

Which yields a compile error because dist.get triggers a immutable borrow which stays in scope until after the if ... {...} statement, and in particular when I dist.insert, asking for a mutable borrow.

I think I miss a pattern or a keyword allowing me this type of process. For now I tried a drop at the beginning of the if scope, and other current_dist evaluation such as

let current_dist;
{
    current_dist = dist.get(&state.node);
}

or

let current_dist = {|| dist.get(&state.node)}();

but the end of scope of the immutable borrow still happen after the if statement.

`(current_dist == None) | (state.cost < *current_dist.unwrap())` I can't imagine that you want a bitwise OR operation there. — Shepmaster, Feb 06 '17 at 19:31
Please review how to create a [MCVE]. You haven't defined what `Node`, `State` or `heap` are. — Shepmaster, Feb 06 '17 at 19:38
I corrected the `||`. The definition of `State` seemed implied by the creation of `state`, and `Node` and `heap` didn't seem relevant (`Node` is a set of coordinates, `heap` is a heap). — Thrastylon, Feb 07 '17 at 01:56
*and `Node` and `heap` didn't seem relevant* — then **remove them from your question**. Again, I strongly encourage you to review what we mean by a [MCVE]; emphasis on the **Minimal**. The reason for doing this is twofold: 1. by reducing the problem, you are more likely to solve it yourself, 2. a reduced problem is easier for *other* people to quickly understand. These people include answerers *and* future question askers who find your question. — Shepmaster, Feb 07 '17 at 02:54

Shepmaster · Accepted Answer · 2019-11-20T14:05:51.677

After non-lexical lifetimes

Since non-lexical lifetimes are now enabled, the original code compiles. That being said, you should still use the entry API for efficiency, otherwise you have to hash the key multiple times:

use std::collections::hash_map::Entry;
use std::collections::HashMap;

fn main() {
    let mut dist: HashMap<u8, u8> = HashMap::new();

    let cost = 21;

    match dist.entry(42) {
        Entry::Vacant(entry) => {
            entry.insert(42);
        }
        Entry::Occupied(mut entry) => {
            if *entry.get() < cost {
                entry.insert(42);
            }
        }
    }
}

Before non-lexical lifetimes

because dist.get triggers a mutable borrow

No, it's just an immutable borrow:

pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
where
    K: Borrow<Q>,
    Q: Hash + Eq,

I tried a drop

Explicit drops do not affect lifetimes.

let current_dist;
{
    current_dist = dist.get(&state.node);
}

Here you aren't fooling anyone. If the compiler was confused by this, it wouldn't be very good. This still has a borrow to the HashMap, there's just some extra blocks scattered about.

let current_dist = {|| dist.get(&state.node)}();

Same here. Returning the reference from a closure is still returning a reference. You really cannot easily trick the compiler into thinking that your reference to the HashMap doesn't exist.

You need to use a block to constrain how long the borrow exists. the simplest transformation is something akin to:

use std::collections::HashMap;

fn main() {
    let mut dist: HashMap<u8, u8> = HashMap::new();

    let do_it = {
        let current_dist = dist.get(&42);
        current_dist == None || true
    };

    if do_it {
        dist.insert(42, 42);
    }
}

This isn't the prettiest, but some combinators can clean it up:

use std::collections::HashMap;

fn main() {
    let mut dist: HashMap<u8, u8> = HashMap::new();

    let cost = 21;

    if dist.get(&42).map_or(true, |&val| val < cost) {
        dist.insert(42, 42);
    }
}

Note that now there's no more implicit panic from the unwrap call.

Drop a immutable borrow to make a mutable borrow

1 Answers1

After non-lexical lifetimes

Before non-lexical lifetimes

Linked