Open jamesmunns opened 2 weeks ago
I've gotten feedback from @RalfJung in Zulip that it's probably more reasonable to have set_discriminant()
take an *mut T
rather than an &mut MaybeUninit<T>
, and I think that is reasonable. I'll probably update this draft to switch these in a bit, if there are no counter-objections.
Edit: Changes made.
Addressed all review comments so far. Link to the diff of changes since the initial version: https://github.com/rust-lang/rfcs/pull/3727/files/5c74b691972d36d30e27f253e8e2673d9930316f..eb486ef529eea98b7d2d9b0ffcd2254ac16a4ea0
What's the current legality of initializing niche-optimized enums?
let mut x: Option<NonNull<u8>> = None;
(&raw mut x).cast::<NonNull<u8>>().write(NonNull::dangling()); // UB?
I suppose there are guarantees made about the layout of Option for FFI purposes, but this feels... sketchy (though miri says it's fine, including with more complex niches like char). That to say, unless these semantics are already defined, I feel like the section about calling set_discriminant on niche-optimized enums should be strengthened from "encouraged for explicitness" to "should"/"must". Maybe with an exception for Option/Result-like enums with a single niche at 0, since FFI-compatibility guarantees are already made for those, but I'd err on not allowing it if possible.
I was thinking about the (possible) future alternative receivers for set_discriminant
:
.as_mut_ptr()
is not so bad, what is more annoying is the loss of guarantees. Yes my &mut MaybeUninit
has given me a non-null & sufficiently-aligned pointer, thank you very much. This is the true boilerplate.set_discriminant_mu
and set_discriminant_nn
(or whatever) feels... heavy. Remembering the specific names is going to be annoying.SetDiscriminant::set_discriminant
trait or a set_discriminant<T: SealedDiscriminant>(t: T, ..)
method muddle safety guarantees: it makes it harder, at the call site, to ensure that the expected receiver is used, and it makes it brittle with regard to refactorings. Change the "receiver" from NonNull<T>
to *mut T
? Make sure the pointer is non-null now!NonNull
and MaybeUninit
directly is another possibility. If just called set_discriminant
they would have some of the disadvantages of the trait-based approach, but for MaybeUninit
a write_discriminant
inherent method could be more in keeping with the current write...
API and the different name would prevent a maintenance mistake.So, all in all, for now I see two alternatives I like:
unsafe
.MaybeUninit::<T>::write_discriminant(&mut self, disc: Discriminant<T>)
. It's natural to have, and offers a lot of guarantees that *mut T
doesn't -- non null, sufficiently aligned, exclusive access -- leaving only "correctly initialized" to fulfill.
Summary
This RFC proposes a way to write the discriminant of an enum when building it "from scratch". This introduces two new library components, an unsafe
set_discriminant
function, and adiscriminant_of!
macro, which can be used with any enum, regardless ofrepr
or any other details.This RFC is a follow-on to the
offset_of_enum
feature, tracked by https://github.com/rust-lang/rust/issues/120141.Rendered