Skip to content

Rework drop check stuff, it's probably outdated #17

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

Open
Gankra opened this issue May 4, 2017 · 7 comments
Open

Rework drop check stuff, it's probably outdated #17

Gankra opened this issue May 4, 2017 · 7 comments

Comments

@Gankra
Copy link
Contributor

Gankra commented May 4, 2017

I don't really understand or care about the new rules (they mostly just plug super-degenerate-you'll-probably-never-see-this holes in the language, afaict), but they're different and any discussion of dropcheck is probably wrong now.

CC @pnkfelix

@Emerentius
Copy link

One thing I didn't understand in the nomicon, is how dropck interacts with PhantomData. One section says for a hypothetical Vec without PhantomData<T>

The drop checker will generously determine that Vec does not own any values of type T. This will in turn make it conclude that it doesn't need to worry about Vec dropping any T's in its destructor for determining drop check soundness. This will in turn allow people to create unsoundness using Vec's destructor.

There's no example for how this could be done and I couldn't construct one myself.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2018

A couple years ago I posted an answer to StackOverflow about this topic: https://stackoverflow.com/a/42720413/36585

But I didn't actually make a concrete test case to illustrate the point, and its possible that with all the changes that have happened over the years, the argument no longer applies.

I should probably at least try to make the test again.

@spunit262
Copy link

An unsound example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=ad0487c46e34d0075444769f06ac3721

Adding PhantomData<T> or removing #[may_dangle] fixes the unsoundness.

@pnkfelix
Copy link
Member

(Ideally we'd have an example that illustrates a case where, for correctness i.e. API completeness, you cannot remove the #[may_dangle] and thus are forced to use PhantomData<T> to keep things sound.)

@RalfJung
Copy link
Member

Corresponding reference issue: rust-lang/reference#303

@RalfJung
Copy link
Member

Adding PhantomData or removing #[may_dangle] fixes the unsoundness.

So, is it the case that PhantomData is never required for dropck purposes unless one uses #[may_dangle]?

@Gankra
Copy link
Contributor Author

Gankra commented Jan 18, 2021

That sounds vaguely accurate, but this is all way too far out of cache

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

No branches or pull requests

5 participants