Rust: Tracking issue for `#![feature(maybe_uninit_ref)]`

[Centril]

This is a tracking issue for the RFC "Deprecate uninitialized in favor of a new MaybeUninit type" (rust-lang/rfcs#1892).

This issue specifically tracks the following unstable methods:

impl<T> MaybeUninit<T> {
    pub unsafe fn assume_init_ref(&self) -> &T { ... }
    pub unsafe fn assume_init_mut(&mut self) -> &mut T { ... }
}

(NOTE: on current nightly, these methods are still called get_ref and get_mut, but the FCP here decided a rename.)

[RalfJung]

The reason these are unstable is that most of the time, people should use raw pointers for as long as possible. So the question remains whether these methods carry their weight.

Of course, without https://github.com/rust-lang/rfcs/pull/2582 it can be hard to avoid creating references.

[danielhenrymantilla]

I'll repost here something that I posted in the old tracker issue, for the sake of visibility, and because it is something that remains, imho, relevant to the topic at hand:

Unstable get_ref / get_mut are definitely adviseable because the validity of references to uninitialized data (specially integer and floating point data) hasn't yet been settled. However, there are cases where get_ref / get_mut may be used when the MaybeUninit has been init: to get a safe handle to the (now known initialised) data while avoiding any memcpy (instead of assume_init, which may trigger a memcpy).

• this may seem like a particularly specific situation, but the main reason people (may want to) use uninitialised data is precisely for this kind of cheap savings.

Because of this, I imagine assume_init_by_ref / assume_init_by_mut could be nice to have (since into_inner has been called assume_init, it seems plausible that the ref / ref mut getters also get a special name to reflect this).

There are two / three options for this, related to the Drop interaction:

1. Exact same API as get_ref and get_mut, which can lead to memory leaks when there is drop glue;

   • (Variant): same API as get_ref / get_mut, but with a Copy bound;

2. Closure style API, to guarantee drop:

impl<T> MaybeUninit<T> {
    /// # Safety
    ///
    ///   - the contents must have been initialised
    unsafe
    fn assume_init_with_mut<R, F> (mut self: MaybeUninit<T>, f: F) -> R
    where
        F : FnOnce(&mut T) -> R,
    {
        if mem::needs_drop::<T>().not() {
            return f(unsafe { self.get_mut() });
        }
        let mut this = ::scopeguard::guard(self, |mut this| {
            ptr::drop_in_place(this.as_mut_ptr());
        });
        f(unsafe { MaybeUninit::<T>::get_mut(&mut *this) })
    }
}

(Where scopeguard's logic can easily be reimplemented, so there is no need to depend on it)

---

These could be stabilized faster than get_ref / get_mut, given the explicit assume_init requirement.

Drawbacks

If a variant of option .1 was chosen, and get_ref / get_mut were to become usable without the assume_init situation, then this API would become almost strictly inferior (I say almost because with the proposed API, reading from the reference would be okay, which it can never be in the case of get_ref and get_mut)

[SimonSapin]

I agree that these methods are useful after the value has been fully initialized, at which points references are valid not matter which way future language decisions go. However I think that dropping the value should be an entirely separate concern, and should not be conflated in the same APIs as obtaining a reference.

[SimonSapin]

Based on my previous comment, let’s?

@rfcbot fcp merge

[rfcbot]

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

• [x] @Amanieu
• [x] @Centril
• [x] @Kimundi
• [x] @SimonSapin
• [x] @alexcrichton
• [x] @cramertj
• [x] @dtolnay
• [x] @eddyb
• [x] @joshtriplett
• [x] @nikomatsakis
• [x] @pnkfelix
• [x] @scottmcm
• [x] @sfackler
• [ ] @withoutboats

Concerns:

• docs resolved by https://github.com/rust-lang/rust/issues/63568#issuecomment-585981854
• naming resolved by https://github.com/rust-lang/rust/issues/63568#issuecomment-585989759

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!

See this document for info about what commands tagged team members can give me.

[danielhenrymantilla]

So, despite .get_ref / .get_mut being currently unsound when applied on an uninit value, we want to stabilize them with that name? My comment maybe went too far thinking about drops but my point about them being more aptly named as assume_init_by_ref / ..._by_mut still stands. This is an easy but important detail to fix before stabilization: the whole point of MaybeUninit is too avoid asserting "init-ness" too early, and I feel that there will be more misuages with .get_ref/mut methods than with the assume_init_by_ref/mut counterparts.

[Centril]

Naming

I basically agree with @RalfJung's point in https://github.com/rust-lang/rust/issues/63567#issuecomment-521357943 and I feel the answer to the question in https://github.com/rust-lang/rust/issues/63567#issuecomment-543991607 is "yes". It seems appropriate to reflect the assume_init nature of these methods in their names -- after all, there was a reason we named the method .assume_init().

I would propose that we rename these to:

• assume_init_ref
• assume_init_ref_mut (variants: assume_init_mut_ref and assume_init_mut -- I'm not sure exactly what the API convention was...)

(I've tried to pick names that are clear but also not super long at the same time.)

Documentation

Separately, I think fn get_ref/get_mut could use examples in the actual documentation (https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.get_ref and https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.get_mut).

Two aspects should I think be clarified about these methods (with examples):

• You cannot, ~at least for now~, use this to do field-by-field gradual initialization.
• It's not OK to use this to get a slice to pass to Read::read.

These are both examples of "assuming it is initialized before it is" but examples are helpful.
I think it is also a good idea to add positive examples of where this would be used.

(I think this should happen before we stabilize as it tends not to happen for some time otherwise. It is also the quality we have for other methods in the MaybeUninit<T> docs.)

[SimonSapin]

Both good points, let’s formally file them:

@rfcbot concern naming
@rfcbot concern docs

[RalfJung]

After thinking about this a bit, I think I agree with stabilization (subject to the concerns raised by @Centril). In particular when it comes to docs, the big advantage of having these methods compared to letting people do &mut *m.as_mut_ptr() is that there are docs to look at (someone probably raised this above). So we should use these methods as a teaching opportunity for what the rules are around references and uninitialized data. In particular, we should clarify that all the UB rules for these methods equally apply when calling the raw ptr methods and creating a reference manually.

[danielhenrymantilla]

(And also .get_ref/mut references' lifetimes are tied to the borrow on the MaybeUninit, which is not the case with &[mut]* _.as[_mut]_ptr())

I can be the one drafting the docs if you want

[SimonSapin]

@danielhenrymantilla Please do! And submit a PR to get feedback on the wording.

[danielhenrymantilla]

Done (#65948), waiting on review: should I/we also update .as_ptr() and .as_mut_ptr() documentations ?

[scottmcm]

@rfcbot reviewed

I like the functionality, and I'm happy with whatever names people agree on for things here.

[RalfJung]

should I/we also update .as_ptr() and .as_mut_ptr() documentations ?
@scottmcm

What are you proposing to update?

[RalfJung]

In terms of naming, @Centril's assume_init_ref seems fine; I think the parallel name for mutable references then would be assume_init_mut.

[dtolnay]

I believe all of the documentation concerns in https://github.com/rust-lang/rust/issues/63568#issuecomment-544106668 have been resolved as of #65948.

@rfcbot resolve docs

[dtolnay]

This should be ready for a stabilization PR as soon as the rename from MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} goes through (#66174). I don't think we need to start a new checklist from the beginning. I expect everyone is either ambivalent about the name, prefers the new name, or would have spoken up already. But let me know if anyone would prefer otherwise!

[joshtriplett]

I agree with the renaming to include the assume_init_ prefix; with that renaming, this seems fine to me.

[nikomatsakis]

@SimonSapin did you want to resolve the docs concern? I think @dtolnay tried to but you registered them =)

[SimonSapin]

@rfcbot resolve docs

Did we decide on naming?

[Centril]

I think we've sorta decided on naming but it seems like https://github.com/rust-lang/rust/pull/66174 was closed unmerged. Maybe someone could pick up that work or it could be done as part of the stabilization PR?

[SimonSapin]

Applying the rename in the stabilization PR sounds ok to me, as long as we don’t forget to do it.

@rfcbot resolve naming

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.