92

I am learning Rust and I don't quite get why this is not working.

#[derive(Debug)]
struct Node {
    value: String,
}

#[derive(Debug)]
pub struct Graph {
    nodes: Vec<Box<Node>>,
}

fn mk_node(value: String) -> Node {
    Node { value }
}

pub fn mk_graph() -> Graph {
    Graph { nodes: vec![] }
}

impl Graph {
    fn add_node(&mut self, value: String) {
        if let None = self.nodes.iter().position(|node| node.value == value) {
            let node = Box::new(mk_node(value));
            self.nodes.push(node);
        };
    }

    fn get_node_by_value(&self, value: &str) -> Option<&Node> {
        match self.nodes.iter().position(|node| node.value == *value) {
            None => None,
            Some(idx) => self.nodes.get(idx).map(|n| &**n),
        }
    }
}


#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn some_test() {
        let mut graph = mk_graph();

        graph.add_node("source".to_string());
        graph.add_node("destination".to_string());

        let source = graph.get_node_by_value("source").unwrap();
        let dest = graph.get_node_by_value("destination").unwrap();

        graph.add_node("destination".to_string());
    }
}

(playground)

This has the error

error[E0502]: cannot borrow `graph` as mutable because it is also borrowed as immutable
  --> src/main.rs:50:9
   |
47 |         let source = graph.get_node_by_value("source").unwrap();
   |                      ----- immutable borrow occurs here
...
50 |         graph.add_node("destination".to_string());
   |         ^^^^^ mutable borrow occurs here
51 |     }
   |     - immutable borrow ends here

This example from Programming Rust is quite similar to what I have but it works:

pub struct Queue {
    older: Vec<char>,   // older elements, eldest last.
    younger: Vec<char>, // younger elements, youngest last.
}

impl Queue {
    /// Push a character onto the back of a queue.
    pub fn push(&mut self, c: char) {
        self.younger.push(c);
    }

    /// Pop a character off the front of a queue. Return `Some(c)` if there /// was a character to pop, or `None` if the queue was empty.
    pub fn pop(&mut self) -> Option<char> {
        if self.older.is_empty() {
            if self.younger.is_empty() {
                return None;
            }

            // Bring the elements in younger over to older, and put them in // the promised order.
            use std::mem::swap;
            swap(&mut self.older, &mut self.younger);
            self.older.reverse();
        }

        // Now older is guaranteed to have something. Vec's pop method // already returns an Option, so we're set.
        self.older.pop()
    }

    pub fn split(self) -> (Vec<char>, Vec<char>) {
        (self.older, self.younger)
    }
}

pub fn main() {
    let mut q = Queue {
        older: Vec::new(),
        younger: Vec::new(),
    };

    q.push('P');
    q.push('D');

    assert_eq!(q.pop(), Some('P'));
    q.push('X');

    let (older, younger) = q.split(); // q is now uninitialized.
    assert_eq!(older, vec!['D']);
    assert_eq!(younger, vec!['X']);
}
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
Shulhi Sapli
  • 2,286
  • 3
  • 21
  • 31

3 Answers3

157

A MRE of your problem can be reduced to this:

// This applies to the version of Rust this question
// was asked about; see below for updated examples.
fn main() {
    let mut items = vec![1];
    let item = items.last();
    items.push(2);
}
error[E0502]: cannot borrow `items` as mutable because it is also borrowed as immutable
 --> src/main.rs:4:5
  |
3 |     let item = items.last();
  |                ----- immutable borrow occurs here
4 |     items.push(2);
  |     ^^^^^ mutable borrow occurs here
5 | }
  | - immutable borrow ends here

You are encountering the exact problem that Rust was designed to prevent. You have a reference pointing into the vector and are attempting to insert into the vector. Doing so might require that the memory of the vector be reallocated, invalidating any existing references. If that happened and you used the value in item, you'd be accessing uninitialized memory, potentially causing a crash.

