Skip to content

Commit

Permalink
Rollup merge of rust-lang#76150 - matklad:droporder, r=withoutboats
Browse files Browse the repository at this point in the history
Don't recommend ManuallyDrop to customize drop order

See
https://internals.rust-lang.org/t/need-for-controlling-drop-order-of-fields/12914/21
for the discussion.

TL;DR: ManuallyDrop is unsafe and footguny, but you can just ask the compiler to do all the work for you by re-ordering declarations.

Specifically, the original example from the docs is much better written as

```rust
struct Peach;
struct Banana;
struct Melon;
struct FruitBox {
    melon: Melon,
    // XXX: mind the relative drop order of the fields below
    peach: Peach,
    banana: Banana,
}
```
  • Loading branch information
Dylan-DPC authored Sep 22, 2020
2 parents 6be8212 + 60b102d commit e9aaa3b
Showing 1 changed file with 19 additions and 36 deletions.
55 changes: 19 additions & 36 deletions library/core/src/mem/manually_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,50 +15,33 @@ use crate::ptr;
/// be exposed through a public safe API.
/// Correspondingly, `ManuallyDrop::drop` is unsafe.
///
/// # Examples
/// # `ManuallyDrop` and drop order.
///
/// This wrapper can be used to enforce a particular drop order on fields, regardless
/// of how they are defined in the struct:
/// Rust has a well-defined [drop order] of values. To make sure that fields or
/// locals are dropped in a specific order, reorder the declarations such that
/// the implicit drop order is the correct one.
///
/// ```rust
/// use std::mem::ManuallyDrop;
/// struct Peach;
/// struct Banana;
/// struct Melon;
/// struct FruitBox {
/// // Immediately clear there’s something non-trivial going on with these fields.
/// peach: ManuallyDrop<Peach>,
/// melon: Melon, // Field that’s independent of the other two.
/// banana: ManuallyDrop<Banana>,
/// }
/// It is possible to use `ManuallyDrop` to control the drop order, but this
/// requires unsafe code and is hard to do correctly in the presence of
/// unwinding.
///
/// impl Drop for FruitBox {
/// fn drop(&mut self) {
/// unsafe {
/// // Explicit ordering in which field destructors are run specified in the intuitive
/// // location – the destructor of the structure containing the fields.
/// // Moreover, one can now reorder fields within the struct however much they want.
/// ManuallyDrop::drop(&mut self.peach);
/// ManuallyDrop::drop(&mut self.banana);
/// }
/// // After destructor for `FruitBox` runs (this function), the destructor for Melon gets
/// // invoked in the usual manner, as it is not wrapped in `ManuallyDrop`.
/// }
/// }
/// ```
/// For example, if you want to make sure that a specific field is dropped after
/// the others, make it the last field of a struct:
///
/// 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.
/// ```
/// struct Context;
///
/// 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.
/// struct Widget {
/// children: Vec<Widget>,
/// // `context` will be dropped after `children`.
/// // Rust guarantees that fields are dropped in the order of declaration.
/// context: Context,
/// }
/// ```
///
/// [drop order]: https://doc.rust-lang.org/reference/destructors.html
/// [`mem::zeroed`]: crate::mem::zeroed
/// [`MaybeUninit<T>`]: crate::mem::MaybeUninit
/// [pinned]: crate::pin
#[stable(feature = "manually_drop", since = "1.20.0")]
#[lang = "manually_drop"]
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand Down

0 comments on commit e9aaa3b

Please sign in to comment.