1

I've encountered a problem that I cannot understand involving mutable references and borrow checker. I am writing a naive binary search tree using safe rust. Full code can be seen here: https://github.com/adensur/rust_learning/blob/master/bst/src/lib.rs
This is the node and the tree itself:

struct Node {
 key: i64,
 left_ptr: Option<Box<Node>>,
 right_ptr: Option<Box<Node>>,
}

#[derive(Default)]
pub struct BstSet {
 root: Option<Box<Node>>,
 len: usize,
}

Here is the insert function (similar to HashSet::insert()), and it works:

pub fn insert(&mut self, key: i64) -> bool {
        let mut node_ptr = &mut self.root;
        while let Some(node) = node_ptr {
            match node.key.cmp(&key) {
                Ordering::Equal => return false,
                Ordering::Less => {
                    node_ptr = &mut node.right_ptr;
                }
                Ordering::Greater => {
                    node_ptr = &mut node.left_ptr;
                }
            }
        }
        *node_ptr = Some(Box::new(Node {
            key,
            left_ptr: None,
            right_ptr: None,
        }));
        self.len += 1;
        true
    }

I try to move this code to a function for some code reuse, and it doesn't compile:

fn find_node_mut(&mut self, key: i64) -> &mut Option<Box<Node>> {
        let mut node_ptr = &mut self.root;
        while let Some(node) = node_ptr {
            match node.key.cmp(&key) {
                Ordering::Equal => break,
                Ordering::Less => {
                    //node_ptr = &mut node_ptr.as_mut().unwrap().right_ptr;
                    node_ptr = &mut node.right_ptr;
                }
                Ordering::Greater => {
                    // node_ptr = &mut node_ptr.as_mut().unwrap().left_ptr;
                    node_ptr = &mut node.left_ptr;
                }
            }
        }
        node_ptr
    }

Giving me this error:

error[E0499]: cannot borrow `*node_ptr` as mutable more than once at a time
  --> src/lib.rs:61:9
   |
46 |     fn find_node_mut(&mut self, key: i64) -> &mut Option<Box<Node>> {
   |                      - let's call the lifetime of this reference `'1`
47 |         let mut node_ptr = &mut self.root;
48 |         while let Some(node) = node_ptr {
   |                        ---- first mutable borrow occurs here
...
61 |         node_ptr
   |         ^^^^^^^^
   |         |
   |         second mutable borrow occurs here
   |         returning this value requires that `node_ptr.0` is borrowed for `'1`

For more information about this error, try `rustc --explain E0499`.

Why does it work in one case but not the other? What is the correct way to reason about this to be able to predict what the compiler would say?

After fiddling about with the code I've found a (somewhat more messy) way to write it so that it works (the commented lines in the code), but the fact that I don't understand why this is happening is bothering me.

Maksim Gayduk
  • 1,051
  • 6
  • 13
  • It doesn't really make sense to borrow `Box`ed values since the `Box` itself is just a pointer to a value that lives on the heap. I haven't tested but you can workaround this by borrowing the `let Some`. By using the syntax `while let Some(ref node) = node_ptr` – Midnight Exigent Jan 19 '22 at 09:35
  • 1
    Implementing double linked lists is not an easy task in rust. There are plenty of questions here on SO which encounter similar problems like you do. Here's some reading material about this topic: https://rust-unofficial.github.io/too-many-lists/ – hellow Jan 19 '22 at 09:49

2 Answers2

1

The main error here is that you have a &mut Option<Box<Node>>, but that doesn't makes much sense, you probably want an Option<&mut Box<Node>>. Thing of it as finding or not something, that Option doesnt need to be a referenced mutable. With that, you can leverage as_mut and and_then to get your code started, then some modifications to checks inside the loop:

impl BstSet {
    fn find_node_mut(&mut self, key: i64) -> Option<&mut Box<Node>> {
        self.root.as_mut().and_then(|mut node_ptr| loop {
            match node_ptr.key.cmp(&key) {
                Ordering::Equal => return Some(node_ptr),
                Ordering::Less => {
                    if let Some(right) = node_ptr.right_ptr.as_mut() {
                        node_ptr = right;
                    } else {
                        return None
                    }
                }
                Ordering::Greater => {
                    if let Some(left) = node_ptr.left_ptr.as_mut() {
                        node_ptr = left;
                    } else {
                        return None
                    }                
                }
            }
        })
    }
}

Playground

Netwave
  • 40,134
  • 6
  • 50
  • 93
  • The semantic of ```&mut Option>``` is the same as ```Node**`` in C: it is a pointer to a pointer, allowing me to find a node with a certain key as well as its "place" inside a tree, effectively allowing me to change the parent node's link, setting it to None, for example. That wouldn't be possible with ```Option<&mut Box>```, because modifications of this box will invalidate the original reference – Maksim Gayduk Jan 19 '22 at 10:05
  • @MaksimGayduk,uhm, I understand the approach. But, in this case you would want at `&mut` to the node you want to modify the ptr itself. I think with the other approach it would be really convoluted. – Netwave Jan 19 '22 at 10:11
0

Seems like this thread Returning a reference from a HashMap or Vec causes a borrow to last beyond the scope it's in? that refers to a known issue https://github.com/rust-lang/rust/issues/21906#issuecomment-73296543 answers my question. Running my program with Polonius borrow checker also seems to work:

RUSTFLAGS="-Z polonius" cargo +nightly test

And here is the perfect video explaining just the thing: https://www.youtube.com/watch?v=_agDeiWek8w
How to properly reason about this to determine why does insert function compile, and find_node_mut doesn't:
In both functions, node and node_ptr are used interchangeably, so their scope sort of "overlaps". In insert function, the compiler is able to properly determine the "liveness" of the loans: the variable is "dead" if it is about to get written to (current value cannot be referenced in the future). That way, even though the two mutable references have overlapping scopes, their 'lifetimes', represented by a set of lines, do not overlap.
In find_node_mut, the reference is returned from the function, blowing out the lifetime of the reference from a small set of lines to some "unknown" lifetime 'a, covering at least the entire function body.

Maksim Gayduk
  • 1,051
  • 6
  • 13