Repository navigation
Tracking issue for ManuallyDrop::take #55422
Description
Activity
- added a commit that references this issue
on Oct 28, 2018 I am a bit worried about the name of this method being too innocent-looking. We have
takeas a safe method in other places, likeOptionandIterator. This leaves the fact that the method isunsafeas the only safeguard -- which is a problem e.g. when this gets (accidentally) used inside anunsafe fnor in a largeunsafeblock.I'd prefer a name that already indicates this to be a dangerous operation. Strawman:
take_uncheckedbecause it doesn't check you are taking twice?Note that it has to be written as
ManuallyDrop::take, which eliminates some of the risk of using a familiar name, as it's attached toManuallyDrop, which is known to have interesting semantics, and thus is hard to use accidentally.(Procedural note: I'll update OP with useful information soon.)
@RalfJung I’m sympathetic to this argument, but we also have
ManuallyDrop::dropwhich is (almost) as innocent-looking, and also doesn’t check if we are dropping twice.(I also notice that
ManuallyDrop::into_inneris safe, even though it would be unsound to call afterdroportake.)So I’m inclined to FCP to stabilize as-is. Or do you feel strongly we should rename?
No, not very strongly. And not being able to say
foo.take()helps a lot.Anything that needs to be resolved for this to stabilize, libs?
I just ended up here because someone on discord was just using
mem::replace(x, mem::uninitialized())(😭) where hopefully they would have just used this if it were stable.The corresponding (unstable) method on
MaybeUninitis (currently) called "read", because the safety concerns are a lot like the ones ofptr::read. Would the same make sense here as well?Reacted by scottmcmThis sounds reasonable. In fact both methods are implemented based on
ptr::read. We can repeat it again in the docs:Like
std::ptr::read, this function semantically moves […]@rust-lang/libs, what do you think?
Reacted by scottmcm and Crystal DurhamI think this method is only reasonable only if we don't have
MaybeUninitin stable. Practically we useManuallyDropwhen we need untaggedOption, because we don't have any other way to prevent accidentally dropping invalid data. But now we haveMaybeUninitin stable which exactly is for this purpose.Is there some use case that can't be reasonably expressed using
MaybeUninit?11 remaining items
IMO, anyone who sees
ManuallyDropandunsafealready knows there's something fishy going on, I don't think it needs to be reinforced by_unchecked.Team member @Amanieu 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!
See this document for info about what commands tagged team members can give me.
🔔 This is now entering its final comment period, as per the review above. 🔔
Reacted by jyn and Crystal DurhamThe 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.
Reacted by jyn and Crystal Durham- added a commit that references this issue
on Jan 20, 2020
This is a tracking issue for the function
ManuallyDrop::take, gated on the featuremanually_drop_take.Steps:
Unresolved questions:
ManuallyDrop::take#55422 (comment))&Selfor&mut Self? (Tracking issue forManuallyDrop::take#55422 (comment))