Skip to content

Deprecate PhantomData dropck#3331

Closed
SoniEx2 wants to merge 2 commits intorust-lang:masterfrom
SoniEx2:phantomdata
Closed

Deprecate PhantomData dropck#3331
SoniEx2 wants to merge 2 commits intorust-lang:masterfrom
SoniEx2:phantomdata

Conversation

@SoniEx2
Copy link
Copy Markdown

@SoniEx2 SoniEx2 commented Oct 19, 2022

Rendered

We had a lot of fun writing this and even learned some new things! =^-^=

Basically after a lot of thought we think this is the proper way to handle rust-lang/rust#102810

@RalfJung
Copy link
Copy Markdown
Member

Participation of PhantomData in dropck is absolutely crucial, as explained in the Nomicon. The RFC fails to explain how the standard library should implement Vec if PhantomData does not participate in dropck.

So I think this is a non-starter of a proposal.

(Also the 'rendered' link is broken.)

@SoniEx2
Copy link
Copy Markdown
Author

SoniEx2 commented Oct 20, 2022

we're convinced PhantomData isn't required for dropck today, except for variance and auto traits, specifically because of nonparametric drop.

but we'll be honest: we haven't actually tested it. we'd be extremely surprised if the following were somehow wrong tho:

/// A mostly-no-op wrapper around `T` which happens to be explicitly `Drop`.
struct Wrapper<T>(ManuallyDrop<T>);

unsafe impl<#[may_dangle] T> Drop for Wrapper<T> {
  fn drop(&mut self) {
    unsafe {
      ManuallyDrop::drop(&mut self.0);
    }
  }
}

edit: ah, looks like may_dangle is still parametric, so we're not quite ready to launch this RFC yet. (and yes, the above is broken, because ManuallyDrop is special and evades (parametric) dropck.)

@SoniEx2
Copy link
Copy Markdown
Author

SoniEx2 commented Oct 20, 2022

do we just

  1. replace this whole RFC with a refinement of may_dangle or
  2. keep it and also make a refinement of may_dangle or
  3. close it (and maybe make a refinement of may_dangle?)
  4. edit: alternatively, we could tighten the scope of this RFC, with the goal of only fixing non-Drop types with drop glue leak may_dangle implementation details on stable rust#103318, without the risk of altering the behaviour of may_dangle, in preparation for a future, separate refinement of may_dangle.

(do we want a refinement of may_dangle?)

@SoniEx2
Copy link
Copy Markdown
Author

SoniEx2 commented Oct 26, 2022

we're just gonna close this for now but we plan on revisiting it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants