RFC Legal double reference

[Dynisious]

A proposal for allowing the creation of immutable references to a value WHILE there is an existing mutable reference when the immutable reference will be dropped before the next use of the mutable reference.
This change will mean that you can write Safe Rust without having to worry about the borrow checker as much and make writing functional Rust code much easier.

Rendered

[TimNN]

I don't think that can (easily, if at all) be done without making currently safe code unsound.

Consider for example the following code (which, AFAIK, is safe in current rust -- edit: or not, see below):

use std::{mem, ptr};

fn tmp_move<T, F: FnOnce(T) -> T>(val: &mut T, f: F) {
    let v = unsafe { ptr::read(val) };
    let v = f(v);
    unsafe { ptr::write(val, v) };
}

fn main() {
    let mut b = Box::new(0u32);
    tmp_move(&mut b, |v| {
        mem::drop(v);
        
        // The following line would be allowed by this RFC, however 
        // would cause a use-after-free.
        // println!("{}", &b);
        
        Box::new(1u32)
    });
}

As I understand the RFC, the currently commented println! would be allowed, but would cause a use-after-free. The problem I see, is that unsafe code can currently rely on the promise "immutable XOR mutable borrow" to be safe, which would no longer be true with the proposed semantics. (Also, tools like crater won't really be able to test which crates would be affected by this change).

[TimNN]

I don't think that can (easily, if at all) be done without making currently safe code unsound.

Consider for example the following code (which, AFAIK, is safe in current rust -- edit: or not, see below):

use std::{mem, ptr};

fn tmp_move<T, F: FnOnce(T) -> T>(val: &mut T, f: F) {
    let v = unsafe { ptr::read(val) };
    let v = f(v);
    unsafe { ptr::write(val, v) };
}

fn main() {
    let mut b = Box::new(0u32);
    tmp_move(&mut b, |v| {
        mem::drop(v);
        
        // The following line would be allowed by this RFC, however 
        // would cause a use-after-free.
        // println!("{}", &b);
        
        Box::new(1u32)
    });
}

As I understand the RFC, the currently commented println! would be allowed, but would cause a use-after-free. The problem I see, is that unsafe code can currently rely on the promise "immutable XOR mutable borrow" to be safe, which would no longer be true with the proposed semantics. (Also, tools like crater won't really be able to test which crates would be affected by this change).

[Centril]

@TimNN A nit: tmp_move as currently written is not safe. You can cause double drops. Other than that, your point is well made.

[Centril]

@TimNN A nit: tmp_move as currently written is not safe. You can cause double drops. Other than that, your point is well made.

[TimNN]

@Centril: Good catch! (I checked that tmp_move was not UnwindSafe but didn't consider the double-drop issue). Anyway, I guess you can fix the double-drop by causing a double-panic.

There is also probably a nicer way to illustrate the problem than my original code example, but that was the first thing I could come up with 😄

[TimNN]

@Centril: Good catch! (I checked that tmp_move was not UnwindSafe but didn't consider the double-drop issue). Anyway, I guess you can fix the double-drop by causing a double-panic.

There is also probably a nicer way to illustrate the problem than my original code example, but that was the first thing I could come up with 😄

[eddyb]

cc @nikomatsakis to whom I argued for something similar a while back, but this proposal might be missing a way to avoid data races. I believe we used to allow reading from a borrowed variable a while ago, but that didn't play well when another thread actually held the reference.

[eddyb]

cc @nikomatsakis to whom I argued for something similar a while back, but this proposal might be missing a way to avoid data races. I believe we used to allow reading from a borrowed variable a while ago, but that didn't play well when another thread actually held the reference.

[ExpHP]

This snippet has far greater issues than a being immutably borrowed inside the closure:

1. Closures begin borrowing from their environment as soon as they are created, not just when they are called. In other words, calling a.len() inside the closure does not cause it to be immutably borrowed each iteration, but rather, it is borrowed once, and this borrow lasts for the entire duration of the for_each loop.
2. Similar is true for the mutable borrow. iter_mut() returns a struct that holds a &mut [T]. It holds this reference for the entire duration of the loop.
3. A function like iter_mut is currently allowed to do things that put a in an invalid state (i.e. where len() could invoke undefined behavior), because it knows that it has unique access to that object until the IterMut struct is dropped.

[nikomatsakis]

@rfcbot fcp close

If I understood this RFC correctly, it is very similar to @arielb1's prior proposal of Ref2Φ. I believe that the discussion of Ref2 from the nested method calls RFC is basically still apt. In short, it may be a direction that we want to go eventually, but let's move cautiously, because it could make borrows much harder to understand without a big increase in expressiveness.

In any case, I think that now is not the time to be making these sorts of deeper changes to borrowing. We're still working on integrating NLL and two-phase-borrows, which is going to be a big change in how Rust feels. I am reluctant to tinker more with borrows until we have a string feeling for how that will play out.

Therefore, I move to close this RFC.

Thanks @Dynisious for the suggestion, however! I think this is something we may well want to revisit in the future.

[nikomatsakis]

@rfcbot fcp close

If I understood this RFC correctly, it is very similar to @arielb1's prior proposal of Ref2Φ. I believe that the discussion of Ref2 from the nested method calls RFC is basically still apt. In short, it may be a direction that we want to go eventually, but let's move cautiously, because it could make borrows much harder to understand without a big increase in expressiveness.

In any case, I think that now is not the time to be making these sorts of deeper changes to borrowing. We're still working on integrating NLL and two-phase-borrows, which is going to be a big change in how Rust feels. I am reluctant to tinker more with borrows until we have a string feeling for how that will play out.

Therefore, I move to close this RFC.

Thanks @Dynisious for the suggestion, however! I think this is something we may well want to revisit in the future.

[rfcbot]

Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period is now complete.

[rfcbot]

The final comment period is now complete.

[aturon]

Closing, though open to revisiting later, as per @nikomatsakis's summary. Thanks @Dynisious!

[aturon]

Closing, though open to revisiting later, as per @nikomatsakis's summary. Thanks @Dynisious!