Stabilize const_maybe_uninit_zeroed and const_mem_zeroed

[tgross35]

Make MaybeUninit::zeroed and mem::zeroed const stable. Newly stable API:

// core::mem
pub const unsafe fn zeroed<T>() ->;

impl<T> MaybeUninit<T> {
    pub const fn zeroed() -> MaybeUninit<T>;
}

This relies on features based around const_mut_refs. Per @RalfJung, this should be OK since we do not leak any &mut to the user.

For this to be possible, intrinsics assert_zero_valid and assert_mem_uninitialized_valid were made const stable.

Tracking issue: #91850
Zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/.60const_mut_refs.60.20dependents

r? libs-api
@rustbot label -T-libs +T-libs-api +A-const-eval
cc @RalfJung @oli-obk @rust-lang/wg-const-eval

[tgross35]

@rustbot label -T-libs

[tgross35]

cc implementer @lilasta

[RalfJung]

LGTM, but the FCP should probably include t-lang since this makes the internal mutable referece/ptr support in our const-eval engine accessible from stable for the first time. I'm not at all concerned about doing so (that support is very mature), but it's worth a bit of extra scrutiny.

[dtolnay]

@rfcbot fcp merge

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @tmandry

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[scottmcm]

Should we make https://doc.rust-lang.org/std/mem/fn.zeroed.html const at the same time, since it's a simple wrapper?

But either way, no concerns from me about doing this. Letting people have a zeroinitializer easily makes sense.

@rfcbot reviewed

(And we could easily add an intrinsic to get a zeroed value if for some reason we had to stop using &mut in the implementation of this for a while.)

[RalfJung]

Should we make https://doc.rust-lang.org/std/mem/fn.zeroed.html const at the same time, since it's a simple wrapper?

Sure, why not.

[scottmcm]

nit: could we find a better example? This could be PluginInfo { id: 0, action: None }, which is both shorter and doesn't need unsafe.

(And please put a // SAFETY: … comment on any unsafe { … } used in examples, to encourage good behaviour and explain why it's not UB.)

[tgross35]

Sure, any suggestions? I was struggling to come up with something better, null-terminated arrays for plugins are just the use case I always come across

[Lokathor]

The entire problem with a good example here is that you use zeroed when the full thing would be a pain to write or even look at. So usually that means that you're talking about some type with 50 integer fields or something. But regardless of details it's "something so big it doesn't fit in a 2 line example".

[dtolnay]

Are we able to use dev dependencies? Is there a libc use case to feature here?

[lilasta]

I think that a generic function can be a good example.

#![feature(const_maybe_uninit_zeroed)]

use core::mem::MaybeUninit;

struct Time32(u32);
struct Time64(u64);
struct Time<T>(T);

// I couldn't come up with a good name. Zeroable?
trait SafeZeroed {}

impl SafeZeroed for Time32 {}
impl SafeZeroed for Time64 {}
impl<T: SafeZeroed> SafeZeroed for Time<T> {}

const fn time_zeroed<T>() -> Time<T>
where
    Time<T>: SafeZeroed,
{
    unsafe { MaybeUninit::zeroed().assume_init() }
}

(Well, in this example it makes more sense to define a trait that has TimeTrait::zeroed, etc...)

[Lokathor]

SafeZeroed would actually have to be an unsafe trait in this scenario.

Honestly I don't think we need example code on this particular function/method. It makes the all-zeroed value, that's already extremely simple I think.

[tgross35]

I really just wanted a sanity check test for the functionality, but agree it’s redundant as an example. And that usually it’s with pretty big C structs for plugins, so there isn’t really a good minimal example. I’ll remove the example and change it to a unit test.

(Not in front of a computer now but I will do it tomorrow)

[tgross35]

Nevermind, I was able to do it now

@dtolnaybot ready 🙂

[tgross35]

I updated this PR to do mem::zeroed as well. It depends on intrinsic assert_zero_valid becoming const, I assumed that isn't a problem either.

[RalfJung]

Yeah, assert_inhabited, assert_zero_valid, assert_mem_uninitialized_valid are very similar and can just all be made stably const (since assert_inhabited already is stably const).

[tgross35]

Even easier 🙂 I added assert_mem_uninitialized_valid

[the8472]

I'm not sure what motivated this test but there actually isn't any guarantee that all-zero bytes are a valid value for Option<&'static str> since that's a fat pointer.

NPO is only guaranteed for thin pointers

[RalfJung]

Yeah but just because there is no guarantee doesn't mean we should have no test.

Or put differently, library and ui tests are a rustc implementation detail and not source for stable guarantees. Only docs are stable guarantees.

[Lokathor]

Also we should make that a docs guarantee for &str, as a separate future PR

[the8472]

That'd conflict with niches-for-metadata that I'm working on. The length field will have a larger niche which means it becomes preferred by the layout algorithm and Option::None is no longer zero but some other value.
That's how I found this.

Or put differently, library and ui tests are a rustc implementation detail and not source for stable guarantees.

I mean that this looks like it has been inspired by some real code and that that code is suspect.

[Lokathor]

That sounds extremely unfortunate and that a special case should be put back in or something. But we can go into that on zulip or whatever PR you open.

[tgross35]

This test was just a sanity check that it does indeed do the right thing. I suppose &str might not have been the best optional reference, but that specific choice wasn't motivated by any real code.

Otoh, it should still zero everything even if invalid, even if Miri one day starts flagging this test.

[the8472]

error[E0080]: it is undefined behavior to use this value
   --> library/core/tests/mem.rs:789:5
    |
789 |     static UNINIT: FooPtr = FooPtr([unsafe { MaybeUninit::zeroed().assume_init() }].as_ptr());
    |     ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0].a.<enum-variant(Some)>.0: encountered a null reference
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
    = note: the raw bytes of the constant (size: 8, align: 8) {
                ╾───alloc7286<imm>────╼                         │ ╾──────╼
            }

[tgross35]

Otoh, it should still zero everything even if invalid, even if Miri one day starts flagging this test.

...or the compiler itself I suppose. Feel free to change or #[allow(...)] this test as needed, as long as there isn't any risk of something like MaybeUninit::zeroed()'s output later being fixed up with a non-zero niche in the pointer metadata (seems unlikely)

[RalfJung]

Even if we never guaranteed NPO for wide pointers, changing that seems very risky and I expect it will break some unsafe code out there. Most people won't expect such a discontinuity between thin and wide ptrs.

[RalfJung]

Turns out the actual problem with this test is that it is UB because the type has padding, so the memory will not be all-zeroes after being returned by assume_init(). ;)