Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow moving out of &'static mut T #125868

Open
Ddystopia opened this issue Jun 1, 2024 · 17 comments
Open

Allow moving out of &'static mut T #125868

Ddystopia opened this issue Jun 1, 2024 · 17 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team

Comments

@Ddystopia
Copy link
Contributor

Hello, it is perfectly sound to move out of &'static mut T and have that reference consumed, as no safe code can access T under the reference after that point.

struct NotDefaultNotCopy([u8; 20]);

fn main() {
    let foo: &'static mut NotDefaultNotCopy = Box::leak(Box::new(NotDefaultNotCopy([0; 20])));
    let foo_taken = *foo;

    println!("{:?}", foo_taken.0);
}

For example, miri is happy with that code if take in unsafely written (but detects leak) playground

Here is code with more unsafe but with no miri leak warnings: playground

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 1, 2024
@workingjubilee workingjubilee added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team labels Jun 1, 2024
@workingjubilee
Copy link
Member

Miri also doesn't complain about this program, one which is surely not what you intend: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=4a15c73887bce9565554ebf5b12e3627

@workingjubilee workingjubilee added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 1, 2024
@saethlin
Copy link
Member

saethlin commented Jun 1, 2024

@Ddystopia your take does not move out, it does a typed copy. The demonstration you want to write cannot be written in surface Rust because you can't get past the compiler frontend. That's the whole point of this issue.

To write the demo you want, you must use custom MIR: https://doc.rust-lang.org/nightly/std/intrinsics/mir/index.html

@QuineDot
Copy link

QuineDot commented Jun 1, 2024

Original(?) discussion started here.

@saethlin
Copy link
Member

saethlin commented Jun 1, 2024

In any case, I think the basic reasoning here is wrong: The 'static bound doesn't help because you can reborrow a &'static mut into another &'static mut.

If the following entirely safe code were to be allowed to compile, we would have access to a moved-from value:

#[derive(Debug)]
struct NotCopy(u8);

fn main() {
    let x: &'static mut NotCopy = Box::leak(Box::new(NotCopy(0u8)));
    let y: &'static mut NotCopy = &mut *x;
    let z = *y;
    println!("{:?}", *x); // oops, use of moved-from value
}

@QuineDot
Copy link

QuineDot commented Jun 2, 2024

The reborrow used to create y makes x unsuable for 'static (forever). The state of x after the move is akin to how replace_with functions. Or std::mem::replace (for the duration of the method body), for that matter.

There's more discussion in this RFC thread.

I personally don't feel there are strong enough upsides for language support, e.g. DerefMove for &'static mut T. But either (the originally posted) fn take is sound or replace_with is unsound.

fn take<T: Send>(reference: &'static mut T) -> T {
    let (sender, receiver) = std::sync::mpsc::channel();
    std::thread::spawn(move || {
        replace_with::replace_with_or_abort(reference, |t| {
            let _ = sender.send(t);
            loop {}
        })
    });
    
    receiver.recv().unwrap()
}

replace_with isn't available on the playground or I'd link a runnable example. But an example usage is

    let foo: &'static mut NotDefaultNotCopy = Box::leak(Box::new(NotDefaultNotCopy([1; 20])));
    let reborrowed_even = &mut *foo;
    let foo_taken = take(reborrowed_even);

    println!("{:?}", foo_taken.0);
    // Trying to use `foo` is an error because `*foo` is exclusively reborrowed forever

@saethlin
Copy link
Member

saethlin commented Jun 2, 2024

Thanks for the correction!

@Ddystopia
Copy link
Contributor Author

I do agree that there might not be urging need for this, but it sometimes might be useful during embedded development and if something will change with borrow checker's attitude to moving out of mutable references, then it might not take a lot of extra work to consider adding this feature, which will not break anything and apparently sound.

@QuineDot
Copy link

QuineDot commented Jun 2, 2024

which will not break anything and apparently sound.

There was a potentially breaking use case in the RFC thread, depending on how the decision goes. As far as I know there hasn't been an FCP on the topic. That said, opinions did seem favorable on the side of this being sound.

That RFC was closed in part due to concerns about adding an API that aborts. I will note that a proposal to add a function like take in the OP (which is limited to the &'static mut _ case) also gives an opportunity to bless the operation as sound without introducing an aborting API (and without trying to tackle unwinding as a whole).

Blessing this operation means that every &'static mut T can bet converted into a newtype which is analogous to a &'static mut Option<T> (which you can move T into and, conditionally, out of; conditionally borrow; etc).

@LunarLambda

This comment was marked as off-topic.

@WaffleLapkin

This comment was marked as off-topic.

@LunarLambda

This comment was marked as off-topic.

@WaffleLapkin

This comment was marked as off-topic.

@LunarLambda

This comment was marked as off-topic.

@WaffleLapkin

This comment was marked as off-topic.

@LunarLambda

This comment was marked as off-topic.

@Ddystopia
Copy link
Contributor Author

Miri also doesn't complain about this program, one which is surely not what you intend: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=4a15c73887bce9565554ebf5b12e3627

Thank you for note! Here is another example, which is closer to what I wanted to demonstrate playground

I also tried custom MIR, but compiler kept ICE-ing and I gave up

@RalfJung
Copy link
Member

I agree that this is sound:

fn take<T>(reference: &'static mut T) -> T {
    unsafe { core::ptr::read(reference) }
}

It would be fairly easy to prove this in Rustbelt as well.

Not sure if it's worth having special support for this in the borrow checker, a dedicated method seems more clear / less magic to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team
Projects
None yet
Development

No branches or pull requests

8 participants