PhantomData: fix documentation wrt interaction with dropck

[RalfJung]

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. 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 #102810
Fixes #70841
Cc @nikomatsakis I hope what I am saying here is right.^^

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

[RalfJung]

Looks like this was brought up more than 2 years ago already, in #70841. @pnkfelix pointed at the problematic docs paragraph that this PR is changing, and also raised the concern that

I do worry a little bit about the user's mental model for this case, however. The claim "PhantomData<T> is just like a T" doesn't quite hold up. (not that it ever did ...)

[RalfJung]

I think this PR closes #70841. The main question that remains is about subtle types of the form I am sketching here. I should probably try actually coming up with that weird example...

[mu001999]

It seems that the PhantomData-dropck interaction not only affects code using #[may_dangle]. Considering the following demo:

use std::marker::PhantomData;

struct Foo<T>(Box<i32>, PhantomData<T>);

struct Bar<T>(T);
impl<T> Drop for Bar<T> {
    fn drop(&mut self) {}
}

fn main() {
    let _foo;
    let s = String::from("Hello, world!");
    _foo = helper(&*s);
}

fn helper<'a>(_: &'a str) -> Foo<Bar<&'a str>> {
    Foo(Box::new(1), PhantomData)
}

Without impl<T> Drop for Foo<T> and #[may_dangle], the PhantomData introduces the dropck for Bar<&'a str>.

[SoniEx2]

our proposed MCP for dropck bounds might be useful to explain the PhantomData behaviour.

tho with our MCP's flexibility, we'd argue this should be changed in the future. so, don't rely on it.

(but hey, it is only an experiment so far)

[RalfJung]

@mu001999 Interesting example... I first thought this was not dropck, but this compiles, indicating that it is indeed dropck.

Looks like we need a research project to even figure out our current dropck rules...

[TOETOE55]

I outlined the current drop check rules for myself:

1. Drop check occurs only on "need drop" variables in the current scope when they are dropping.

