Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the documentation for ManuallyDrop to resolve conflicting usage of terminology #71625

Merged
merged 1 commit into from
May 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions src/libcore/mem/manually_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ use crate::ptr;
///
/// `ManuallyDrop<T>` is subject to the same layout optimizations as `T`.
/// As a consequence, it has *no effect* on the assumptions that the compiler makes
/// about all values being initialized at their type. In particular, initializing
/// a `ManuallyDrop<&mut T>` with [`mem::zeroed`] is undefined behavior.
/// about its contents. For example, initializing a `ManuallyDrop<&mut T>`
/// with [`mem::zeroed`] is undefined behavior.
/// If you need to handle uninitialized data, use [`MaybeUninit<T>`] instead.
///
/// # Examples
///
/// This wrapper helps with explicitly documenting the drop order dependencies between fields of
/// the type:
/// This wrapper can be used to enforce a particular drop order on fields, regardless
/// of how they are defined in the struct:
///
/// ```rust
/// use std::mem::ManuallyDrop;
Expand Down Expand Up @@ -43,8 +43,18 @@ use crate::ptr;
/// }
/// ```
///
/// However, care should be taken when using this pattern as it can lead to *leak amplification*.
/// In this example, if the `Drop` implementation for `Peach` were to panic, the `banana` field
/// would also be leaked.
///
/// In contrast, the automatically-generated compiler drop implementation would have ensured
/// that all fields are dropped even in the presence of panics. This is especially important when
/// working with [pinned] data, where reusing the memory without calling the destructor could lead
/// to Undefined Behaviour.
///
/// [`mem::zeroed`]: fn.zeroed.html
/// [`MaybeUninit<T>`]: union.MaybeUninit.html
/// [pinned]: ../pin/index.html
#[stable(feature = "manually_drop", since = "1.20.0")]
#[lang = "manually_drop"]
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand Down Expand Up @@ -113,19 +123,28 @@ impl<T> ManuallyDrop<T> {
}

impl<T: ?Sized> ManuallyDrop<T> {
/// Manually drops the contained value.
/// Manually drops the contained value. This is exactly equivalent to calling
/// [`ptr::drop_in_place`] with a pointer to the contained value. As such, unless
/// the contained value is a packed struct, the destructor will be called in-place
/// without moving the value, and thus can be used to safely drop [pinned] data.
///
/// If you have ownership of the value, you can use [`ManuallyDrop::into_inner`] instead.
///
/// # Safety
///
/// This function runs the destructor of the contained value and thus the wrapped value
/// now represents uninitialized data. It is up to the user of this method to ensure the
/// uninitialized data is not actually used.
/// In particular, this function can only be called at most once
/// for a given instance of `ManuallyDrop<T>`.
/// This function runs the destructor of the contained value. Other than changes made by
/// the destructor itself, the memory is left unchanged, and so as far as the compiler is
/// concerned still holds a bit-pattern which is valid for the type `T`.
///
/// However, this "zombie" value should not be exposed to safe code, and this function
/// should not be called more than once. To use a value after it's been dropped, or drop
/// a value multiple times, can cause Undefined Behavior (depending on what `drop` does).
/// This is normally prevented by the type system, but users of `ManuallyDrop` must
/// uphold those guarantees without assistance from the compiler.
Diggsey marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`ManuallyDrop::into_inner`]: #method.into_inner
/// [`ptr::drop_in_place`]: ../ptr/fn.drop_in_place.html
/// [pinned]: ../pin/index.html
#[stable(feature = "manually_drop", since = "1.20.0")]
#[inline]
pub unsafe fn drop(slot: &mut ManuallyDrop<T>) {
Expand Down
8 changes: 7 additions & 1 deletion src/libcore/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,17 @@ mod mut_ptr;
/// as the compiler doesn't need to prove that it's sound to elide the
/// copy.
///
/// * It can be used to drop [pinned] data when `T` is not `repr(packed)`
/// (pinned data must not be moved before it is dropped).
///
/// Unaligned values cannot be dropped in place, they must be copied to an aligned
/// location first using [`ptr::read_unaligned`].
/// location first using [`ptr::read_unaligned`]. For packed structs, this move is
/// done automatically by the compiler. This means the fields of packed structs
/// are not dropped in-place.
///
/// [`ptr::read`]: ../ptr/fn.read.html
/// [`ptr::read_unaligned`]: ../ptr/fn.read_unaligned.html
/// [pinned]: ../pin/index.html
///
/// # Safety
///
Expand Down