rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.76k stars 12.76k forks source link

Splitting borrows through smart pointers has confusing diagnostics #72297

Open Manishearth opened 4 years ago

Manishearth commented 4 years ago
use std::cell::RefCell;

#[derive(Default)]
struct NotCopy;

#[derive(Default)]
struct Split {
    one: Option<NotCopy>,
    two: NotCopy,
}

fn read(_: &NotCopy) {}

fn main() {
    let mut split = Split::default();

    // comment this:
    let split = &mut split;

    // uncomment this:
    // let split = RefCell::new(split);
    // let mut split = split.borrow_mut();

    if let Some(ref mut one) = split.one {
        read(&split.two);
        *one = NotCopy;
    }
}

(Playground)

This compiles fine, but if you switch the commented out parts, you get:

error[E0502]: cannot borrow `split` as immutable because it is also borrowed as mutable
  --> src/main.rs:21:15
   |
20 |     if let Some(ref mut one) = split.one {
   |                                ----- mutable borrow occurs here
21 |         read(&split.two);
   |               ^^^^^ immutable borrow occurs here
22 |         *one = NotCopy;
   |         -------------- mutable borrow later used here

The reason this does not error in the original case is that Rust is aware of the semantics of &mut T's DerefMut and can reason about splitting borrows, however it is not aware of the semantics of RefMut<T>'s DerefMut and &split.two is able to read from split.one in a safely-written DerefMut impl.

The fix is to convert the RefMut<T> into an &mut T, via &mut *split before doing any of reads or writes.

One typically expects splitting mutable borrows to work in Rust, so it's weird when it doesn't. It would be nice if the compiler could detect when a situation is one of splitting borrows through a DerefMut impl and make the appropriate suggestion.

Manishearth commented 4 years ago

It would also be kinda nice if we had a way to mark Deref/DerefMut impls as pure

Manishearth commented 4 years ago

cc @estebank

estebank commented 9 months ago

Triage: no change