A borrow checker problem

These code can not compile. I try to understand the error: As long as v is valid, my_dict is mutable borrowed. So when I call my_dict.set_value(2, v), my_dict is mutable borrowed at the same time immutable borrowed.

struct MyDict {
    dict: HashMap<i32, Vec<f32>>,
}
impl MyDict {
    fn new() -> Self {
        MyDict { dict: HashMap::new() }
    }
    fn set_value(&mut self, k: i32, v: &Vec<f32>) {
        let res = v.iter().map(|x| x + 1f32).collect();
        self.dict.insert(k + 1, res);
    }
    fn get_inner_dict(&self) -> &HashMap<i32, Vec<f32>> {
        &self.dict
    }
}

fn main() { 
    let mut my_dict = MyDict::new();
    my_dict.dict.insert(1, vec![1f32, 2., 3.]);

    let v = &my_dict.get_inner_dict()[&1];
    my_dict.set_value(2, v);
}

[E0502] Error: cannot borrow `my_dict` as mutable because it is also borrowed as immutable
    โ•ญโ”€[command_104:1:1]
    โ”‚
 21 โ”‚     let v = &my_dict.get_inner_dict()[&1];
    ยท              โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€  
    ยท                          โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ immutable borrow occurs here
 22 โ”‚     my_dict.set_value(2, v);
    ยท     โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€  
    ยท                โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ mutable borrow occurs here
    ยท                 โ”‚            
    ยท                 โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ immutable borrow later used by call
โ”€โ”€โ”€โ”€โ•ฏ

Maybe I can do this:

impl MyDict {
    fn get_and_set(&mut self, k_get: i32, k_set: i32) {
        let v = &self.dict[&k_get];
        self.dict.insert(k_set, v.iter().map(|x| x + 1f32).collect());
    }
}

But it may not seem brief? What am I missing?

The error is nothing more: it just tells you that while a value is immutably borrowed, it cannot also be borrowed mutably. This is one of the cornerstones of Rust's safety guarantees: aliased mutability is a single-threaded race condition and it can result in dangling pointers and use-after-free (google "iterator invalidation" for the well-known C++ equivalent).

In your case, this can probably be solved by just cloning the value out of the map:

    let v = my_dict.get_inner_dict()[&1].clone();
    my_dict.set_value(2, &v);
3 Likes

For starters, Rust is about safety and correctness, not brevity. While there are a lot of ways to make ergonomic APIs, brevity on its own is not considered a virtue, and there are many cases where more explicit (and likely more wordy) solution is strongly preferred. Some implementation details are too important to hide behind magic APIs.

Yes, this seems to work. Although I wouldn't write it like that myself. The function looks pretty meaningless, with generic names of function and variables which don't tell you much. Neither is the function body more enlightening.

You should never take &Vec<T> as an explicit function parameter. It is strictly more limiting than &[T] parameter, because there is nothing useful which you could do with &Vec<T> but couldn't do with &[T], but an explicit Vec would require the caller to make an extra memory allocation, even if they have no use for it otherwise.

This is one of Clippy lints. I suggest you always run Clippy on your project, and strive to make it warnings-free.

You are doing some complex hidden transformation, which maps the incoming vector. There is no use at all for the vector in parameter, other than be mapped into the new vector. In that case it is generally a better practice to expose that allocation, taking Vec<T> (rather than &Vec<T>) as parameter. This lets the caller know about the hidden allocation cost, and gives them slightly more flexibility in handling the consequences.

In your example, you would need to clone v anyway to satisfy the borrow checker. There is no point in making yet another vector inside of the function, you could reuse the existing one. I.e. I suggest the following solution:

struct MyDict {
    dict: HashMap<i32, Vec<f32>>,
}

impl MyDict {
    fn new() -> Self {
        MyDict { dict: HashMap::new() }
    }
    fn set_value(&mut self, k: i32, mut v: Vec<f32>) {
        for x in &mut v {
            *x += 1f32;
        }
        self.dict.insert(k + 1, v);
    }
    fn get_inner_dict(&self) -> &HashMap<i32, Vec<f32>> {
        &self.dict
    }
}

fn main() {
    let mut my_dict = MyDict::new();
    my_dict.dict.insert(1, vec![1f32, 2., 3.]);

    let v = my_dict.get_inner_dict()[&1].clone();
    my_dict.set_value(2, v);
}
3 Likes

in this line:

let v = &my_dict.get_inner_dict()[&1];

you announced v a reference to

> &self.dict

then,when the life time of v continued, you created a mut ref :

 fn set_value(&mut self, k: i32, v: &Vec<f32>) {

and since we can't have ref and mut ref to same data at the same time you got a error

1 Like

This error actually is fairly subtle, whose real cause may be hard to discern. In particular, why does the nearly equivalent definition (one in which a single function call is inlined) compile correctly?

Consider adding some lifetime annotations to set_value:

    fn set_value<'this, 'a>(&'this mut self, k: i32, v: &'a Vec<f32>) {
        // same as before
    }

This function takes two references whose lifetimes are unrelated; v may live longer or shorter than self, the function doesn't care how they related to one another.

.. Or does it? Crucially, this type means that the reference v must live at least as long as the function invocation. But the function implementation in fact has a much stronger guarantee: the two reference lifetimes will not overlap! In particular:

    fn set_value<'this, 'a>(&'this mut self, k: i32, v: &'a Vec<f32>) {
        // 'this is not active here
        // 'a is active here
        let res = v.iter().map(|x| x + 1f32).collect();
        // 'a is not active here (last use)
        // 'this is active here (first use)
        self.dict.insert(k + 1, res);
    }

Unfortunately, the Rust type system has no way to express this fact. In all function invocations, any lexical references (i.e. function parameters which introduce lifetime variables) are considered to be "active" for the duration of that lifetime, and lifetime parameters on a function type must span the entire function invocation. Rust simply does not permit to do the analysis we did above across a function invocation.

For a series of statements in a single block/function, the borrow checker is able to perform such an analysis. You can rewrite the 2nd implementation as:

  fn get_and_set_2(&mut self, k_get: i32, k_set: i32) {
    let res = {
      let v = &self.dict[&k_get]; // this reference certainly cannot live to point 1 below
      v.iter().map(|x| x + 1f32).collect()
    };
    self.dict /* .insert(k_set, res);
  }

The borrow checker is able to perform the same sort of 'rewriting' to see that you don't in fact have any aliasing references (it doesn't actually rewrite anything, but this is one logical way to think of it).

1 Like