Skip to content

Commit 049c25a

Browse files
committed
replace may dangle with 2 attributes
1 parent e66fda2 commit 049c25a

File tree

1 file changed

+234
-0
lines changed

1 file changed

+234
-0
lines changed

text/3417-dropck-eyepatch-v3.md

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
- Feature Name: (`dropck_eyepatch_v3`)
2+
- Start Date: (fill me in with today's date, YYYY-MM-DD)
3+
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000)
4+
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
5+
6+
# Summary
7+
[summary]: #summary
8+
9+
Cleanup the rules for implicit drops by splitting `#[may_dangle]` into two separate attributes:
10+
`#[only_dropped]` and `#[fully_ignored]`. Change `PhantomData` to get completely ignored
11+
by dropck as its current behavior is confusing and inconsistent.
12+
13+
# Motivation
14+
[motivation]: #motivation
15+
16+
The current rules around dropck and `#[may_dangle]` are confusing and have even resulted in
17+
unsoundness [multiple](https://github.com/rust-lang/rust/issues/76367)
18+
[times](https://github.com/rust-lang/rust/issues/99408). Even without `#[may_dangle]`,
19+
dropping `PhantomData` is currently quite weird as you get "spooky-dropck-at-a-distance":
20+
21+
```rust
22+
use std::marker::PhantomData;
23+
struct PrintOnDrop<'a>(&'a str); // requires `'a` to be live on drop.
24+
impl Drop for PrintOnDrop<'_> {
25+
fn drop(&mut self) {
26+
println!("{}", self.0)
27+
}
28+
}
29+
30+
fn assign<T>(_: T) -> PhantomData<T> {
31+
PhantomData
32+
}
33+
34+
// The type of `_x` is `AdtNoDrop<'not_live>` which doesn't have drop glue, OK
35+
struct AdtNoDrop<'a>(PhantomData<PrintOnDrop<'a>>, u32);
36+
fn phantom_data_adt_no_drop() {
37+
let _x;
38+
{
39+
let temp = String::from("temporary");
40+
_x = AdtNoDrop(assign(PrintOnDrop(&temp)), 0);
41+
}
42+
}
43+
44+
// The type of `_x` is `AdtNoDrop<'not_live>` which has drop glue, ERROR
45+
struct AdtNeedsDrop<'a>(PhantomData<PrintOnDrop<'a>>, String);
46+
fn phantom_data_adt_needs_drop() {
47+
let _x;
48+
{
49+
let temp = String::from("temporary");
50+
_x = AdtNeedsDrop(assign(PrintOnDrop(&temp)), String::new());
51+
}
52+
}
53+
```
54+
[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9ce9d368d2f13df9ddcbfaf9580721e0)
55+
56+
# Guide-level explanation
57+
[guide-level-explanation]: #guide-level-explanation
58+
59+
When a value goes out of scope the compiler adds drop glue for that value, recursively dropping it and all its fields.
60+
Dropping a type containing a lifetime which is no longer live is accepted if that lifetime is never accessed:
61+
```rust
62+
struct MyType<'s> {
63+
reference: &'s str,
64+
needs_drop: String,
65+
}
66+
fn can_drop_dead_reference() {
67+
let _x;
68+
{
69+
let temp = String::from("I am only temporary");
70+
_x = MyType {
71+
reference: &temp,
72+
needs_drop: String::from("I have to get dropped"),
73+
};
74+
}
75+
// We drop `_x` here even though `reference` is no longer live.
76+
//
77+
// This is fine as dropping a reference is a noop and does not
78+
// acess the pointee.
79+
}
80+
```
81+
The above example will however fail if we add a manual `Drop` impl as the compiler conservatively
82+
assumes that all generic parameters of the `Drop` impl are used:
83+
[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e604bcaecb7b2b4cf7fd0440faf165ac).
84+
85+
In case a manual `Drop` impl does not access a generic parameter, you can add
86+
`#[fully_unused]` or `#[only_dropped]` to that parameter. This **unsafely** asserts
87+
that the parameter is either completely unused when dropping your type or only
88+
recursively dropped.
89+
90+
```rust
91+
struct MyType<'s> {
92+
reference: &'s str,
93+
needs_drop: String,
94+
}
95+
// The impl has to be `unsafe` as the compiler does may not check
96+
// that `'s` is actually unused.
97+
unsafe impl<#[only_dropped] 's> Drop for MyType<'s> {
98+
fn drop(&mut self) {
99+
// println!("{}", reference); // this would be unsound
100+
println!("{}", needs_drop);
101+
}
102+
}
103+
fn can_drop_dead_reference() {
104+
let _x;
105+
{
106+
let temp = String::from("I am only temporary");
107+
_x = MyType {
108+
reference: &temp,
109+
needs_drop: String::from("I have to get dropped"),
110+
};
111+
}
112+
// We drop `_x` here even though `reference` is no longer live.
113+
//
114+
// This is accepted as `'s` is marked as `#[only_dropped]` in the
115+
// `Drop` impl of `MyType`.
116+
}
117+
```
118+
119+
The ability to differentiate between `#[fully_unused]` and `#[only_dropped]` is significant
120+
for type parameters:
121+
122+
```rust
123+
pub struct BTreeMap<K, V> {
124+
root: Option<Root<K, V>>,
125+
length: usize,
126+
}
127+
128+
unsafe impl<#[only_dropped] K, #[only_dropped] V> Drop for BTreeMap<K, V> {
129+
fn drop(&mut self) {
130+
// Recursively drops the key-value pairs but doesn't otherwise
131+
// inspect them, so we can use `#[only_dropped]` here.
132+
drop(unsafe {ptr::read(self) }.into_iter())
133+
}
134+
}
135+
```
136+
137+
A type where `#[fully_unused]` would be useful is a `Weak` pointer for a variant of `Rc`
138+
where the value is dropped when the last `Rc` goes out of scope. Dropping a `Weak` pointer
139+
would never access `T` in this case.
140+
141+
142+
# Reference-level explanation
143+
[reference-level-explanation]: #reference-level-explanation
144+
145+
Whenever we use a value of a given type this type has to be **well-formed**, requiring
146+
that all lifetimes in this type are live. An exception to this is the implicit drop when
147+
a variable goes out of scope. While borrowck ensures that dropping the variable is safe,
148+
this does not necessarily require all lifetimes to be live.
149+
150+
When implicitly dropping a variable of type `T`, liveness requirements are computed as follows:
151+
- If `T` does not have any drop glue, do not add any requirements.
152+
- If `T` is a trait object, `T` has to be live.
153+
- If `T` has an explicit `Drop` impl, require all generic argument to be live, unless
154+
- they are marked with `#[fully_unused]`, in which case they are ignored,
155+
- or they are marked with `#[only_dropped]`, in which case recurse into the generic argument.
156+
- Regardless of whether `T` implements `Drop`, recurse into all types *owned* by `T`:
157+
- references, raw pointers, function pointers, function items and scalars do not own
158+
anything. They can be trivially dropped.
159+
- tuples and arrays consider their element types to be owned.
160+
- all fields (of all variants) of ADTs are considered owned. We consider all variants
161+
for enums. The only exception here is `ManuallyDrop<U>` which is not considered to own `U`. `PhantomData<U>` does not have any fields and therefore also does not consider
162+
`U` to be owned.
163+
- closures and generators own their captured upvars.
164+
165+
Checking drop impls may error for generic parameters which are known to be incorrectly marked:
166+
- `#[fully_unused]` parameters which are recursively owned
167+
- `#[only_dropped]` parameters which are required to be live by a recursively owned type
168+
169+
This cannot catch all misuses, as the parameters can be incorrectly used by the `Drop` impl itself.
170+
We therefore require the impl to be marked as `unsafe`.
171+
172+
## How this differs from the status quo
173+
174+
Instead of `#[fully_unused]` and `#[only_dropped]`,there is only the `#[may_dangle]` attribute which
175+
skips the generic parameter. This is equivalent to the behavior of `#[fully_unused]` and relies on the recursion
176+
into types owned by `T` to figure out the correct constraints.
177+
178+
`PhantomData<U>` currently considers `U` to be owned while not having drop glue itself. This means
179+
that `(PhantomData<PrintOnDrop<'s>>, String)` requires `'s` to be live while
180+
`(PhantomData<PrintOnDrop<'s>>, u32)` does not. This is required for get the
181+
behavior of `#[only_dropped]` for parameters otherwise not owned by adding `PhantomData` as a field.
182+
One can easily forget this, which caused the [unsound](https://github.com/rust-lang/rust/issues/76367)
183+
[issues](https://github.com/rust-lang/rust/issues/99408) mentioned above.
184+
185+
# Drawbacks
186+
[drawbacks]: #drawbacks
187+
188+
It requires an additional attribute when compared with `#[may_dangle]` and also proposes checks that the
189+
attributes are correctly used. This adds a small amount of implementation complexity to the compiler.
190+
These new attributes are still not fully checked by the compiler and require `unsafe`.
191+
192+
This RFC does not explicitly exclude stabilizing these two attributes, as they are clearer and far less
193+
dangerous to use when compared with `#[may_dangle]`. Stabilizing these attributes will make it harder to
194+
stabilize a more general solution like type state.
195+
196+
# Rationale and alternatives
197+
[rationale-and-alternatives]: #rationale-and-alternatives
198+
199+
The status quo of `#[may_dangle]` and "spooky-dropck-at-a-distance" is far from ideal and has already
200+
resulted in unsound issues. Documenting the current behavior makes is more difficult to change later
201+
while not officially documenting it is bound to lead to more issues and confusion going forward.
202+
It is therefore quite important to improve the status quo.
203+
204+
A more general extension to deal with partially invalid types is far from trivial. We currently
205+
assume types to always be well-formed and any approach which generalizes `#[may_dangle]` will
206+
have major consequences for how well-formedness is handled. This impacts many - often implicit -
207+
interactions and assumptions. It is highly unlikely that we will have the capacity for any such change
208+
in the near future. The benefits from such are change are likely to be fairly limited while
209+
adding significant complexity.
210+
211+
# Prior art
212+
[prior-art]: #prior-art
213+
214+
`#[may_dangle]` is already a refinement of the previous `#[unsafe_destructor_blind_to_params]` attribute
215+
([RFC 1327](https://github.com/rust-lang/rfcs/pull/1327)).
216+
217+
There is also [RFC 3390](https://github.com/rust-lang/rfcs/pull/3390) which attempts to define a more
218+
general extension to replace `#[may_dangle]`. As mentioned in the rationale, such an approach is not
219+
feasable right now.
220+
221+
222+
# Unresolved questions
223+
[unresolved-questions]: #unresolved-questions
224+
225+
Should these attributes remain purely unstable for use in the standard library or do we want
226+
to provide them to stable users?
227+
228+
229+
# Future possibilities
230+
[future-possibilities]: #future-possibilities
231+
232+
Extending or generalizing the dropck eyepatch... something something type state.
233+
234+
[`IndexVec`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_index/vec/struct.IndexVec.html)

0 commit comments

Comments
 (0)