Make mem::uninitialized and mem::zeroed panic for (some) types where 0 is a niche

I was expecting that this would be a private implementation detail, like Freeze is, and thus the name would be ultimately unimportant. We can add , "It's unsound to use zeroed with this type" to the code sketch to hide it completely.

1 Like

This is in fact one reason why I want the panic -- people using fn() instead of Option<fn()> has been causing SIGILLs recently.

The advantage of an intrinsic is that we could also eventually make it check enums and see if 0 is a valid discriminant and if all fields for that discriminant are 0-valid.

But ultimately I do not care strongly about how this is implemented. The discussion was supposed to be more about whether we want this at all and on which timeline. We should probably wait at least until 1.39 which deprecates mem::uninitialized -- so there were three releases worth of time to port things to MaybeUninit.

2 Likes

Hmm, but any compile-time error better then crash in runtime. Even it says "somewhere in your 1M lines of code program there is code that do wrong thing", it is better then runtime failure.

2 Likes

There is precedent for the opposite (@oli-obk knows the RFC number): when we find out that a piece of code will definitely overflow or otherwise cause a panic at run-time, we have to (the RFC mandates) still only make this a warning, not a hard compiler error.

For example, a compile-time error can be worse if this is dead code in a crate you depend on (now your project doesn't compile any more after a rustc upgrade, great). There is a conflict between such compile-time errors and our stability guarantees.

2 Likes

Easy to remember number, it's like 1337, but not: https://github.com/rust-lang/rfcs/pull/1229

Note that you can always get the compile-time error behaviour via #[deny(const_err)]

2 Likes

Why opposite? I am not picky, if it is not possible to make error, warning would be also fine. Warning during compilation is still much much better then crash during running the program.

And what about rust edition, it is impossible to make transition from warning to error during edition = 20xy change?

3 Likes

@Dushistov then you will like the discussion here.

1 Like

This is a post-monomorphization check, so we can't really give good compile-time errors.

I think that a bad compilation error is better than a panic, in particular, when running crater.


I think we should do this, soon, but with care.

I imagine that code getting hit by this might not be easy to fix, e.g., because some dependencies are exposing FFI bindings that, e.g., use fn types instead of Option<fn> types in FFI or similar. If the libc crate does this somewhere, which it probably does, and some important dependency depends on it, it might take a while till the issue is solved.

So I think the first step would be implementing this, and doing crater runs iterations in which we try to fix the most fundamental crates in the ecosystem, filling issues, etc. EDIT: for these iterations, we can make these emit a compilation error, instead of a panic.

Once that's done, we can enable this on nightly only. Those who run into breakage don't need a flag, because they can just pin an older nightly until their issues are resolved.

Once this has been enabled on nightly for a couple of releases, only then we should consider promoting the check to stable Rust. We probably want to do a couple of crater iterations again with beta and stable Rust releases having this feature enabled, making sure that most of the ecosystem builds.

1 Like

That's a good point.

As discussed in this issue, while we cannot have a perfect lint, and (for the reasons given above) don't want a hard error, we can have a best-effort warn-level lint. So if anyone reading wants to help with this situation, writing that lint would be a great way to do that. :slight_smile:

The lint should find calls of mem::zeroed::<T> and mem::uninitialized::<T>, and it should look at T to see if it can definitely determine that this T is not valid for the all-0 bit pattern. This is basically about recursing into fields of structs and tuples to find a reference or function pointer. Even better if it can also find/identify NonNull and NonZero*. We stop when hitting a type variable, and prefer not to warn over having way too many false positives.

4 Likes

What does "0 is a niche" mean? I've never heard the term niche used this way. Reading drXor's comment, it looks like your using niche to talk about a value that has meaning on its own regardless of initialization? In this case 0 is a "nullptr" and that is the "niche"? Could you elaborate what you mean by your usage of niche here?

"Niche" refers to unused/invalid values for that type, which may then be used by the compiler to represent other states, per PR45225. The most common example is Option<T>, where the niche for T may be used to represent None, since it wouldn't be a valid Some(T). Then we don't need to use any additional memory for the enum discriminant.

In this context, where 0 is a niche for T, then Some(mem::zeroed::<T>()) would effectively create a None instead.

The UCG Glossary also has a definition of "niche".

4 posts were split to a new topic: The term "niche" when referring to disallowed bit patterns and reusing them in option-like enums

I submitted a PR for a lint at

1 Like

It seems mem:: uninitialized::<char/bool>() is allowed with this lint. Is this desired behavior?

I wouldn't say "allowed". It is just not (yet) linted against. This is a conservative lint. It will anyway never catch 100% of the errors, so it starts with a small and very uncontroversial subset and will grow from there.

What if we got the FromZeros trait in the std and then used that as the basis for the lint?

This is a pre-monomorphization lint. Do we have a trait query asking "does this type definitely not satisfy this trait, for all possible instantiations of its generic parameters"?

This is unlike Freeze and other traits we have, where we ask "does this definitely satisfy this trait, for all possible instantiations of its generic parameters".

1 Like

Looking at code like this, we might want to lint on transmute(0usize) to a reference as well.

7 Likes