Introducing a redundant variable makes the "cannot borrow as mutable more than once at a time" go away

[nikita-volkov]

Compiling this code:

use std::collections::*;

pub struct BTreeMultimap<key, value> {
  map: BTreeMap<key, Vec<value>>,
}

impl<key: Ord, value> BTreeMultimap<key, value> {
  pub fn remove_equal_or_greater(&mut self, key: &key) -> Option<value>
    where key: Clone
  {
    match self.map.range_mut(key..).next() {
      Some((key, vec)) => {
        match vec.pop() {
          Some(value) => {
            if vec.is_empty() {
              self.map.remove(&key.clone());
            }
            Some(value)
          }
          None => panic!("Vector is never expected to be empty")
        }
      }
      None => None
    }
  }
}

results in the following error:

error[E0499]: cannot borrow `self.map` as mutable more than once at a time
  --> src/logic/states/b_tree_multimap.rs:16:15
   |
11 |     match self.map.range_mut(key..).next() {
   |           -------- first mutable borrow occurs here
...
16 |               self.map.remove(&key.clone());
   |               ^^^^^^^^         --- first borrow later used here
   |               |
   |               second mutable borrow occurs here

However it compiles fine if I introduce an intermediate variable like this:

-              self.map.remove(&key.clone());
+              let cloned_key = key.clone();
+              self.map.remove(&cloned_key);

Complete working code

use std::collections::*;

pub struct BTreeMultimap<key, value> {
  map: BTreeMap<key, Vec<value>>,
}

impl<key: Ord, value> BTreeMultimap<key, value> {
  pub fn remove_equal_or_greater(&mut self, key: &key) -> Option<value>
    where key: Clone
  {
    match self.map.range_mut(key..).next() {
      Some((key, vec)) => {
        match vec.pop() {
          Some(value) => {
            if vec.is_empty() {
              let cloned_key = key.clone();
              self.map.remove(&cloned_key);
            }
            Some(value)
          }
          None => panic!("Vector is never expected to be empty")
        }
      }
      None => None
    }
  }
}

Meta

rustc --version --verbose:

rustc 1.33.0 (2aa4c46cf 2019-02-28)
binary: rustc
commit-hash: 2aa4c46cfdd726e97360c2734835aa3515e8c858
commit-date: 2019-02-28
host: x86_64-apple-darwin
release: 1.33.0
LLVM version: 8.0

[pnkfelix]

This is not a bug. It is the system behaving as expected.

The let-bindings is not just introducing a new variable name; it is also changing order that things run in; in particular, in let cloned_key = key.clone();, the key.clone() call runs in the RHS of the let-binding, to completion, before the self.map is mutably borrowed by the self.map.remove(..) call. Since the key is borrowing a key still held by self.map, you cannot access key while self.map is mutably borrowed.

(An aside: There is a related feature associated with NLL and the 2018 edition, "two-phase borrows", that does allow one to do things like vec.push(vec.len()) instead of { let len = vec.len(); vec.push(len); }. But that deliberately only applies in very narrow circumstances that do not include the one outlined here.)

Closing as "not a bug".)

[nikita-volkov]

Okay. Thanks for clearing that out!