Skip to content

Commit

Permalink
Auto merge of #103413 - RalfJung:phantom-dropck, r=lcnr
Browse files Browse the repository at this point in the history
PhantomData: fix documentation wrt interaction with dropck

As far as I could find out, the `PhantomData`-dropck interaction *only* affects code using `may_dangle`. The documentation in the standard library has not been updated for 8 years and thus stems from a time when Rust still used "parametric dropck", before [RFC 1238](https://rust-lang.github.io/rfcs/1238-nonparametric-dropck.html). Back then what the docs said was correct, but with `may_dangle` dropck it stopped being entirely accurate and these days, with NLL, it is actively misleading.

Fixes rust-lang/rust#102810
Fixes rust-lang/rust#70841
Cc `@nikomatsakis` I hope what I am saying here is right.^^
  • Loading branch information
bors committed May 13, 2023
2 parents 83b7d71 + 248c6c4 commit 971ef73
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 10 deletions.
15 changes: 5 additions & 10 deletions core/src/marker.rs
Expand Up @@ -766,24 +766,19 @@ impl<T: ?Sized> !Sync for *mut T {}
///
/// ## Ownership and the drop check
///
/// Adding a field of type `PhantomData<T>` indicates that your
/// type owns data of type `T`. This in turn implies that when your
/// type is dropped, it may drop one or more instances of the type
/// `T`. This has bearing on the Rust compiler's [drop check]
/// analysis.
/// The exact interaction of `PhantomData` with drop check **may change in the future**.
///
/// If your struct does not in fact *own* the data of type `T`, it is
/// better to use a reference type, like `PhantomData<&'a T>`
/// (ideally) or `PhantomData<*const T>` (if no lifetime applies), so
/// as not to indicate ownership.
/// Currently, adding a field of type `PhantomData<T>` indicates that your type *owns* data of type
/// `T` in very rare circumstances. This in turn has effects on the Rust compiler's [drop check]
/// analysis. For the exact rules, see the [drop check] documentation.
///
/// ## Layout
///
/// For all `T`, the following are guaranteed:
/// * `size_of::<PhantomData<T>>() == 0`
/// * `align_of::<PhantomData<T>>() == 1`
///
/// [drop check]: ../../nomicon/dropck.html
/// [drop check]: Drop#drop-check
#[lang = "phantom_data"]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct PhantomData<T: ?Sized>;
Expand Down
68 changes: 68 additions & 0 deletions core/src/ops/drop.rs
Expand Up @@ -132,6 +132,74 @@
/// are `Copy` get implicitly duplicated by the compiler, making it very
/// hard to predict when, and how often destructors will be executed. As such,
/// these types cannot have destructors.
///
/// ## Drop check
///
/// Dropping interacts with the borrow checker in subtle ways: when a type `T` is being implicitly
/// dropped as some variable of this type goes out of scope, the borrow checker needs to ensure that
/// calling `T`'s destructor at this moment is safe. In particular, it also needs to be safe to
/// recursively drop all the fields of `T`. For example, it is crucial that code like the following
/// is being rejected:
///
/// ```compile_fail,E0597
/// use std::cell::Cell;
///
/// struct S<'a>(Cell<Option<&'a S<'a>>>, Box<i32>);
/// impl Drop for S<'_> {
/// fn drop(&mut self) {
/// if let Some(r) = self.0.get() {
/// // Print the contents of the `Box` in `r`.
/// println!("{}", r.1);
/// }
/// }
/// }
///
/// fn main() {
/// // Set up two `S` that point to each other.
/// let s1 = S(Cell::new(None), Box::new(42));
/// let s2 = S(Cell::new(Some(&s1)), Box::new(42));
/// s1.0.set(Some(&s2));
/// // Now they both get dropped. But whichever is the 2nd one
/// // to be dropped will access the `Box` in the first one,
/// // which is a use-after-free!
/// }
/// ```
///
/// The Nomicon discusses the need for [drop check in more detail][drop check].
///
/// To reject such code, the "drop check" analysis determines which types and lifetimes need to
/// still be live when `T` gets dropped. The exact details of this analysis are not yet
/// stably guaranteed and **subject to change**. Currently, the analysis works as follows:
/// - If `T` has no drop glue, then trivially nothing is required to be live. This is the case if
/// neither `T` nor any of its (recursive) fields have a destructor (`impl Drop`). [`PhantomData`]
/// and [`ManuallyDrop`] are considered to never have a destructor, no matter their field type.
/// - If `T` has drop glue, then, for all types `U` that are *owned* by any field of `T`,
/// recursively add the types and lifetimes that need to be live when `U` gets dropped. The set of
/// owned types is determined by recursively traversing `T`:
/// - Recursively descend through `PhantomData`, `Box`, tuples, and arrays (including arrays of
/// length 0).
/// - Stop at reference and raw pointer types as well as function pointers and function items;
/// they do not own anything.
/// - Stop at non-composite types (type parameters that remain generic in the current context and
/// base types such as integers and `bool`); these types are owned.
/// - When hitting an ADT with `impl Drop`, stop there; this type is owned.
/// - When hitting an ADT without `impl Drop`, recursively descend to its fields. (For an `enum`,
/// consider all fields of all variants.)
/// - Furthermore, if `T` implements `Drop`, then all generic (lifetime and type) parameters of `T`
/// must be live.
///
/// In the above example, the last clause implies that `'a` must be live when `S<'a>` is dropped,
/// and hence the example is rejected. If we remove the `impl Drop`, the liveness requirement
/// disappears and the example is accepted.
///
/// There exists an unstable way for a type to opt-out of the last clause; this is called "drop
/// check eyepatch" or `may_dangle`. For more details on this nightly-only feature, see the
/// [discussion in the Nomicon][nomicon].
///
/// [`ManuallyDrop`]: crate::mem::ManuallyDrop
/// [`PhantomData`]: crate::marker::PhantomData
/// [drop check]: ../../nomicon/dropck.html
/// [nomicon]: ../../nomicon/phantom-data.html#an-exception-the-special-case-of-the-standard-library-and-its-unstable-may_dangle
#[lang = "drop"]
#[stable(feature = "rust1", since = "1.0.0")]
#[const_trait]
Expand Down

0 comments on commit 971ef73

Please sign in to comment.