Warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future

Hello,
does anybody know why the following code produces the warning:

warning: cannot borrow h as mutable because it is also borrowed as immutable

use std::collections::HashMap;

fn main() {
    let mut h = HashMap::new();
    h.insert("h", 2);
    if let Some(cur) = h.get("h") { // Immutable borrow
        h.insert("h", cur + 1); // Mutable borrow + use of mutable and immutable borrow
    }
    println!("{:?}", h);
}

whereas

fn main() {
    let mut v = vec![];
    v.push(v.len()); // Mutable borrow + Immutable borrow + use of both
    println!("{:?}", v);
}

produces no warning?

And how do I fix the produced compiler warning in the first code snippet without changing the meaning?

This lint fires when the mutable borrow (reservation) created by a method invocation conflicts with an existing borrow. In your case, the mutable borrow of h in h.insert(...) conflicts with the existing borrow of h created by h.get(...), and which is later used because you use cur + 1 in the method arguments.

v.push(v.len()) does not warn, because no borrows of v exist before that expression is evaluated.

You can fix this by adding .copied() after the call to .get, which gives you a copy of the value in the map, instead of a reference.

v.push(v.len()) works because of two-phase borrows. This is a special exception to the normal borrow-checking rules, to make this common pattern easier to write.

Two-phase borrowing does not apply to your earlier code because it was deliberately limited to one specific form of expression. This limitation was added because it turned out to be very complicated to implement and specify a more general version of two-phase borrowing.

5 Likes

The error points to this issue which has a good description. From that issue:

For example, given a vector v , v.push(v.len()) first reserves a borrow of v when it evaluates the method receiver, but does not activate that borrow until later when transferring control to the push method itself, after v.len() has been evaluated.

Or in other words, in order, you

  • Reserve a mutable borrow of v
  • Immutably borrow v to evaluate v.len(), and then immutable borrow immediately ends
  • Activates mutable borrow from earlier

In particular, the immutable borrow of v didn't exist when the reservation happened.

And adapting their why-this-warns example with Vec to your code, we have

    if let Some(cur) = h.get("h") {
        //      ^^^ 1: The immutable borrow (&i32)
        h.insert(
        // ^^^ 2: the mutable reservation
            "h", cur + 1
            //   ^^^ 3: The use of the immutable borrow
        );
        // 4: activate the mutable borrow from earlier
    }

And you can fix it by finishing up your use of the immutable borrow before the mutable reservation. .copied() was one way already mentioned, or you could do

    if let Some(cur) = h.get("h") {
        let val = cur + 1;
        h.insert("h", val);
    }

However you may benefit more generally from learning how to use Entry:

use std::collections::HashMap;

fn main() {
    let mut h = HashMap::new();
    h.insert("h", 2);
    h.entry("h").and_modify(|cur| *cur += 1);
    println!("{:?}", h);
}

(Playground)

4 Likes

I am a little confused by this explanation. Doesn't Rust evaluate method parameters before doing method calls?

Meaning that an expression like:

...
h.insert("h", *cur +1)`
// more code

should be exactly the same after being destructured into sub-expressions?Let's assume param_1 & param_2 are the passed parameters after evaluation without additonal temporary copies:

// code before
{ // Just to express parameter lifetimes
    let param_1 = "h"; // Evaluation of param 1
    let param_2 = *cur + 1; // Evaluation of param 2 + end of borrow
    h.insert(param_1, param_2); // use of mutable borrow
} // Cleanup of method call parameters
// more code

If the expression in the method call parameters are evaluated before the method gets called (the compiler must evaluate the expression to get the actual method call parameters), shouldn't the use of the borrow end before activating the mutable borrow of the method owner?
@quinedot Maybe this should have been my answer to your post.

The call to insert involves the receiver parameter &mut self, a mutable reference to your HashMap. Here's another way to write it.

// code before
{ // Just to express parameter lifetimes
    let param_0 = &mut h;
    let param_1 = "h";
    let param_2 = *cur + 1;
    HashMap::insert(param_0, param_1, param_2);
} // Cleanup of method call parameters
// more code

I'm not sure if this desugaring order is written down as guaranteed anywhere, but the linked issue implies it.

Mh, this is the thing I don't get. Why should the compiler borrow h before evaluating the parameters. Wouldn't this order make much more sense?:

    let param_1 = "h";
    let param_2 = *cur + 1;
    let self_param = &mut h; // use of immutable borrow of h ended before mutable borrow
    HashMap::insert(self_param, param_1, param_2);

That would solve the problem nicely (at least for the specific cases that two-phase borrowing was designed for), but it would be a backward-incompatible change, since it would change the order of evaluation. (The change would be observable whenever there are side effects from evaluating the argument expressions.)

Having these two be the same is at a minimum desirable:

foo.f(one, two, three);
Foo::f(f, one, two, three);

But also probably required for backwards compatibility. Certainly it is in the general case of

(may().have_a().side_effect).method_call(one, two);

(And carving out exceptions for the simple case may be too unexpected, difficult, error prone, or all the above. And I haven't even touched on Deref.)

1 Like

Consider a method call chain:

a().foo(b()).bar(c())

The functions are called in the order [[a(), b(), foo()], c(), bar()]. If the order of evaluation within method calls were changed to "receiver last", then the overall order would have to be [c(), [b(), a(), foo()], bar()], which would be surprising given that Rust has a general left-to-right rule.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.