r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

r? @SimonSapin

cc @RalfJung

This is more consistent with MaybeUninit. I am not sure if that's what we want, but I proposed it, so I am curious what T-libs thinks. :)

It's surprising to me that a method called read would mutate the container, especially to the point of leaving its contents in an invalid state.

method called read would mutate the container

It doesn't actually mutate the container, just like ptr::read doesn't mutate what's behind the pointer. Rather, the state is changed logically (just like with ptr::read which logically moves the value out) without changing anything in memory.

I only used &mut slot.value because I already had &mut and forgot I didn't need it 😅. The only reason ManuallyDrop::read takes &mut currently rather than & like MaybeUninit::read is as a lint; it's almost certainly unsound to use if someone else has a reference as well. That said, MaybeUninit::read uses &self and ptr::read uses *const, so if the idea is to parallel those, this should probably use &slot as well.

The other option where we use take(&mut self) is treating ManuallyDrop as basically an untagged, unchecked Option with Deref sugar due to being Some 99.99% of its lifetime. The only time I expect this function will be used soundly is in drop(&mut self).

That said, MaybeUninit::read uses &self and ptr::read uses *const, so if the idea is to parallel those, this should probably use &slot as well.

Good point, now I wonder if MaybeUninit::read should take &mut self...

Relevant note: this method has just been mentioned in a blog post as take.

The link seems to be broken?

@RalfJung fixed (missing http:// messed up GitHub's URL parser)

Relevant note: this method has just been mentioned in a blog post as take.

I'm sure @pnkfelix could change that if need be when we decide. :)

ping from triage @SimonSapin any updates on this?

Per #55422 (comment), MaybeUninit::read may get renamed, removing that reasoning for the rename. Whether the function gets called read or take, the documentation updates in this PR I think are a clear improvement.

I'm happy to remove the rename if we can agree take is the better name.

Ping from triage. @SimonSapin any updates on this? Thanks.

can anyone from @rust-lang/libs review this?