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.

Sign up for GitHub

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

Add note about PhantomData and dropck #432

Merged
merged 9 commits into from
Sep 7, 2020
Merged

Add note about PhantomData and dropck #432

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4fd1f93
add note about PhantomData and dropck
KodrAus Sep 7, 2020
7861c36
Update maintaining-std.md
KodrAus Sep 7, 2020
b3b2e92
rework may_dangle guidance
KodrAus Sep 7, 2020
3ac1ab3
Update src/libs/maintaining-std.md
KodrAus Sep 7, 2020
fc26bba
Update src/libs/maintaining-std.md
KodrAus Sep 7, 2020
bbdeb5f
clarify may_dangle docs
KodrAus Sep 7, 2020
68fdc6c
inline `take_inner` impl
KodrAus Sep 7, 2020
f85d006
Update src/libs/maintaining-std.md
KodrAus Sep 7, 2020
d81ddc8
clarify safety comments in `Drop` impl
KodrAus Sep 7, 2020
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
18 changes: 18 additions & 0 deletions src/libs/maintaining-std.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,23 @@ Changes to collection internals may affect the order their items are dropped in.

Generic types that manually implement `Drop` should consider whether a `#[may_dangle]` attribute is appropriate. The [Nomicon][dropck] has some details on what `#[may_dangle]` is all about.

If the type pretends to store a value that may dangle, but doesn't do so directly (perhaps through `MaybeUninit<T>` or a raw pointer) then make sure there's an appropriate use of `PhantomData` to support dropck. As a [real-world example][rust/issues/76367], adding a `#[may_dangle]` attribute to an `OptionCell<T>` that internally stores its value as a `MaybeUninit<T>` requires both a `PhantomData` marker and a `#[may_dangle]` attribute:
Copy link
Member

Choose a reason for hiding this comment

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

When reading this paragraph, I wouldn't know what "pretends to store a value that may dangle" means -- specifically the "may dangle" part.

The key point here is that if dropping Type<T> means that a T could be dropped, then dropck needs to know this. If the drop occurs through the normal auto-generated drop glue, no action is required as dropck know what that one does. But if the drop occurs through a manual drop impl, then there needs to be a PhantomData<T>.


```diff
struct OptionCell<T> {
is_init: bool,
value: MaybeUninit<T>,
+ _marker: PhantomData<T>,
}

- impl Drop<T> OptionCell<T> {
+ impl Drop<#[may_dangle] T> OptionCell<T> {
..
}
```

Types in the standard library that use the internal `Unique<T>` don't need a `PhantomData` marker field. That's taken care of for them by `Unique<T>`.

### How could `mem` break assumptions?

#### `mem::replace` and `mem::swap`
Expand Down Expand Up @@ -262,5 +279,6 @@ Where `unsafe` and `const` is involved, e.g., for operations which are "unconst"
[RFC 1105]: https://rust-lang.github.io/rfcs/1105-api-evolution.html
[Everyone Poops]: http://cglab.ca/~abeinges/blah/everyone-poops
[rust/pull/46799]: https://github.com/rust-lang/rust/pull/46799
[rust/issues/76367]: https://github.com/rust-lang/rust/issues/76367
[hashbrown/pull/119]: https://github.com/rust-lang/hashbrown/pull/119
[rollup guidelines]: ../compiler/reviews.md#rollups