Why the need to go round the houses?

The following code snippet compiles, but if you remove the comment around process1, it fails to. The two functions process1 and process2 are intended to do the same job, but the simpler, process1, is the one that doesn't compile, forcing a more verbose workaround in process2.

use std::collections::HashMap;

pub struct Thing {
    param: i64,
    data: HashMap<String, i64>,
}

impl Thing {
    fn expensive_derivation(&self, i: i64) -> i64 {
        i * self.param
    }
/*
    fn process1(&mut self, s: &str) {
        if let Some(v) = self.data.get_mut(s) {
            let derived = self.expensive_derivation(*v);
            *v = derived;
        }
    }
*/
    fn process2(&mut self, s: &str) {
        let mut pending: Option<i64> = None;

        if let Some(v) = self.data.get(s) {
            pending = Some(self.expensive_derivation(*v));
        }

        if let Some(p) = pending {
            if let Some(v) = self.data.get_mut(s) {
                *v = p;
            }
        }
    }
}

(Playground)

The error is:

   Compiling playground v0.0.1 (/playground)
error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
  --> src/lib.rs:15:27
   |
14 |         if let Some(v) = self.data.get_mut(s) {
   |                          --------- mutable borrow occurs here
15 |             let derived = self.expensive_derivation(*v);
   |                           ^^^^                      -- mutable borrow later used here
   |                           |
   |                           immutable borrow occurs here

error: aborting due to previous error

The question I have is, what bad thing is the Rust compiler trying to prevent with this compilation error? Since expensive_derivation can't mutate self, the call should not cause unexpected stuff to happen from process1's point of view. I understand why mixing immutable and mutable references in the general case can lead to problems, but what could be the problem being prevented here?

If it compiled, then expensive_derivation could mutate self through the given argument, since this argument borrows from self. If expensive_derivation somehow uses the fact that self is immutable (and compiler is free to do so), this would lead to error.

Doesn't the &self in the declaration fn expensive_derivation(&self, i: i64) -> i64 prevent mutation of self in expensive_derivation, enforced by the compiler? Wouldn't you have to declare it as &mut self in the signature to allow mutation in the function?

Well, how exactly? Methods that mutate their instance have &mut self and methods that don't mutate their instance have &self in their signatures, right? That's my understanding from the documentation.

Down to the compiler not being smart enough. It can't look into functions to see how they do or don't modify internal data. Its a bit too dumb with arguments.

Make a small change and get;

    fn process1(&mut self, s: &str) {
        if let Some(v) = self.data.get_mut(s) {
            let iv = *v;
            let derived = self.expensive_derivation(iv);
            *v = derived;
        }
    }
error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
  --> src/lib.rs:16:27
   |
14 |         if let Some(v) = self.data.get_mut(s) {
   |                          --------- mutable borrow occurs here
15 |             let iv = *v;
16 |             let derived = self.expensive_derivation(iv);
   |                           ^^^^ immutable borrow occurs here
17 |             *v = derived;
   |             ------------ mutable borrow later used here

Note there is the functional way;

    fn process3(&mut self, s: &str) {
        self.data
            .get(s)
            .map(|v| self.expensive_derivation(*v))
            .map(|p| *self.data.get_mut(s).unwrap() = p);
    }
    fn process1(&mut self, s: &str) {
        if let Some(v) = self.data.get_mut(s) { // -- mut borrow --+
            let derived = self.expensive_derivation(*v);        // |
            *v = derived; // <------ lasts at least up until here -+
        }
    }

And the mut (i.e., exclusive) borrow originated from self, meaning that you cannot go and get another borrow on self at the let derived line, since that would contradict the exclusivity of the v borrow.

Two solutions:

  • Using a non-exclusive mutable reference, such as &Cell:

    1. Make the data field "mutable":

      struct ... {
          data: HashMap<String, Cell<i64>>,
      }
      
    2. No longer use exclusive references

      if let Some(v/*: &Cell<i64>*/) = self.data.get(s) {
         let derived = self.expensive_derivation(v.get());
         v.set(derived);
      }
      
  • Use two distinct borrows for reading and writing:

    if let Some(&mut v) = self.data.get_mut(s) {
        let derived = self.expensive_derivation(v);
        let at_v = self.data.get_mut(s).unwrap();
        *at_v = derived;
    }
    
1 Like

A way easier and more obvious solution is to just split up the borrows so that the compiler can reason about individual fields, instead of borrowing the entirety of self with no reason:

impl Thing {
    fn expensive_derivation(param: &i64, i: i64) -> i64 {
        i * *param
    }

    fn process(&mut self, s: &str) {
        if let Some(v) = self.data.get_mut(s) {
            *v = Self::expensive_derivation(&self.param, *v);
        }
    }
}
6 Likes

I get that it won't let me make an immutable borrow while the mutable borrow is in effect, but my question was, and is - why is that the case? What undefined (or other bad) behaviour is it trying to prevent? Or is it, as jonh says, just that the compiler is "too dumb" and applies these rules in a sledgehammer way?

I sort of did that in my process2 example. Your example, coded as

    fn process3(&mut self, s: &str) {
        if let Some(&mut v) = self.data.get_mut(s) {
            let derived = self.expensive_derivation(v);
            let at_v = self.data.get_mut(s).unwrap();
            *at_v = derived;
        }
    }

compiles, but notice that self was borrowed mutably on the if let Some(...) line and immutably on the next, expensive_derivation line - exactly as in the process1 example - but now it doesn't complain that you

cannot borrow `*self` as immutable because it is also borrowed as mutable

but why is that? At the very least, it suggests that the error message doesn't indicate the true nature of the problem, because both process1 and process3 have a mutable borrow immediately followed by an immutable borrow - but one is allowed, and the other isn't.

That works in this toy example I put together, but in the real case I'm working on, there are additional fields which are read in the computation. Of course, one could pass a long argument list with these fields, but that just makes an ugly interface to get around a compiler roadblock which I can't see the reason for.

I really appreciate the feedback you've all given - but I'm trying to understand if there's an actual worked through reason why the specific process1 use case is blocked. In concurrency environments it's not uncommon to have many readers, but only one writer allowed - and similarly, I understand the reason for not allowing multiple concurrent mutable borrows (as they could write over each other's changes and make assumptions about what's read problematic) but I can't see a reason why an immutable borrow can't be used in a block which starts with a mutable borrow. After all, any number of calls to immutable methods (&self rather than &mut self) can't change the state of self, and while an immutable method call is in progress, the caller can't do anything to the state of self either, so the called immutable function can, it seems to me, proceed with the passed state of self.

Another question about Yandros' example in process3 above - why is the second mutable borrow allowed inside the first mutable borrow? I interpret the borrows in that example as

    fn process3(&mut self, s: &str) {
        if let Some(&mut v) = self.data.get_mut(s) {     <-- first mutable borrow
            let derived = self.expensive_derivation(v);  <-- first immutable borrow
            let at_v = self.data.get_mut(s).unwrap();    <-- second mutable borrow
            *at_v = derived;
        }
    }

Have I understood that correctly? I can see that the second mutable borrow might be OK because v isn't used later, but self is used in all three lines and the error messages all refer to borrowing self, not v.

I'll redirect you to this other post of mine: Immutable references

I just want to insist on this point: a &mut reference / borrow is one that is exclusive. It so happens that one can derive interesting sound / unchecked properties out the exclusivity property.

The most famous one is sound unsynchronized mutation. But there are others, that may or not be useful and that may or not be currently exploited by the compiler, but in all cases, the compiler is allowed to perform optimizations based on this exclusivity, so one is not allowed to break it (else it is a contract violation with the compiler, i.e., Undefined Behavior).

One example of such optimization is that data that is pointed to by a shared reference &_ and that is not wrapped in a mutability wrapper (UnsafeCell, Cell, Atomic{Integer}, Mutex, etc.) is then immutable for the duration of the borrow / shared reference.

This allows the compiler to perform the following optimization:

fn delta (data: &i32, do_stuff: impl FnOnce()) -> i32
{
    let before = *data;
    do_stuff();
    let now = *data;
    return now - before;
}
// becomes
fn delta (_: &i32, do_stuff: impl FnOnce()) -> i32
{
    do_stuff();
    return 0;
}

If the exclusiveness of &mut was not enforced, then do_stuff could be mutating the data and the transformation would not be valid.


In your example, you are using a helper function,

pub struct Thing {
    param: i64,
    data: HashMap<String, i64>,
}

impl Thing {
    fn expensive_derivation(&self, i: i64) -> i64 {
        // ???
    }

Now, something important to understand is that Rust does not take into account what a function does.
In this example, expensive_derivation is allowed to have shared access over all of Thing fields, even if you are only using param. So Rust assumes that you may very well access the data HashMap, which is guaranteed to be read-only for the duration of the expensive_derivation call.

But what process1 expresses is that from the .get_mut() call up until the *v = write, you have exclusive access to data. If someone wants to read data now the only way they can do it is through the v reference. And yet your self.expensive_derivation(*v); is trying to give read-access to data through self, i.e., not through v, to the function.

In this example the Rust &mut exclusive abstraction is not protecting you from anything bad, and that's also the reason there are many workarounds to this compilation error:

  • one can "relax" the whole "I want an exclusive reference from here and up until here" thing,

    • either by not requiring exclusivity, as with my HashMap<_, Cell<i64>> example,

    • or by using a new distinct &mut borrow right before writing (thus allowing an area of non-exclusivity in between the initial .get_mut() and the second one).

  • or one can relax the "expensive_derivation needs shared access to all of Thing fields", by changing the signature of the function to one more precise, as shown by @H2CO3.

But imagine this other code:

    fn process1(&mut self, s: &str) {
        if let Some(v) = self.data.get_mut(s) {
            let v_copy = *v;
            *v = 42;
            let derived = self.expensive_derivation(v_copy);
            *v = derived;
        }
    }

Now the question is: can we remove that seemingly useless *v =42? If the exclusiveness of the &mut access to v is enforced, then we can do so, because we can just know that nobody is reading that 42 value. But if we allowed the self.expensive_derivation(v_copy) call to happen, then the read access of all of self means that whether the 42 has been written or not is potentially observable, so the optimization would not be valid.

6 Likes

One thing to realize is that the compiler doesn't look inside other functions when performing the borrowcheck. It doesn't matter that your functions don't interact in pratice — functions with those signatures are allowed to do things that would be invalid if your simultaneous borrows were allowed.

Maybe get_mut temporarily replaced the hashmap with uninitialized data, which it would be undefined behavior to read. Maybe get_mut spawned another thread which is currently modifying the hash map, and it would be a race condition to read from it. Maybe there's a RefCell somewhere and the expensive_derivation modified the hashmap and invalidated the mutable reference. Maybe there's some other thread that communicates with the thing using channels, and the combination would send invalid messages that cause undefined behaviour.

I admit the usual obvious examples don't work here due to the second borrow being the immutable one, along with the immutable borrow quickly ending. That said, realize also that desugaring of function calls works like this:

Thing::expensive_derivation(&self, v);

If you can create the immutable reference so you can pass it to expensive_duration, you must also be able to create it for other purposes, including those that involve keeping it around, then modifying stuff through the mutable reference, then looking at the immutable data that has now changed.

All of these things are allowed by the API you have exposed, and there are very good reasons not to look inside functions for borrowchecks:

  1. It would be impossible to reason about what would be a breaking API change if more than the function signatures was important.
  2. The compiler is already slow with local reasoning, and you want global reasoning?

Regarding the example that works: Since you destructured the return from get_mut with Some(&mut v), the v variable is actually a copy, not a reference: Thus the borrow ends before you call expensive_derivation, making it ok.

Edit: I just want to reiterate that this is an incredibly important point:

4 Likes

The compiler intentionally chooses to only rely on what the coder said in the function signature. That's very important because it allows you to do things like implement a function as panic!("not implemented yet") without changing what other functions in your code are allowed to do. Similarly, it means that changing the internals of a function can only cause compiler errors inside that function, never in other functions that call it.

The compiler could implement more relaxed rules, but Rust has intentionally chosen to have that "firewall" between things.

6 Likes

In other words, Rust intentionally supports nearly painless refactoring, greatly reducing the burden of code maintenance.

1 Like

The compiler can't (and the type system doesn't try to) solve the halting problem. Probably with any sort of strong type system, there will be cases where the compiler isn't and can't be smart enough to allow some "valid" code.

You have to live with it, rewrite your code in a way that does work, and move on, really.

Thanks all for your comments. I'll read that and the other linked posts for more insights.

Did you get the answer about why two mutable references are allowed in code snippet you mentioned at the end in function process3? I think I understood other comments but this is something I cannot get my head around.

It's because v is not used anymore before that second mutable borrow happens, so the compiler can end the borrow earlier than the end of the block.

1 Like

On I see, non lexical lifetimes hit again! Thank you.