In this particular case, you aren't actually using item (or source, in the original) so you could just... not call that line. I assume you did that for some reason, so you could wrap the references in a block so that they go away before you try to mutate the value again:

fn main() {
    let mut items = vec![1];
    {
        let item = items.last();
    }
    items.push(2);
}

This trick is no longer needed in modern Rust because non-lexical lifetimes have been implemented, but the underlying restriction still remains — you cannot have a mutable reference while there are other references to the same thing. This is one of the rules of references covered in The Rust Programming Language. A modified example that still does not work with NLL:

let mut items = vec![1];
let item = items.last();
items.push(2);
println!("{:?}", item);

In other cases, you can copy or clone the value in the vector. The item will no longer be a reference and you can modify the vector as you see fit:

fn main() {
    let mut items = vec![1];
    let item = items.last().cloned();
    items.push(2);
}

If your type isn't cloneable, you can transform it into a reference-counted value (such as Rc or Arc) which can then be cloned. You may or may not also need to use interior mutability:

struct NonClone;

use std::rc::Rc;

fn main() {
    let mut items = vec![Rc::new(NonClone)];
    let item = items.last().cloned();
    items.push(Rc::new(NonClone));
}

this example from Programming Rust is quite similar

No, it's not, seeing as how it doesn't use references at all.

See also

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
  • why would the example from Programming Rust is not the same? I don't quite get it. I thought it can also be written as `(&mut q).push('D')` and `(&q).pop`, which does borrow `q` as mutable reference and then immutable reference. – Shulhi Sapli Dec 04 '17 at 00:38
  • ok, I think I get it now. stackoverflow.com/q/36565833/155423 this answers why. – Shulhi Sapli Dec 04 '17 at 01:36
  • @Shepmaster in your example instead of last() returning a reference...what if it moved out the value? Then there would be no reference inside the vec and the immutable borrow will end right there after the statement? (this is just hypothetical scenario. If it returns an owned value..last() would be & and not &mut) – Boss Man Apr 10 '19 at 15:39
  • 2
    @SujitJoshi it's ambiguous what you mean, but if it were either `fn last(self) -> Option` or `fn last(&mut self) -> Option` (a.k.a. `pop`), then yes, it would be fine because the returned value would not be invalidated by further mutating the vector. – Shepmaster Apr 10 '19 at 19:04
  • At least as of today, the original example will not yield a compilation error. – devoured elysium Apr 08 '23 at 04:36
  • I assume the API must have changed and that `last()` nowadays returns a value instead of a reference by default? – devoured elysium Apr 08 '23 at 04:52
  • @devouredelysium [`slice::last` returns an `Option<&T>`](https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last) and always will. Without digging too deep, the change is most likely [non-lexical lifetimes](https://stackoverflow.com/q/50251487/155423). – Shepmaster Apr 14 '23 at 20:34
10

Try to put your immutable borrow inside a block {...}. This ends the borrow after the block.

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn some_test() {
        let mut graph = mk_graph();

        graph.add_node("source".to_string());
        graph.add_node("destination".to_string());

        {
            let source = graph.get_node_by_value("source").unwrap();
            let dest = graph.get_node_by_value("destination").unwrap();
        }

        graph.add_node("destination".to_string());
    }
}
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
Jan
  • 101
  • 2
3

So for anyone else banging their head against this problem and wanting a quick way out - use clones instead of references. Eg I'm iterating this list of cells and want to change an attribute so I first copy the list:

let created = self.cells
  .into_iter()
  .map(|c| {
    BoardCell {
      x: c.x,
      y: c.y,
      owner: c.owner,
      adjacency: c.adjacency.clone(),
    }
   })
   .collect::<Vec<BoardCell>>();

And then modify the values in the original by looping the copy:

for c in created {
  self.cells[(c.x + c.y * self.size) as usize].adjacency[dir] = count;
}

Using Vec<&BoardCell> would just yield this error. Not sure how Rusty this is but hey, it works.

TeemuK
  • 2,095
  • 1
  • 18
  • 17