Calling &self method inside a &mut self method

I came across a case where I want to use a non-mut method while self is borrowed as mutable and I'm not sure what's the best way of approaching this.

use std::collections::HashMap;

#[derive(Default)]
struct Demo {
    map: HashMap<usize, usize>,
}

impl Demo {
    pub fn foo(&self) {
    }
    
    pub fn bar(&mut self) {
        if let Some(v) = self.map.get_mut(&42) {
            self.foo();
            *v = 42;
        }
    }
}

fn main() {
    let mut d = Demo::default();
    d.bar();
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
  --> src/main.rs:14:13
   |
13 |         if let Some(v) = self.map.get_mut(&42) {
   |                          -------- mutable borrow occurs here
14 |             (&*self).foo();
   |             ^^^^^^^^ immutable borrow occurs here
15 |             *v = 42;
   |             ------- mutable borrow later used here

error: aborting due to previous error

What does foo do? Does foo need to access the map, or just other fields?

1 Like

Doesn't need map, only other self fields.

In my particular case, I have a couple of bar()-like methods and foo() is a common helper to handle input params based on a config stored in self. I could solve this by making foo() a free function and passing the config but I'm wondering whether there's something I can do with the structure as it is.

Dealing with exclusive borrows (&mut) leads to having to be very careful with what a functions gets access to or not.

By having foo (self: &'_ Self, ..., you are telling Rust that the foo function may have shared access to any member of Self, including map. This is true even if the body of foo does not access .map, since Rust ignores (other) functions' bodies on purpose.

To solve this, you have three options:

  • the most general solution, is to narrow the input to foo(), by "sacrificing" the method call sugar (instead of self.foo(), you'd do Self::foo(&self.field) to remove that self: &'_ Self parameter, and putting the necessary field_of_self: &'_ Field arguments instead.

  • the other option is to make foo misbehave (with logic bugs or panics) if it ever accesses the .map field (which you are promising doesn't happen.

    Then, bar can take the .map temporarily and put a dummy in its stead, so as not to be &mut borrowing self while the .map is:

    fn bar (self: &'_ mut Self)
    {
        let mut map = mem::take(&mut self.map); // take the map, and put a dummy in its stead
        if let Some(at_v) = map.get_mut(&42) {
            self.foo(); // Ok, gets a dummy `.map`
            *at_v = 42;
        }
        self.map = map; // put it back!
    }
    

    For some situations this has the advantage of requiring the least changes, so it is an easy-to-apply solution. It does have the issue of having to remember to put the thing back into self, and also of no longer statically guaranteeing that foo() does not access .map(): if it does, it will be dealing with the dummy value (here, a Default empty .map(), so there will be nothing to read) that gets discarded afterwards (so anything written to it would be lost). Very bug prone w.r.t. code refactoring.

  • The last option is to stop using &mut within bar(), since exclusive borrows are quite restrictive. In that case, you can used shared mutability (also called interior mutability):

    struct Demo {
        map: HashMap<usize, Cell<usize>>,
    }
    
    impl Demo {
        fn bar (self: &'_ Self)
        {
             if let Some(at_v) = self.map.get(&42) {
                 self.foo();
                 at_v.set(42);
             }
         }
    }
    
1 Like

Thanks @Yandros, that's very useful.

I like the 3rd option, as a C++ dev I keep forgetting about interior mutability. What is the memory/computation overhead of the underlying UnsafeCell?

I'll need to check if it's safe in my case but I think I'll end up with something like

struct Demo {
    map: RefCell<HashMap<usize, usize>>,
}

impl Demo {
    pub fn bar(&self) {
        if let Some(v) = self.map.borrow_mut().get_mut(&42) {
            // ...
        }
    }
}

UnsafeCell by itself has no overhead, save for optimisations blocking. RefCell, on the other hand, has a cost of branching on every borrow call.

If you use RefCell, the only "unsafety" you can get is the runtime panic.

2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.