Skip to content

Commit 39131d4

Browse files
lcnrlcnr
committed
replace may dangle with 2 attributes
1 parent e66fda2 commit 39131d4

File tree

1 file changed

+192
-54
lines changed

1 file changed

+192
-54
lines changed

‎0000-template.md

Lines changed: 192 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,235 @@
1-
- Feature Name: (fill me in with a unique ident, `my_awesome_feature`)
1+
- Feature Name: (`dropck_eyepatch_v3`)
22
- Start Date: (fill me in with today's date, YYYY-MM-DD)
33
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000)
44
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
55

66
# Summary
77
[summary]: #summary
88

9-
One paragraph explanation of the feature.
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.
1012

1113
# Motivation
1214
[motivation]: #motivation
1315

14-
Why are we doing this? What use cases does it support? What is the expected outcome?
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)
1555

1656
# Guide-level explanation
1757
[guide-level-explanation]: #guide-level-explanation
1858

19-
Explain the proposal as if it was already included in the language and you were teaching it to another Rust programmer. That generally means:
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 considered 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 especially useful
120+
for type parameters, consider the following simplified types from the standard library:
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.
20140

21-
- Introducing new named concepts.
22-
- Explaining the feature largely in terms of examples.
23-
- Explaining how Rust programmers should *think* about the feature, and how it should impact the way they use Rust. It should explain the impact as concretely as possible.
24-
- If applicable, provide sample error messages, deprecation warnings, or migration guidance.
25-
- If applicable, describe the differences between teaching this to existing Rust programmers and new Rust programmers.
26-
- Discuss how this impacts the ability to read, understand, and maintain Rust code. Code is read and modified far more often than written; will the proposed feature make code easier to maintain?
27-
28-
For implementation-oriented RFCs (e.g. for compiler internals), this section should focus on how compiler contributors should think about the change, and give examples of its concrete impact. For policy RFCs, this section should provide an example-driven introduction to the policy, and explain its impact in concrete terms.
29141

30142
# Reference-level explanation
31143
[reference-level-explanation]: #reference-level-explanation
32144

33-
This is the technical portion of the RFC. Explain the design in sufficient detail that:
34-
35-
- Its interaction with other features is clear.
36-
- It is reasonably clear how the feature would be implemented.
37-
- Corner cases are dissected by example.
38-
39-
The section should return to the examples given in the previous section, and explain more fully how the detailed proposal makes those examples work.
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 are trivially drop.
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 does not consider `U` to
162+
be owned. `PhantomData<U>` does not have any fields and therefore also does not consider
163+
`U` to be owned.
164+
- closures and generators consider their upvars to be owned.
165+
166+
Checking drop impls may error for generic parameters which are known to be incorrectly marked:
167+
- `#[fully_unused]` parameters which are recursively owned
168+
- `#[only_dropped]` parameters which are required to be live by a recursively owned type
169+
170+
This cannot catch all misuses, as the parameters can be incorrectly used by the `Drop` impl itself.
171+
We therefore require the impl to be marked as `unsafe`.
172+
173+
## How this differs from the status quo
174+
175+
Instead of `#[fully_unused]` and `#[only_dropped]`,there is only the `#[may_dangle]` attribute which
176+
skips the generic parameter. This is equivalent to the behavior of `#[fully_unused]` and relies on the recursion
177+
into types owned by `T` to figure out the correct constraints.
178+
179+
`PhantomData<U>` currently considers `U` to be owned while not having drop glue itself. This means
180+
that `(PhantomData<PrintOnDrop<'s>>, String)` requires `'s` to be live while
181+
`(PhantomData<PrintOnDrop<'s>>, u32)` does not. The current behavior is required for get the
182+
behavior of `#[only_dropped]` for unowned parameters by adding `PhantomData` as a field.
183+
One can easily forget this, which caused the [unsound](https://github.com/rust-lang/rust/issues/76367)
184+
[issues](https://github.com/rust-lang/rust/issues/99408) mentioned above.
40185

41186
# Drawbacks
42187
[drawbacks]: #drawbacks
43188

44-
Why should we *not* do this?
189+
It requires an additional attribute when compared with `#[may_dangle]` and also proposes checks that the
190+
attributes are correctly used. This adds a small amount of implementation complexity to the compiler.
191+
These new attributes are still not fully checked by the compiler and require `unsafe`.
192+
193+
This RFC does not explicitly exclude stabilizing these two attributes, as they are clearer and far less
194+
dangerous to use when compared with `#[may_dangle]`. Stabilizing these attributes will make it harder to
195+
stabilize a more general solution like type state.
45196

46197
# Rationale and alternatives
47198
[rationale-and-alternatives]: #rationale-and-alternatives
48199

49-
- Why is this design the best in the space of possible designs?
50-
- What other designs have been considered and what is the rationale for not choosing them?
51-
- What is the impact of not doing this?
52-
- If this is a language proposal, could this be done in a library or macro instead? Does the proposed change make Rust code easier or harder to read, understand, and maintain?
200+
The status quo of `#[may_dangle]` and "spooky-dropck-at-a-distance" is far from ideal and has already
201+
resulted in unsound issues. Documenting the current behavior makes is more difficult to change later
202+
while not officially documenting it is bound to lead to more issues and confusion going forward.
203+
It is therefore quite important to improve the status quo.
204+
205+
A more general extension to deal with partially invalid types is far from trivial. We currently
206+
assume types to always be well-formed and any approach which generalizes `#[may_dangle]` will
207+
have huge consequences for how well-formedness is handled. This impacts many - often implicit -
208+
interactions and assumptions. It is highly unlikely that we will have the capacity for any such change
209+
in the near future. The benefits from such are change are also likely to be fairly limited while
210+
adding significant complexity.
53211

54212
# Prior art
55213
[prior-art]: #prior-art
56214

57-
Discuss prior art, both the good and the bad, in relation to this proposal.
58-
A few examples of what this can include are:
215+
`#[may_dangle]` is already a refinement of the previous `#[unsafe_destructor_blind_to_params]` attribute
216+
([RFC 1327](https://github.com/rust-lang/rfcs/pull/1327)).
59217

60-
- For language, library, cargo, tools, and compiler proposals: Does this feature exist in other programming languages and what experience have their community had?
61-
- For community proposals: Is this done by some other community and what were their experiences with it?
62-
- For other teams: What lessons can we learn from what other communities have done here?
63-
- Papers: Are there any published papers or great posts that discuss this? If you have some relevant papers to refer to, this can serve as a more detailed theoretical background.
218+
There is also [RFC 3390](https://github.com/rust-lang/rfcs/pull/3390) which attempts to define a more
219+
general extension to replace `#[may_dangle]`. As mentioned in the rationale, such an approach is not
220+
feasable right now.
64221

65-
This section is intended to encourage you as an author to think about the lessons from other languages, provide readers of your RFC with a fuller picture.
66-
If there is no prior art, that is fine - your ideas are interesting to us whether they are brand new or if it is an adaptation from other languages.
67-
68-
Note that while precedent set by other languages is some motivation, it does not on its own motivate an RFC.
69-
Please also take into consideration that rust sometimes intentionally diverges from common language features.
70222

71223
# Unresolved questions
72224
[unresolved-questions]: #unresolved-questions
73225

74-
- What parts of the design do you expect to resolve through the RFC process before this gets merged?
75-
- What parts of the design do you expect to resolve through the implementation of this feature before stabilization?
76-
- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?
226+
Should these attributes remain purely unstable for use in the standard library or do we want
227+
to provide them to stable users?
228+
77229

78230
# Future possibilities
79231
[future-possibilities]: #future-possibilities
80232

81-
Think about what the natural extension and evolution of your proposal would
82-
be and how it would affect the language and project as a whole in a holistic
83-
way. Try to use this section as a tool to more fully consider all possible
84-
interactions with the project and language in your proposal.
85-
Also consider how this all fits into the roadmap for the project
86-
and of the relevant sub-team.
87-
88-
This is also a good place to "dump ideas", if they are out of scope for the
89-
RFC you are writing but otherwise related.
90-
91-
If you have tried and cannot think of any future possibilities,
92-
you may simply state that you cannot think of anything.
233+
Extending or generalizing the dropck eyepatch... something something type state.
93234

94-
Note that having something written down in the future-possibilities section
95-
is not a reason to accept the current or a future RFC; such notes should be
96-
in the section on motivation or rationale in this or subsequent RFCs.
97-
The section merely provides additional information.
235+
[`IndexVec`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_index/vec/struct.IndexVec.html)

0 commit comments

Comments
 (0)