Rust: Make core::mem::zeroed() a const fn

[alex]

Right now this isn't callable from const fns. This would be useful for using const fns in combination with FFI.

[alex]

(Apologies if this shouldn't have a dedicated issue, it didn't seem to be covered by any of the existing A-const-fn bugs)

[alex]

I copied mem::zeroed into my project and marked it as as const fn:

error: any use of this value will cause an error
   --> /home/alexgaynor/projects/linux-kernel-module-rust/src/chrdev.rs:125:5
    |
125 |       intrinsics::panic_if_uninhabited::<T>();
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |       |
    |       "calling intrinsic `panic_if_uninhabited`" needs an rfc before being allowed inside constants
    |       inside call to `linux_kernel_module::chrdev::zeroed::<linux_kernel_module::bindings::file_operations>` at /home/alexgaynor/projects/linux-kernel-module-rust/src/chrdev.rs:132:71
    |       inside call to `linux_kernel_module::chrdev::FileOperationsVtable::new::<CycleFile>` at src/lib.rs:14:9
    | 
   ::: src/lib.rs:13:5
    |
13  | /     const VTABLE: linux_kernel_module::chrdev::FileOperationsVtable =
14  | |         linux_kernel_module::chrdev::FileOperationsVtable::new::<Self>();
    | |_________________________________________________________________________-
    |
    = note: #[deny(const_err)] on by default

So there's more work here than simply marking it const. I don't fully understand this error message -- it seems to happen for calls to _any_ intrinsic, so I guess there's a deeper question there.

[alex]

Ah, source diving it seems that message simply means that support for the intrisinc hasn't been added to the MIR interpreter: https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/intrinsics.rs#L44

Does adding support for these need their own bugs, or can they just be covered here?

[alex]

After source diving a bit, I'd be happy to provide a PR to implement panic_if_uninhabited, if it's wanted. I'd be happy to do init as well, but I'd need a bit more guidance, I don't quite understand how that'd work just from looking at the other intrinsic implementations.

[RalfJung]

Cc @oli-obk

mem::zeroed and mem::uninitialized are a mess (the latter is deprecated), so I am opposed to making either of them const fn.

What should become a const fn is MaybeUninit::zeroed. But that is still waiting on some more CTFE work, I think.

[RalfJung]

Ah, source diving it seems that message simply means that support for the intrisinc hasn't been added to the MIR interpreter

All of these intrinsics are already implemented in Miri at https://github.com/rust-lang/miri/blob/master/src/intrinsic.rs. But so far they deliberately were not moved into rustc proper.

[oli-obk]

What should become a const fn is MaybeUninit::zeroed. But that is still waiting on some more CTFE work, I think.

We could make just the zeroed intrinsic const fn then MaybeUninit::zeroed can be const fn immediately and we can un-const-fn the zeroed intrinsic (or even remove it entirely, once we get loops and conditions in const fn)

Or we can try getting

union Foo<T> {
    a: [u8; std::mem::size_of::<T>()],
    b: T,
}

to work, but that's lazy normalization, so... :laughing:

[oli-obk]

mem::zeroed and mem::uninitialized are a mess (the latter is deprecated), so I am opposed to making either of them const fn.

I'm in full agreement, the mem::zeroed and mem::uninitialized functions should never become const fn.

[RalfJung]

My concern is with the implementation of the zeroed intrinsic (actually called init).

What's missing to make MaybeUninit::zeroed a const fn? Probably support for mutable references?

[RalfJung]

But given that people have been asking for this for quite a while... do you think it is realistic to add an implementation of the init intrinsic now, but remove it again later once we got everything together to have the current implementation of zeroed in CTFE?

[RalfJung]

Oh, you basically suggested that above.^^ Sorry.

So I think I wouldn't block that plan:

1. Move the init and uninit implementations from Miri to librustc_mir. (Having the two not in the same place seems odd.)
2. Make init unstable available as a const fn, use that to implement MaybeUninit::zeroed.
3. Open an issue that says we will want to completely kill both of these intrinsics as soon as CTFE gets powerful enough to make MaybeUninit::zeroed a const fn without them.

If we want to avoid seeing uninit in librustc_mir, maybe we can kill that intrinsic today (and implement mem::uninitialized as MaybeUninit::uninit().assert_init()). That would be a separate 0th step then.

[oli-obk]

That would be a separate 0th step then.

Let's do that first, yes

Couldn't we also make mem::zeroed call MaybeUninit::zeroed().assert_init() so that we only have a single use site of intrinsics::zeroed

The rest of the plan sounds good to me. We can also mark the init intrinsic as deprecated and allow that lint in the two places where it's "ok for now". This would help preventing new uses in libstd from popping up.

[RalfJung]

Couldn't we also make mem::zeroed call MaybeUninit::zeroed().assert_init() so that we only have a single use site of intrinsics::zeroed

We could.

[alex]

Just to be clear, the proposal is first a PR that does:

diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs
index 770d1ca8e7..487785b878 100644
--- a/src/libcore/mem/mod.rs
+++ b/src/libcore/mem/mod.rs
@@ -450,8 +450,7 @@ pub const fn needs_drop<T>() -> bool {
 #[inline]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub unsafe fn zeroed<T>() -> T {
-    intrinsics::panic_if_uninhabited::<T>();
-    intrinsics::init()
+    MaybeUninit::zeroed().assert_init()
 }

 /// Bypasses Rust's normal memory-initialization checks by pretending to
@@ -476,8 +475,7 @@ pub unsafe fn zeroed<T>() -> T {
 #[rustc_deprecated(since = "1.38.0", reason = "use `mem::MaybeUninit` instead")]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub unsafe fn uninitialized<T>() -> T {
-    intrinsics::panic_if_uninhabited::<T>();
-    intrinsics::uninit()
+    MaybeUninit::uninit().assert_init()
 }

 /// Swaps the values at two mutable locations, without deinitializing either one.

[oli-obk]

that and completely remove the uninit intrinsic and any code in the compiler that supports it

[oli-obk]

Actually, the panic_if_uninhabited may need to stay, not sure how MaybeUninit handles it

[alex]

panic_if_uninhabited is used in MaybeUninit.

[alex]

Cool, I'll hopefully have a patch up for that shortly.

[oli-obk]

As discussed in the old PR (https://github.com/rust-lang/rust/pull/54832#issuecomment-432263848) we could also allow &mut references in const fn first, then make ptr::write_bytes const fn. That would allow us to skip the init minefield entirely

[RalfJung]

What would it take to allow mutable references in const fn? And given that I could write const FOO: &mut T = foo();, how does that make things any easier compared to allowing them in all consts?

[oli-obk]

Oh, I mean enable them in all consts. I'm not sure about the const qualif implications right now, but we could start by adding them behind a WIP feature gate without having to worry about accidental stabilizations. Wherever a &mut error is reported, only report if the relevant feature gate is not active. Should be a fairly simple PR (add feature gate and use it + some tests)

cc https://github.com/rust-lang/rust/issues/57349

[RalfJung]

But AFAIK currently stable const fn can only use stable const features, so that wouldn't help for a stable const fn zeroed.

[oli-obk]

right, I mean just for working towards &mut stabilization. Babysteps

[Centril]

Baby steps (a more principled approach) sound great. The upside is that we have more incentive to implement, polish, write a bunch of tests, and ship &mut Type / place which is more important anyways.

[alex]

Notes to self:

• Have bindgen emit a cfg/feature/thingy by kernel version
• Bindgen generated/linux/uapi/version.h
• Set fields based on kernel version

[RalfJung]

And that has... what to do with zeroed being a const fn?^^

[alex]

Absolutely nothing at all... I was looking at the wrong tab! Sorry about that!

[RalfJung]

Okay, uninit is gone for good. :-)

init stiill exists, but is deprecated. Progress on this issue will require deciding if we want to go with a minimal-mutable-references-for-CTFE (and killing init), or using init for MaybeUninit::zeroed until CTFE got more powerful.

Also @oli-obk is on vacation so we should probably wait until they are back.^^

[Centril]

deciding if we want to go with a minimal-mutable-references-for-CTFE (and killing init)

I'm strongly in favor of killing init and going with mutable references as the cleaner and principled approach (which is consistent with the principled approach we've taken thus far with const fn). As aforementioned it's also a good incentive to get &mut place/T working in const fn which seems more important.

[mjbshaw]

For some prior art, see #54832, where I tried to make MaybeUninit::zeroed a const fn, but was ultimately rejected due to it relying on init (and didn't have any viable alternative since &mut isn't const-friendly) (edit: whoops, I overlooked the comment above that mentions this).

I would love to revive #54832 if people are okay with using init temporarily until CTFE has minimal support for mutable references.

As aforementioned it's also a good incentive to get &mut place/T working in const fn

I get that, and to a degree I agree, but to be honest I don't think it's working. We have people who could really use a const MaybeUninit::zeroed today, but this has been blocked on CTFE for a really long time.

[oli-obk]

I get that, and to a degree I agree, but to be honest I don't think it's working.

I can guide you through implementing mutable references in const fn if you want. I believe it's not much work and stabilization wise it's just an extension of let bindings in const fn. The only reason I didn't add mutable references together with let bindings was a "small steps" approach which resulted in a standstill (and noone was pushing for mutable references).

[RalfJung]

Okay, uninit is gone for good. :-)

Status update: that PR was reverted because it broke some programs. Those programs all had UB, but we reverted to give them more time to fix their stuff.

But the main blocker for this issue anyway is &mut T in const, and I heard rumors that @christianpoveda is working on this? Any updates?

[christianpoveda]

Hi Ralf, here is the related PR: https://github.com/rust-lang/rust/pull/66606