2. Denote a dropping variable t and its type T, in the scope of 't.

   dropck(T, 't) :=

   1. if T isn't a "needs drop" type expect T == PhantomData, skip the drop check
   2. if T == PhantomData<P>, check dropck(P, 't)
   3. if T is dyn trait or impl trait, check T: 't
   4. if T is ADT with generic params <..., P, ...>,(P could be lifetime or type)
      1. if T impl Drop, check P: 't, unless P is marked as #[may_dangle]
      2. drop check for all fields of T recursively.

NB: all T: 't in drop check means "strictly outlive".

---

For the first situation , PhantomData<T>, ManuallyDrop<T> or Pending<T> variables never "need drop", so they won't do the drop check.

[danielhenrymantilla]

From all we've seen, I've been able to gather the following understanding of the dropck behavior.

Given a Wrapper<T> (which could be a struct Wrapper<T>, or any T-dependent type (e.g., (String, T)).

To illustrate, consider Wrapper<T> = Box<T>, and T being &'dangling_in_drop str or DisplayOnDrop<&'dangling_in_drop str>

Code example

struct Box<T> {
    ptr: *const T,
    // … (e.g., should we `PhantomData<T>` or not)
}

impl<T> Drop for Box<T> { /* or:
unsafe impl<#[may_dangle] T> Drop for Box<T> { */
    fn drop(&mut self) {
        unsafe { ptr::drop_in_place::<T>(self.ptr); } // no-op if `T` has no drop glue (so `T` could dangle).
        unsafe { alloc::dealloc(self.ptr, …); } // does not use values of type `T`.
    }
}

/* e.g.
impl<'dangling_in_drop> for Box<&'dangling_in_drop str> {
// or
unsafe impl<#[may_dangle] 'dangling_in_drop> for Box<&'dangling_in_drop str> {

// as well as:
impl<'dangling_in_drop> for Box<DisplayOnDrop<&'dangling_in_drop str>> {
// or
unsafe impl<#[may_dangle] 'dangling_in_drop> for Box<DisplayOnDrop<&'dangling_in_drop str>> {
*/

// where
struct DisplayOnDrop<T : Display>(T);
impl<T : Display> Drop for DisplayOnDrop<T> {
    fn drop(&mut self) {
        println!("{}", self.0);
    }
}

fn main() {
    let (b1, b2);
    {
        let dropped_before_the_boxes = String::from("…");
        let thus_dangling_in_drop = &*dropped_before_the_boxes;

        b1 = Box::new(thus_dangling_in_drop);
        b2 = Box::new(DisplayOnDrop(thus_dangling_in_drop));
     } // `drop(dropped_before_the_boxes)`
     /* `thus_dangling_in_drop` is dangling here! */
} // `drop((b1, b2))`

1. Ideally, we'd want b1 to be accepted since drop_in_place::<&str>() does nothing and thus &str being a &'dangling str is fine.

2. For soundness, we need b2 to be rejected since drop_in_place::<DisplayOnDrop<&str>>() displays the given string, so it must not dangle.

The practical TL,DR with drop and PhantomData is that:

• impl<T> Drop for Box<T> { achieves 2., but at the cost of breaking 1.;
• unsafe impl<#[may_dangle] T> for Box<T> {, alone, achieves 1. but at the cost of breaking 2. (unsound!);
• unsafe impl<#[may_dangle] T> for Box<T> {, coupled with a "T-owning field" (even a PhantomData<T> one) in Box, preserves 1. while bringing back 2., and thus, soundness.

But there are other edge cases which may require a more detailed exposition of the rules at play, here:

---

• When does strict dropck (assume any use of T in drop) apply

  A.k.a., should T : 'drop_point have to hold? (this rejects both T = &'dangling str and T = DisplayOnDrop<&'dangling str>).

  This occurs when:

  • impl<T> Drop for …
  • type Wrapper<T> = dyn Bounds… or type Wrapper<T> = impl Bounds… (except when Bounds : Copy), since type erasure (be it dyn or abstract/impl) means the user of that type no longer knows whether there may be drop glue¹

  Since this property is more restrictive than dropck(T, 'drop_point); the following dropck considerations do not matter in practice when we are in this case.

• When does precise dropck (only use of T in drop is drop_in_place::<T>) apply

  A.k.a. should dropck(T, 'drop_point) have to hold? (this rejects T = DisplayOnDrop<&'dangling str> only, allowing T = &'dangling str).

  This occurs when both:

  • Wrapper<T> may have drop glue;

    • which happens when:
      • either unsafe impl<#[may_dangle] T> for Wrapper<T> { (or technically just impl<T> Drop, but as mentioned this does not matter / is not observable);
      • or when one of the fields of Wrapper<T> (which does not have to depend on T) has drop glue.
    • which is known not to happen when:
      • Wrapper<T> : Copy (e.g., Wrapper<T> = PhantomData<T>; this would be PhantomData<T> no longer dropck? #70841 and PhantomData behavior is very surprising w.r.t. lack of Drop implications #102810)
      • Wrapper<T> = ManuallyDrop<…>;

  • and T is structurally present in an owned fashion (e.g., T, PhantomData<T>, ManuallyDrop<T>) in Wrapper<T>. EDIT: ManuallyDrop<T> does not count as owning T ⚠️

• These properties become structural, unless we run into any of these "root rules" at any layer.

Footnotes

1. it could be nice to have a generalization of this Copy opt-out, since this caveat often makes existential types (such as -> impl Iterator) less ergonomic than their named counterparts (struct Iter<'slice, T>). ↩

[SoniEx2]

nope, ManuallyDrop doesn't have drop glue, while PhantomData does. see also #103318 (comment)

[danielhenrymantilla]

Ah, interesting, let me update the post. It does mean that "PhantomData<T> acts as ManuallyDrop<T>" (which I saw somewhere) is not accurate either 🥴

[SoniEx2]

we think it's best to describe these things in terms of bounds. it makes it really intuitive that way. here's hoping we get our MCP through and then we have bounds-based dropck.

and then we should no longer need "PhantomData is relevant for Drop" at all

[mu001999]

I outlined the current drop check rules for myself:

1. Drop check occurs **only** on  "need drop" variables in the current scope when they are dropping.

2. Denote a dropping variable `t` and its type `T`, in the scope of `'t`.
   `dropck(T, 't)` :=
   
   1. if `T` isn't a "needs drop" type expect `T == PhantomData`, skip the drop check
   2. if `T == PhantomData<P>`,  check `dropck(P, 't)`
   3. if `T` is `dyn trait` or `impl trait`, check `T: 't`
   4. if `T` is ADT with generic params `<..., P, ...>`,(`P` could be lifetime or type)
      
      1. if `T` impl `Drop`, check `P: 't`, unless `P` is marked as `#[may_dangle]`
      2. drop check for all fields of `T` recursively.

NB: all T: 't in drop check means "strictly outlive".

PhantomData<T>, ManuallyDrop<T> or Pending<T> variables never "need drop", so they won't do the drop check

These rules are outlined very clearly!

With these rules, it can be said that PhantomData<P> assumes the field(data) of type P is dropped if T "needs drop", impl<..., P, ...> Drop for T<..., P, ...> assumes data of type P is accessed in the T::drop, and #[may_dangle] P assumes data of type P won't be accessed in the T::drop.

In turn, PhantomData<P> introduces dropck for P and means T "owns" the data of type P if and only if T "needs drop", impl<..., P, ...> Drop for T<..., P, ...> requires P: 't, and #[may_dangle] P discards the requirement P: 't.

Thus, PhantomData<P> only cares if T "needs drop" and does not care about if #[may_dangle] P or not. #[may_dangle] P + PhantomData<P> only means that T::drop does not accesses data of type P but drops data of type P, e.g., T = Vec<P>, PhantomData<P> works fine according to the previous rules.

[TOETOE55]

This is the “needs drop” what I said above.
https://doc.rust-lang.org/stable/std/mem/fn.needs_drop.html

[TOETOE55]

I’m still curious about this question, #70841 (comment)

If the following MyBox unsound or not:

struct MyBox<T>(NonNull<T>);

impl<T> Drop for MyBox<T> { … }

From my understanding of dropck, considering MyBox<T> impl no #[may_dangle] Drop:

If it construct

• without PhantomData<T>, the dropck would be:

  dropck(MyBox<T>, 'scope) := T: 'scope (strictly)

• with PhantomData<T>:

  dropck(MyBox<T>, 'scope) := T: 'scope & dropck(T, 'scope) where dropck(PhantomData<T>, 'scope) := dropck(T, 'scope)

If you want to construct a ub from the former, it is equivalent to finding a T that satisfies T: 'scope but not dropck(T, 'scope), and causes ub when it destructs.

But I tried to construct such a T and failed. I can't even find a counterexample for T:' scope - > dropck(T, 'scope).

IMO, generic params in Drop impl/#[may_dangle] attribute/PhantomData<T> play a different role in drop checking:

1. the normal generic param T in Drop impl, means that “I may touch T in drop”
2. the generic param T marked as #[may_dangle], means that “I won’t touch T in drop”
3. T wrapping in PhantomData from the field of a dropping value, means that “It may call drop_in_place::<T>() while the value is dropping”.

I think there’s a subtle difference between 1 and 3.(maybe 3 is weaker than 1, I’m not sure). We usually mark T as may_dangle and add a PhantomData<T> in our structure when implementing Drop.(i.e. Vec<T>).

[mu001999]

@TOETOE55T From my understanding, T: 't is stricter than dropck(T, 't), because the latter requires recursively at most all generic params of T outlives 't. This is also why we may need PhantomData if we have used #[may_dangle].

[TOETOE55]

I just glanced at NLL's RFC, layer-3-accommodating-dropck section https://rust-lang.github.io/rfcs/2094-nll.html#layer-3-accommodating-dropck

This operation executes the destructor for variable, effectively "de-initializing" the memory in which the value resides (if the variable -- or parts of the variable -- have already been dropped, then drop has no effect; this is not relevant to the current analysis)

But the status quo still do the dropck on variables are dropped: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=936fe411cc2d2eb720223c0fdc74f8f2

use std::marker::PhantomData;

struct Foo<T>(Box<i32>, PhantomData<T>);

struct Bar<T>(T);
impl<T> Drop for Bar<T> {
    fn drop(&mut self) {}
}

fn main() {
    let _foo;
    let s = String::from("Hello, world!");
    _foo = helper(&*s);
    std::mem::drop(_foo); // or `drop(_foo.0)
}

fn helper<'a>(_: &'a str) -> Foo<Bar<&'a str>> {
    Foo(Box::new(1), PhantomData)
}

[nikomatsakis]

I think the correct rules are:

• If the value's type has drop glue (needs proper definition) and a generic parameter T:
  • It is considered to drop a value of type T if...
    • the struct has a field which owns a T
    • there is phantom-data of type T
    • or the struct implements Drop
• Note that phantom-data by itself does not have drop-glue, so when dropped it is a no-op to borrow check

Another way to put it:

• The behavior of the DROP GLUE knows what gets dropped
  • but it also considers PhantomData values to be dropped

But I've not run any experiments, so maybe I'm missing something. Have to pull back in the examples we tried last time.

Per these rules:

• PhantomData<T> is not considered to drop T, it has no drop-glue
• a struct Foo<T> { d: T, e: String } is considered to drop T (because String has drop-glue)
• and so is struct Foo<T> { d: PhantomData<T>, e: String }
• but struct Foo<'a, T> { d: &'a T, e: String } is not (I think)
• not sure about struct Foo<T> { d: *mut T, e: String } ..?

If a struct actually implements Drop, then it is always assumed to drop all the things (modulo the unstable feature). So if there is a impl Drop for Foo for any such examples, it would always be assumed to drop T.

[SoniEx2]

we should probably have an RFC for what to do here.

[RalfJung]

We don't need an RFC to document the existing analysis. We just need an RFC to change it.

[SoniEx2]

nod

we should probably have an RFC to remove the existing spooky action at a distance instead of documenting it.

[lcnr]

@rfcbot concern resolve how we handle [T; 0] wrt to dropck

@rfcbot concern want to take another look at rust-lang/rfcs#3390 whether it changes anything about my reasoning here

[lcnr]

@rfcbot resolved want to take another look at rust-lang/rfcs#3390 whether it changes anything about my reasoning here

rust-lang/rfcs#3390 (comment) still summarizes my feelings about this PR even after feeling like I now have a good understanding of dropck: "Spooky-dropck-at-a-distance" does not feel spooky anymore. It just feels unfortunate. Even if accepted, to my knowledge the RFC is not able to resolve the PhantomData behavior of having outlives constraints but not requiring drop itself 🤔

"having outlives constraints" is necessary for the sound usage of PhantomData.

We could change PhantomData back to requiring noop drop glue but that will also be inconsistent as it is Copy so it would lose the "needs to be live at drop" when used in generic functions. I don't think we want PhantomData to be Copy while having a liveness requirement when going out of scope is something we want.

I guess a potential alternative is to make PhantomData even less impactful wrt to drop and actually only adding outlives requirements when used as a field of a type with a manual drop impl, i.e. PhantomData actually only impacts manual Drop impls when they have #[may_dangle]. I would like to explore that route more.

@SoniEx2, would you be available for a sync meeting about this topic in general?

@rustbot concern potential way to remove "Spooky-dropck-at-a-distance"

---

@rustbot resolve resolve how we handle [T; 0] wrt to dropck

opened #110288

[SoniEx2]

@lcnr

I guess a potential alternative is to make PhantomData even less impactful wrt to drop and actually only adding outlives requirements when used as a field of a type with a manual drop impl, i.e. PhantomData actually only impacts manual Drop impls when they have #[may_dangle]. I would like to explore that route more.

this is effectively what rust-lang/rfcs#3390 proposes - tho it tries to do so in a much more formalized way, basically defining some syntax and semantics and a whole system around it, as well as trying to explain how the current and desired behaviours fit into the proposed model (in other words: in a rather roundabout way). but for stable code, yeah, the observable result would be the same. maybe we should've leaned harder on spooky-dropck-at-a-distance as motivation, given that it (and being opposed to this very pull request) is what led us to write the RFC in the first place (indeed, the zulip thread is literally called "deprecating spooky-dropck-at-a-distance")... but anyway!

we can try a sync meeting? uh we've never really done of those tho? so we're not sure what to expect or how to prepare for it? >.<

[lcnr]

we can try a sync meeting? uh we've never really done of those tho? so we're not sure what to expect or how to prepare for it? >.<

wouldn't have needed to prepare anything for it 😀 mostly just wanted to have a quicker conversation because having conversations via github is a horrible experience in my opinion. going to instead use the existing zulip thread. https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/deprecating.20spooky-dropck-at-a-distance

[RalfJung]

There's an argument in rust-lang/rfcs#3417 that we shouldn't stably document this behavior, i.e. we shouldn't land this PR, because we want to change this behavior anyway. I guess alternatively we could add a preface saying that this is not guaranteed? That makes one wonder about the point of the docs though.

Either way I guess I won't spend time on this PR until that discussion is resolved. Seems like a waste of time if we're not going to document this anyway.

[lcnr]

we finally got to this issue in t-types triage: https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2023-05-08.20triage.20meeting/near/356728590

we intend to have a deep dive about the dropck rules and how to change them (rust-lang/types-team#92), but are open (some of us were in favor) of merging this documentation until then with the expectation that this does not block us from changing the subtle details of dropck later.

lgtm on the current documentation, but please add a note that this is not a stability guarantee. after that r=me

[lcnr]

please state somewhere that these exact rules may change in the future.

[RalfJung]

I made it clear that these rules are not stably guaranteed.

Looks like we can finally land this. Thanks a lot to everyone involved!
@bors r=lcnr

[bors]

📌 Commit b93fd83 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit b93fd83 with merge 9850584...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 9850584 to master...

[rust-timer]

Finished benchmarking commit (9850584): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.6%	[0.5%, 0.6%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	3.8%	[2.4%, 5.2%]	2
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.9%	[-3.0%, 1.3%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 659.199s -> 659.629s (0.07%)