Cannot borrow as immutable because it is also borrowed as mutable error when move On-Stack Dynamic Dispatch code into method

help
1

Hi everyone.

I am working on my json iterator and this iterator has a method, where I need to have some immutable and mutable references:

pub fn step_over(&mut self, forward: bool) {
        let current_node: &mut Value = if self.state.path.is_empty() {
            &mut self.origin
        } else {
            self.origin.pointer_mut(&self.state.path.join("/")).unwrap()
        };
        // let current_node: &mut Value = self.get_current_node();

        match current_node {
            Value::Object(map) => {
                if let NodeKeys::Object(keys) = &self.state.keys {
                    let key = keys.get(self.state.current_index.unwrap()).unwrap();
                    // println!("{key}");
                    let val = map.get(key).unwrap();
                    // println!("{} = {:?}", key, val);
                }
            },
            _ => {},
        }
    }

So, here I use this Rust idiom - On-Stack Dynamic Dispatch.

But, when I try to move current_node into separate method:

fn get_current_node(&mut self) -> &mut Value {
    if self.state.path.is_empty() {
        &mut self.origin
    } else {
        self.origin.pointer_mut(&self.state.path.join("/")).unwrap()
    }
}

// and then in step_over method:
let current_node: &mut Value = self.get_current_node();
match current_node {
    Value::Object(map) => {
        if let NodeKeys::Object(keys) = &self.state.keys {
            let key = keys.get(self.state.current_index.unwrap()).unwrap();
            let val = map.get(key).unwrap();
        }
    },
    _ => {},
}

Could somebody explain why it works in this way ? For me it is not transparent. I tried to move self.state.path.is_empty() into scoped variable inside get_current_node - but this not helped.

I have an error:

error[E0502]: cannot borrow `self.state.keys` as immutable because it is also borrowed as mutable
2

This looks like the classic interprocedural conflicts problem.

The simplest explanation for why it currently does not work is that the compiler only does type checking with function signatures; it does not analyze the function bodies. When it sees this signature:

fn get_current_node(&mut self) -> &mut Value

It says, "Aha! When this is called, the entire Self will be exclusively borrowed until the return type is dropped."

And indeed, this is the situation that the error is describing. self is mutably borrowed, and other fields on self cannot be accessed while that loan is alive.

The linked blog post lists some workarounds, but my favorite is to not introduce these &mut self receiver methods for anything that only needs to borrow a single field. Move the method to that specific field instead. In your case, the function body has two fields to deal with, but &self state.path can be passed to Origin::get_current_node(), instead.

impl Origin {
    fn get_current_node(&mut self, path: &Path) -> &mut Value {
        if path.is_empty() {
            self
        } else {
            self.pointer_mut(&path.join("/")).unwrap()
        }
    }
}

impl MyParent {
    pub fn step_over(&mut self, forward: bool) {
        // The compiler can now see that the loan from `self.origin` is
        // disjoint from other uses of fields on `self`.
        let current_node: &mut Value = self.origin.get_current_node(&self.state.path);
        // ...        
    }
}
3 Likes
3

I see now that what I called "Origin" is probably a JSON Value and you may not be able to implement an inherent method on it. An extension trait could work in this case:

trait ValueExt {
    fn get_current_node(&mut self, path: &Path) -> &mut Self;
}

impl ValueExt for Value {
    fn get_current_node(&mut self, path: &Path) -> &mut Self {
        if path.is_empty() {
            self
        } else {
            self.pointer_mut(&path.join("/")).unwrap()
        }
    }
}
4

Thank you for the article and for the explanation ! So, I refactored the code and moved immutable borrows outside, and also refactored path into &str, so this is working result:

pub fn step_over(&mut self, forward: bool) {
    // ...

    let current_index = self.state.current_index.unwrap();
    let keys = mem::take(&mut self.state.keys);
    let current_node = self.get_current_node();

    match current_node {
        Value::Object(map) => {
            if let NodeKeys::Object(keys) = &keys {
                let key = keys.get(current_index).unwrap();

                let val = map.get(key).unwrap();
                println!("{} = {:?}", key, val);
            }
        },
        _ => {},
    }
    
    self.state.keys = keys;
}

fn get_current_node(&mut self) -> &mut Value {
    if self.state.path.is_empty() {
        &mut self.origin
    } else {
        self.origin.pointer_mut(self.state.path).unwrap()
    }
}

I temporary own the keys from the state and after usage I put it back.