Stabilize `const_maybe_uninit_zeroed` and `const_mem_zeroed` by tgross35 · Pull Request #116218 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation51 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.

A-const-eval

Area: Constant evaluation, covers all const contexts (static, const fn, ...)

T-libs-api

Relevant to the library API team, which will review and decide on the PR/issue.

labels

Sep 28, 2023

rustbot removed the T-libs

Relevant to the library team, which will review and decide on the PR/issue.

label

Sep 28, 2023

RalfJung added the T-lang

Relevant to the language team, which will review and decide on the PR/issue.

label

Sep 28, 2023

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 added the needs-fcp

This change is insta-stable, so needs a completed FCP to proceed.

label

Sep 28, 2023

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

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.

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.)

///
/// static PLUGIN_LIST: [PluginInfo; 2] = [
/// PluginInfo { id: 1, action: Some(\|x
/// unsafe { MaybeUninit::zeroed().assume_init() }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

// 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 {}

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I was able to do it now

@dtolnaybot ready 🙂

tgross35 changed the title~~Stabilize const_maybe_uninit_zeroed~~ Stabilize const_maybe_uninit_zeroed and const_mem_zeroed

Sep 29, 2023

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.

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).

Even easier 🙂 I added assert_mem_uninitialized_valid

This comment has been minimized.

kuba-moo pushed a commit to linux-netdev/testing that referenced this pull request

Dec 11, 2023

This patch adds abstractions to implement network PHY drivers; the driver registration and bindings for some of callback functions in struct phy_driver and many genphy_ functions.

This feature is enabled with CONFIG_RUST_PHYLIB_ABSTRACTIONS=y.

This patch enables unstable const_maybe_uninit_zeroed feature for kernel crate to enable unsafe code to handle a constant value with uninitialized data. With the feature, the abstractions can initialize a phy_driver structure with zero easily; instead of initializing all the members by hand. It's supposed to be stable in the not so distant future.

fn const_maybe_uninit_zeroed() {
// Sanity check for `MaybeUninit::zeroed` in a realistic const situation (plugin array term)
#[repr(C)]
struct Foo {
a: Option<&'static str>,
b: Bar,
c: f32,
d: *const u8,
}
#[repr(C)]
struct Bar(usize);
struct FooPtr(*const Foo);
unsafe impl Sync for FooPtr {}

static UNINIT: FooPtr = FooPtr([unsafe { MaybeUninit::zeroed().assume_init() }].as_ptr());

Stabilize const_maybe_uninit_zeroed and const_mem_zeroed by tgross35 · Pull Request #116218 · rust-lang/rust (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stabilize `const_maybe_uninit_zeroed` and `const_mem_zeroed` by tgross35 · Pull Request #116218 · rust-lang/rust (original) (raw)