Unstably constify ptr::drop_in_place and related methods

[clarfonthey]

Tracking: #109342
Supercedes: #145725

Makes methods const:

• core::ptr::drop_in_place
• core::mem::ManuallyDrop::drop
• core::mem::MaybeUninit::assume_init_drop
• <[core::mem::MaybeUninit<_>]>::assume_init_drop
• <*mut _>::drop_in_place
• core::ptr::NonNull::drop_in_place

[rustbot]

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[clarfonthey]

Was hoping just the destruct bound was the issue, but apparently not. Will have to dig deeper into this.

Until then,

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[clarfonthey]

Oh, the scary-looking errors were extremely simple to fix now that I actually investigated them. They were just copying the function signature in the error, which had obviously changed. Hopefully that's all.

[clarfonthey]

That was all, it turns out.

@rustbot ready

[RalfJung]

Please add a test that actually invokes drop_in_place in a const (and checks the destructor gets run).

[clarfonthey]

Thank you not only for reminding me to write that test, but also for helping me notice that I somehow missed ManuallyDrop::drop…

Destructor tests are weird to write, but what I have should be reasonable, I think.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[theemathas]

This code compiles with this PR. Is this intended?

#![feature(const_trait_impl, const_destruct, const_drop_in_place)]

use std::ptr::drop_in_place;

struct Thing(i32);

impl const Drop for Thing {
    fn drop(&mut self) {}
}

static mut X: Thing = {
    unsafe {
        drop_in_place(&raw mut X);
    }
    Thing(1)
};

This also compiles:

#![feature(const_trait_impl, const_destruct, const_drop_in_place)]

use std::ptr::drop_in_place;

struct Thing(i32);

impl const Drop for Thing {
    fn drop(&mut self) {}
}

static mut Y: Thing = Thing(2);

const X: i32 = {
    unsafe {
        drop_in_place(&raw mut Y);
    }
    1
};

This also compiles:

#![feature(const_trait_impl, const_destruct, const_drop_in_place)]

use std::ptr::drop_in_place;

struct Thing(i32);

impl const Drop for Thing {
    fn drop(&mut self) {}
}

static Y: Thing = Thing(2);

const X: i32 = {
    unsafe {
        drop_in_place(&raw const Y as *mut Thing);
    }
    1
};

[theemathas]

This has... interesting interaction with packed structs. The following code compiles:

#![feature(const_trait_impl, const_destruct, const_drop_in_place)]

use std::ptr::drop_in_place;

struct Thing(i32);

impl const Drop for Thing {
    fn drop(&mut self) {
        self.0 = 2;
    }
}

#[repr(packed)]
struct Packed(Thing);

static Y: Packed = Packed(Thing(1));

const X: () = {
    unsafe {
        drop_in_place((&raw const Y).cast_mut());
    }
};

See also #143411

[clarfonthey]

I mean, you've certainly pointed out a lot of compelling reasons why we shouldn't stabilise this feature right now, but also, we can't really test that all those cases aren't occurring without having the unstable constness in the first place.

It's working, just… well, we now have new cases of UB to track and prevent. This is also why we explicitly track which unstable const features are used in stable items, to ensure that we don't accidentally stabilise something unsound. It would, for example, prevent us from properly stabilising const select_unpredictable, which uses this method, until these issues are fixed.

[oli-obk]

This code compiles with this PR. Is this intended?

I think that's a question for opsem to answer. I don't see why this is a new issue considering you can also do all the same at runtime right now (and it would error at compile-time if any mutation would actually happen in the drop impl, at least I think it would?). Or am I missing the point of your code examples entirely? 😅

[RalfJung]

You can't really do this at runtime? Recursive static initializers are a magic special case.
But it is similar to #143047.

[theemathas]

Turns out that Miri also behaves strangely around drop_in_place with static mut at run time. I've reported this as rust-lang/miri#4574.

[oli-obk]

You can't really do this at runtime? Recursive static initializers are a magic special case.

Ah that's what I was missing. But even then, only the first case uses that to get references to uninit memory in the drop impl. The others I don't see a difference to runtime.

[clarfonthey]

Would someone from wg-const-eval be willing to do a proper review of this? I chose the set of methods mostly as just, anything that just defers to ptr::drop_in_place, since I figure they should all be under the same feature flag, and it makes more sense for one of the wg-const-eval folks to review this instead of just rerolling the libs pool, since it is pretty fundamental to const-eval.

(I'm asking this instead of trying to roll wg-const-eval for the review since I don't think that works, but if it does, I can do that instead.)

[ibraheemdev]

r? wg-const-eval

[RalfJung]

Sorry, I don't have time for more reviews currently.
r? @fee1-dead

[rustbot]

fee1-dead is currently at their maximum review capacity.
They may take a while to respond.

[oli-obk]

Sorry it took so long

View changes since this review

[oli-obk]

@bors r+ rollup

[bors]

📌 Commit 07d3e92 has been approved by oli-obk

It is now in the queue for this repository.