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

PhantomData: fix documentation wrt interaction with dropck #103413

Merged
merged 3 commits into from
May 13, 2023
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
15 changes: 5 additions & 10 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,24 +668,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 library/core/src/ops/drop.rs
Original file line number Diff line number Diff line change
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`:
lcnr marked this conversation as resolved.
Show resolved Hide resolved
/// - 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.)
Comment on lines +186 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// - When hitting an ADT without `impl Drop`, recursively descend to its fields. (For an `enum`,
/// consider all fields of all variants.)
/// - When hitting an ADT without `impl Drop`, recursively descend to its fields except for `ManuallyDrop`.
/// (For an `enum`, consider all fields of all variants.)

or sth like that

/// - 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
22 changes: 22 additions & 0 deletions tests/ui/drop/dropck-eyepatch-manuallydrop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass
//! This test checks that dropck knows that ManuallyDrop does not drop its field.
#![feature(dropck_eyepatch)]

use std::mem::ManuallyDrop;

struct S<T>(ManuallyDrop<T>);

unsafe impl<#[may_dangle] T> Drop for S<T> {
fn drop(&mut self) {}
}

struct NonTrivialDrop<'a>(&'a str);
impl<'a> Drop for NonTrivialDrop<'a> {
fn drop(&mut self) {}
}

fn main() {
let s = String::from("string");
let _t = S(ManuallyDrop::new(NonTrivialDrop(&s)));
drop(s);
}