From 45cf48e2dfc83495767a10795e32c60b32e9c089 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 19 Oct 2015 15:40:52 +0200 Subject: [PATCH 1/8] first draft of dropck-eyepatch RFC. --- text/0000-dropck-param-eyepatch.md | 426 +++++++++++++++++++++++++++++ 1 file changed, 426 insertions(+) create mode 100644 text/0000-dropck-param-eyepatch.md diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md new file mode 100644 index 00000000000..69526ec430c --- /dev/null +++ b/text/0000-dropck-param-eyepatch.md @@ -0,0 +1,426 @@ +- Feature Name: dropck_eyepatch +- Start Date: 2015-10-19 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Refine the unguarded-escape-hatch from [RFC 1238][] (nonparametric +dropck) so that instead of a single attribute side-stepping *all* +dropck constraints for a type's destructor, we instead have a more +focused attribute that specifies exactly which type and/or lifetime +parameters the destructor is guaranteed not to access. + +[RFC 1238]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md +[RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md + +# Motivation +[motivation]: #motivation + +The unguarded escape hatch (UGEH) from [RFC 1238] is a blunt +instrument: when you use `unsafe_destructor_blind_to_params`, it is +asserting that your destructor does not access borrowed data whose +type includes *any* lifetime or type parameter of the type. + +For example, the current destructor for `RawVec` (in `liballoc/`) +looks like this: + +```rust +impl Drop for RawVec { + #[unsafe_destructor_blind_to_params] + /// Frees the memory owned by the RawVec *without* trying to Drop its contents. + fn drop(&mut self) { + [... free memory using global system allocator ...] + } +} +``` + +The above is sound today, because the above destructor does not call +any methods that can access borrowed data in the values of type `T`, +and so we do not need to enforce the drop-ordering constraints imposed +when you leave out the `unsafe_destructor_blind_to_params` attribute. + +While the above attribute suffices for many use cases today, it is not +fine-grain enough for other cases of interest. In particular, it +cannot express that the destructor will not access borrowed data +behind a *subset* of the type parameters. + +Here are two concrete examples of where the need for this arises: + +## Example: `CheckedHashMap` + +The original Sound Generic Drop proposal ([RFC 769][]) +had an [appendix][RFC 769 CheckedHashMap] with an example of a +`CheckedHashMap` type that called the hashcode method +for all of the keys in the map in its destructor. +This is clearly a type where we *cannot* claim that we do not access +borrowed data potentially hidden behind `K`, so it would be unsound +to use the blunt `unsafe_destructor_blind_to_params` attribute on this +type. + +However, the values of the `V` parameter to `CheckedHashMap` are, in +all likelihood, *not* accessed by the `CheckedHashMap` destructor. If +that is the case, then it should be sound to instantiate `V` with a +type that contains references to other parts of the map (e.g., +references to the keys or to other values in the map). However, we +cannot express this today: There is no way to say that the +`CheckedHashMap` will not access borrowed data that is behind *just* +`V`. + +[RFC 769 CheckedHashMap]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#appendix-a-why-and-when-would-drop-read-from-borrowed-data + +## Example: `Vec` + +The Rust developers have been talking for [a long time][RFC Issue 538] +about adding an `Allocator` trait that would allow users to override +the allocator used for the backing storage of collection types like +`Vec` and `HashMap`. + +For example, we would like to generalize the `RawVec` given above as +follows: + +```rust +#[unsafe_no_drop_flag] +pub struct RawVec { + ptr: Unique, + cap: usize, + alloc: A, +} + +impl Drop for RawVec { + #[should_we_put_ugeh_attribute_here_or_not(???)] + /// Frees the memory owned by the RawVec *without* trying to Drop its contents. + fn drop(&mut self) { + [... free memory using self.alloc ...] + } +} +``` + +However, we *cannot* soundly add an allocator parameter to a +collection that today uses the `unsafe_destructor_blind_to_params` +UGEH attribute in the destructor that deallocates, because that blunt +instrument would allow someone to write this: + +```rust +// (`ArenaAllocator`, when dropped, automatically frees its allocated blocks) + +// (Usual pattern for assigning same extent to `v` and `a`.) +let (v, a): (Vec, ArenaAllocator); + +a = ArenaAllocator::new(); +v = Vec::with_allocator(&a); + +... v.push(stuff) ... + +// at end of scope, `a` may be dropped before `v`, invalidating +// soundness of subsequent invocation of destructor for `v` (because +// that would try to free buffer of `v` via `v.buf.alloc` (== `&a`)). +``` + +The only way today to disallow the above unsound code would be to +remove `unsafe_destructor_blind_to_params` from `RawVec`/ `Vec`, which +would break other code (for example, code using `Vec` as the backing +storage for [cyclic graph structures][dropck_legal_cycles.rs]). + +[RFC Issue 538]: https://github.com/rust-lang/rfcs/issues/538 + +[dropck_legal_cycles.rs]: https://github.com/rust-lang/rust/blob/098a7a07ee6d11cf6d2b9d18918f26be95ee2f66/src/test/run-pass/dropck_legal_cycles.rs + +# Detailed design +[detailed design]: #detailed-design + + 1. Add a new fine-grained attribute, `unsafe_destructor_blind_to` + (which this RFC will sometimes call the "eyepatch", since it does + not make dropck totally blind; just blind on one "side"). + + 2. Remove `unsafe_destructor_blind_to_params`, since all uses of it + should be expressible via `unsafe_destructor_blind_to` (once that + has been completely implemented). + +## The "eyepatch" attribute + +Add a new attribute, `unsafe_destructor_blind_to(ARG)` (the "eyepatch"). + +The eyepatch is similar to `unsafe_destructor_blind_to_params`: it is +attached to the destructor[1](#footnote1), and it is meant +to assert that a destructor is guaranteed not to access certain kinds +of data accessible via `self`. + +The main difference is that the eyepatch has a single required +parameter, `ARG`. This is the place where you specify exactly *what* +the destructor is blind to (i.e., what will dropck treat as +inaccessible from the destructor for this type). + +There are two things one can put the `ARG` for a given eyepatch: one +of the type parameters for the type, or one of the lifetime parameters +for the type.[2](#footnote2) + +### Examples stolen from the Rustonomicon + +[nomicon dropck]: https://doc.rust-lang.org/nightly/nomicon/dropck.html + +So, adapting some examples from the Rustonomicon +[Drop Check][nomicon dropck] chapter, we would be able to write +the following. + +Example of eyepatch on a lifetime parameter:: + +```rust +struct InspectorA<'a>(&'a u8, &'static str); + +impl<'a> Drop for InspectorA<'a> { + #[unsafe_destructor_blind_to('a)] + fn drop(&mut self) { + println!("InspectorA(_, {}) knows when *not* to inspect.", self.1); + } +} +``` + +Example of eyepatch on a type parameter: + +```rust +use std::fmt; + +struct InspectorB(T, &'static str); + +impl Drop for InspectorB { + #[unsafe_destructor_blind_to(T)] + fn drop(&mut self) { + println!("InspectorB(_, {}) knows when *not* to inspect.", self.1); + } +} +``` + +Both of the above two examples are much the same as if we had used the +old `unsafe_destructor_blind_to_params` UGEH attribute. + +### Example: RawVec + +To generalize `RawVec` from the [motivation](#motivation) with an +`Allocator` correctly (that is, soundly and without breaking existing +code), we would now write: + +```rust +impl Drop for RawVec { + #[unsafe_destructor_blind_to(T)] + /// Frees the memory owned by the RawVec *without* trying to Drop its contents. + fn drop(&mut self) { + [... free memory using self.alloc ...] + } +} +``` + +The use of `unsafe_destructor_blind_to(T)` here asserts that even +though the destructor may access borrowed data through `A` (and thus +dropck must impose drop-ordering constraints for lifetimes occurring +in the type of `A`), the developer is guaranteeing that no access to +borrowed data will occur via the type `T`. + +The latter is not expressible today even with +`unsafe_destructor_blind_to_params`; there is no way to say that a +type will not access `T` in its destructor while also ensuring the +proper drop-ordering relationship between `RawVec` and `A`. + +### Example; Multiple Lifetimes + +Example: The above `InspectorA` carried a `&'static str` that was +always safe to access from the destructor. + +If we wanted to generalize this type a bit, we might write: + +```rust +struct InspectorC<'a,'b,'c>(&'a str, &'b str, &'c str); + +impl<'a,'b,'c> Drop for InspectorC<'a,'b,'c> { + #[unsafe_destructor_blind_to('a)] + #[unsafe_destructor_blind_to('c)] + fn drop(&mut self) { + println!("InspectorA(_, {}, _) knows when *not* to inspect.", self.1); + } +} +``` + +This type, like `InspectorA`, is careful to only access the `&str` +that it holds in its destructor; but now the borrowed string slice +does not have `'static` lifetime, so we must make sure that we do not +claim that we are blind to its lifetime (`'b`). + +(This example also illustrates that one can attach multiple instances +of the eyepatch attribute to a destructor, each with a distinct input +for its `ARG`.) + +Given the definition above, this code will compile and run properly: + +```rust +fn this_will_work() { + let b; // ensure that `b` strictly outlives `i`. + let (i,a,c); + a = format!("a"); + b = format!("b"); + c = format!("c"); + i = InspectorC(a, b, c); +} +``` + +while this code will be rejected by the compiler: + +```rust +fn this_will_not_work() { + let (a,c); + let (i,b); // OOPS: `b` not guaranteed to survive for `i`'s destructor. + a = format!("a"); + b = format!("b"); + c = format!("c"); + i = InspectorC(a, b, c); +} +``` + +## Semantics + +How does this work, you might ask? + +The idea is actually simple: the dropck rule stays mostly the same, +except for a small twist. + +The Drop-Check rule at this point essentially says: + +> if the type of `v` owns data of type `D`, where +> +> (1.) the `impl Drop for D` is either type-parametric, or lifetime-parametric over `'a`, and +> (2.) the structure of `D` can reach a reference of type `&'a _`, +> +> then `'a` must strictly outlive the scope of `v` + +The main change we want to make is to the second condition. +Instead of just saying "the structure of `D` can reach a reference of type `&'a _`", +we want first to replace eyepatched lifetimes and types within `D` with `'static` and `()`, +respectively. Call this revised type `patched(D)`. + +Then the new condition is: + +> (2.) the structure of patched(D) can reach a reference of type `&'a _`, + +*Everything* else is the same. + +In particular, the patching substitution is *only* applied with +respect to a particular destructor. Just because `Vec` is blind to `T` +does not mean that we will ignore the actual type instantiated at `T` +in terms of drop-ordering constraints. + +For example, in `Vec>`, even though `Vec` +itself is blind to the whole type `InspectorC<'a, 'name, 'c>` when we +are considering the `impl Drop for Vec`, we *still* honor the +constraint that `'name` must strictly outlive the `Vec` (because we +continue to consider all `D` that is data owned by a value `v`, +including when `D` == `InspectorC<'a,'name,'c>`). + +## Prototype + +pnkfelix has implemented a proof-of-concept +[implementation][pnkfelix prototype] of this feature. +It uses the substitution machinery we already have in the compiler +to express the semantics above. + +## Limitations of prototype (not part of design) + +Here we note a few limitations of the current prototype. These +limitations are *not* being proposed as part of the specification of +the feature. + +1. The eyepatch is not attached to the +destructor in the current [prototype][pnkfelix prototype]; it is +instead attached to the `struct`/`enum` definition itself. + +2. The eyepatch is only able to accept a type +parameter, not a lifetime, in the current +[prototype][pnkfelix prototype]; it is instead attached to the +`struct`/`enum` definition itself. + +Fixing the above limitations should just be a matter of engineering, +not a fundamental hurdle to overcome in the feature's design in the +context of the language. + +[pnkfelix prototype]: https://github.com/pnkfelix/rust/commits/fsk-nonparam-blind-to-indiv + +# Drawbacks +[drawbacks]: #drawbacks + +This attribute, like the original `unsafe_destructor_blind_to_params` +UGEH attribute, is ugly. + +It would be nicer if to actually change the language in a way where we +could check the assertions being made by the programmer, rather than +trusting them. (pnkfelix has some thoughts on this, which are mostly +reflected in what he wrote in the [RFC 1238 alternatives][].) + +[RFC 1238 alternatives]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md#continue-supporting-parametricity + +# Alternatives +[alternatives]: #alternatives + +## unsafe_destructor_blind_to(T1, T2, ...) + +The eyepatch could take multiple arguments, rather than requiring a +distinct instance of the attribute for each parameter that we are +blind to. + +However, I think that each usage of the attribute needs to be +considered, since it represents a separate "attack vector" where +unsoundness can be introduced, and therefore it deserves more than +just a comma and a space added to the program text when it is added. + +(I only weakly support the latter position; it is obviously easy +to support this form if that is deemed desirable.) + +## Wait for proper parametricity + +As alluded to in the [drawbacks][], in principle we could provide +similar expressiveness to that offered by the eyepatch (which is +acting as a fine-grained escape hatch from dropck) by instead offering +some language extension where the compiler would actually analyze the +code based on programmer annotations indicating which types and +lifetimes are not used by a function. + +In my opinion I am of two minds on this (but they are both in favor +this RFC rather than waiting for a sound compiler analysis): + + 1. We will always need an escape hatch. The programmer will always need + a way to assert something that she knows to be true, even if the compiler + cannot prove it. (A simple example: Calling a third-party API that has not + yet added the necessary annotations.) + + This RFC is proposing that we keep an escape hatch, but we make it more + expressive. + + 2. If we eventually *do* have a sound compiler analysis, I see the + compiler changes and library annotations suggested by this RFC as + being in line with what that compiler analysis would end up using + anyway. In other words: Assume we *did* add some way for the programmer + to write that `T` is parametric (e.g. `T: ?Special` in the [RFC 1238 alternatives]). + Even then, we would still need the compiler changes suggested by this RFC, + and at that point hopefully the task would be for the programmer to mechanically + replace occurrences of `#[unsafe_destructor_blind_to(T)` with `T: ?Special` + (and then see if the library builds). + + In other words, I see the form suggested by this RFC as being a step *towards* + a proper analysis, in the sense that it is getting programmers used to thinking + about the individual parameters and their relationship with the container, rather + than just reasoning about the container on its own without any consideration + of each type/lifetime parameter. + +## Do nothing + +If we do nothing, then we cannot add `Vec` soundly. + +# Unresolved questions +[unresolved]: #unresolved-questions + +Is there any issue with writing `'a` in an attribute like +`#[unsafe_destructor_blind_to('a)]`? (The prototype, as mentioned +[above](#footnote2), does not currently accept lifetime parameter +inputs, so I do not know the answer off hand. + +Is the definition of the drop-check rule sound with this `patched(D)` +variant? (We have not proven any previous variation of the rule +sound; I think it would be an interesting student project though.) From ceb091eb304292a6e1d9d1f444d595cc4acbcf7f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 19 Oct 2015 23:25:46 +0200 Subject: [PATCH 2/8] add note about interaction between macro hygiene and attributes. --- text/0000-dropck-param-eyepatch.md | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index 69526ec430c..dd3175e2aac 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -346,14 +346,53 @@ context of the language. # Drawbacks [drawbacks]: #drawbacks +## Ugliness + This attribute, like the original `unsafe_destructor_blind_to_params` UGEH attribute, is ugly. +## Unchecked assertions boo + It would be nicer if to actually change the language in a way where we could check the assertions being made by the programmer, rather than trusting them. (pnkfelix has some thoughts on this, which are mostly reflected in what he wrote in the [RFC 1238 alternatives][].) +## Attributes lack hygiene + +As noted by arielb1, putting type parameter identifiers into attributes +is not likely to play well with macro hygiene. + +Here is a concrete example: + +```rust +struct Yell2(A, B); + +macro_rules! make_yell2a { + ($A:ident, $B:ident) => { + impl<$A:Debug,$B:Debug> Drop for Yell2<$A,$B> { + #[unsafe_destructor_blind_to(???)] // <---- + fn drop(&mut self) { + println!("Yell1(_, {:?})", self.1); + } + } + } +} + +make_yell2a!(X, Y); +``` + +Here is the issue: In the above, what does one put in for the `???` to +say that we are blind to the first type parameter to `Yell2`? +`#[unsafe_destructor_blind_to(A)` would be nonsense, becauase in the instantiation of the macro, `$A` will be mapped to the identifier `X`. so perhaps we should write it is blind to `X` -- but to me one big point of macro hygiene is that a macro definition should not have to build in knowledge of the identifiers chosen at the usage site, and this is the opposite of that. + +(I don't think `#[unsafe_destructor_blind_to($A)` works, because our attribute system operates at the same meta-level that macros operate at , but I would be happy to be proven wrong.) + +---- + +Despite my somewhat dire attitude above, I don't think this is a significant problem in the short term. This sort of macro is probably rare, and the combination of this macro with UGEH is doubly so. You cannot define a destructor multiple times for the same type, so it seems weird to me to abstract this code construction at this particular level. + + [RFC 1238 alternatives]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md#continue-supporting-parametricity # Alternatives From be75059fbf9468ab6f2be8cddce3f78503c1df94 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 20 Oct 2015 16:36:01 +0200 Subject: [PATCH 3/8] add arielb1 alternatives to the alts section. --- text/0000-dropck-param-eyepatch.md | 105 ++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index dd3175e2aac..4cadb2b88fa 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -89,7 +89,7 @@ pub struct RawVec { } impl Drop for RawVec { - #[should_we_put_ugeh_attribute_here_or_not(???)] + #[should_we_put_ugeh_attribute_here_or_not(???)] /// Frees the memory owned by the RawVec *without* trying to Drop its contents. fn drop(&mut self) { [... free memory using self.alloc ...] @@ -316,6 +316,7 @@ continue to consider all `D` that is data owned by a value `v`, including when `D` == `InspectorC<'a,'name,'c>`). ## Prototype +[prototype]: #prototype pnkfelix has implemented a proof-of-concept [implementation][pnkfelix prototype] of this feature. @@ -359,6 +360,7 @@ trusting them. (pnkfelix has some thoughts on this, which are mostly reflected in what he wrote in the [RFC 1238 alternatives][].) ## Attributes lack hygiene +[attributes-lack-hygiene]: #attributes-lack-hygiene As noted by arielb1, putting type parameter identifiers into attributes is not likely to play well with macro hygiene. @@ -412,7 +414,108 @@ just a comma and a space added to the program text when it is added. (I only weakly support the latter position; it is obviously easy to support this form if that is deemed desirable.) +## Use a blacklist not a whitelist +[blacklist-not-whitelist]: #use-a-blacklist-not-a-whitelist + +The `unsafe_destructor_blind_to` attribute acts as a whitelist of +parameters that we are telling dropck to ignore in its analysis +of this destructor. + +We could instead add a way to list the lifetimes and/or +type-expressions (e.g. parameters, projections from parameters) that +the destructor may access (and thus treat that list as a blacklist of +parameters that dropck needs to *include* in its analysis). + +arielb1 first suggested this as an attribute form +[here][blacklist attribute], but then provided a different formulation +of the idea by expressing it as a [`where`-clause][blacklist where] on +the `fn drop` method (which is what I will show in the next section). + +[blacklist attribute]: https://github.com/rust-lang/rfcs/pull/1327#issuecomment-149302743 + +[blacklist where]: https://github.com/rust-lang/rfcs/pull/1327#issuecomment-149329351 + +## Make dropck "see again" via (focused) where-clauses + +(This alternative carries over some ideas from +[the previous section][blacklist-not-whitelist], but it stands well on +its own as something to consider, so I am giving it its own section.) + +The idea is that we keep the UGEH attribute, blunt hammer that it is. +You first opt out of the dropck ordering constraints via that, and +then you add back in ordering constraints via `where` clauses. + +(The ordering constraints in question would normally be *implied* by +the dropck analysis; the point is that UGEH is opting out of that +analysis, and so we are now adding them back in.) + +Here is the allocator example expressed in this fashion: + +```rust +impl Drop for RawVec { + #[unsafe_destructor_blind_to_params] + /// Frees the memory owned by the RawVec *without* trying to Drop its contents. + fn drop<'s>(&'s mut self) where A: 's { + // ~~~~~~~~~~~ + // | + // | + // This constraint (that `A` outlives `'s`), and other conditions + // relating `'s` and `Self` are normally implied by Rust's type + // system, but `unsafe_destructor_blind_to_params` opts out of + // enforcing them. This `where`-clause is opting back into *just* + // the `A:'s` again. + // + // Note we are *still* opting out of `T: 's` via + // `unsafe_destructor_blind_to_params`, and thus our overall + // goal (of not breaking code that relies on `T` not having to + // survive the destructor call) is accomplished. + + [... free memory using self.alloc ...] + } +} +``` + +This approach, if we can make it work, seems fine to me. It certainly +avoids a number of problems that the eyepatch attribute has. + +Advantages of fn-drop-with-where-clauses: + + * It completely sidesteps the [hygiene issue][attributes-lack-hygiene]. + + * If the eyepatch attribute is to be limited to identifiers (type + parameters) and lifetimes, then this approach is more expressive, + since it would allow one to put type-projections into the + constraints. + +Drawbacks of fn-drop-with-where-clauses: + + * Its not 100% clear what our implementation strategy will be for it, + while the eyepatch attribute does have a [prototype]. + + I actually do not give this drawback much weight; resolving this + may be merely a matter of just trying to do it: e.g., build up the + set of where-clauses when we make the ADT's representatin, and + then have `dropck` insert instantiate and insert them as needed. + + * It might have the wrong ergonomics for developers: It seems bad to + have the blunt hammer introduce all sorts of potential + unsoundness, and rely on the developer to keep the set of + `where`-clauses on the `fn drop` up to date. + + This would be a pretty bad drawback, *if* the language and + compiler were to stagnate. But my intention/goal is to eventually + put in a [sound compiler analysis][wait-for-proper-parametricity]. + In other words, in the future, I will be more concerned about the + ergonomics of the code that uses the sound analysis. I will not be + concerned about "gotcha's" associated with the UGEH escape hatch. + +(The most important thing I want to convey is that I believe that both +the eyepatch attribute and fn-drop-with-where-clauses are capable of +resolving the real issues that I face today, and I would be happy for +either proposal to be accepted.) + ## Wait for proper parametricity +[wait-for-proper-parametricity]: #wait-for-proper-parametricity As alluded to in the [drawbacks][], in principle we could provide similar expressiveness to that offered by the eyepatch (which is From 8f8d56075068eab8167f4b166f43b65c321fe7dc Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 1 Jun 2016 23:17:49 +0200 Subject: [PATCH 4/8] Update dropck-eyepatch RFC to use `#[may_dangle] ARG` attribute (where `ARG` is either a formal lifetime parameter `'a` or a formal type parmeter `T`). --- text/0000-dropck-param-eyepatch.md | 247 +++++++++++++++-------------- 1 file changed, 130 insertions(+), 117 deletions(-) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index 4cadb2b88fa..c5e5c6eedb0 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -9,9 +9,16 @@ Refine the unguarded-escape-hatch from [RFC 1238][] (nonparametric dropck) so that instead of a single attribute side-stepping *all* dropck constraints for a type's destructor, we instead have a more -focused attribute that specifies exactly which type and/or lifetime +focused system that specifies exactly which type and/or lifetime parameters the destructor is guaranteed not to access. +Specifically, this RFC proposes adding the capability to attach +attributes to the binding sites for generic parameters (i.e. lifetime +and type paramters). Atop that capability, this RFC proposes adding a +`#[may_dangle]` attribute that indicates that a given lifetime or type +holds data that must not be accessed during the dynamic extent of that +`drop` invocation. + [RFC 1238]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md [RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md @@ -130,33 +137,127 @@ storage for [cyclic graph structures][dropck_legal_cycles.rs]). # Detailed design [detailed design]: #detailed-design - 1. Add a new fine-grained attribute, `unsafe_destructor_blind_to` - (which this RFC will sometimes call the "eyepatch", since it does - not make dropck totally blind; just blind on one "side"). + 1. Add the ability to attach attributes to syntax that binds formal + lifetime or type parmeters. For the purposes of this RFC, the only + place in the syntax that requires such attributes are `impl` + blocks, as in `impl Drop for Type { ... }` + + 2. Add a new fine-grained attribute, `may_dangle`, which is attached + to the binding sites for lifetime or type parameters on an `Drop` + implementation. + This RFC will sometimes call this attribute the "eyepatch", + since it does + not make dropck totally blind; just blind on one "side". + + 3. Add a new requirement that any `Drop` implementation that uses the + `#[may_dangle]` attribute must be declared as an `unsafe impl`. + This reflects the fact that such `Drop` implementations have + an additional constraint on their behavior (namely that they cannot + access certain kinds of data) that will not be verified by the + compiler and thus must be verified by the programmer. + + 4. Remove `unsafe_destructor_blind_to_params`, since all uses of it + should be expressible via `#[may_dangle]`. + +## Attributes on lifetime or type parameters + +This is a simple extension to the syntax. + +Constructions like the following will now become legal. + +Example of eyepatch attribute on a single type parameter: +```rust +unsafe impl<'a, #[may_dangle] X, Y> Drop for Foo<'a, X, Y> { + ... +} +``` + +Example of eyepatch attribute on a lifetime parameter: +```rust +unsafe impl<#[may_dangle] 'a, X, Y> Drop for Bar<'a, X, Y> { + ... +} +``` + +Example of eyepatch attribute on multiple parameters: +```rust +unsafe impl<#[may_dangle] 'a, X, #[may_dangle] Y> Drop for Baz<'a, X, Y> { + ... +} +``` + +These attributes are only written next to the formal binding +sites for the generic parameters. The *usage* sites, points +which refer back to the parameters, continue to disallow the use +of attributes. + +So while this is legal syntax: + +```rust +unsafe impl<'a, #[may_dangle] X, Y> Drop for Foo<'a, X, Y> { + ... +} +``` + +the follow would be illegal syntax (at least for now): + +```rust +unsafe impl<'a, X, Y> Drop for Foo<'a, #[may_dangle] X, Y> { + ... +} +``` - 2. Remove `unsafe_destructor_blind_to_params`, since all uses of it - should be expressible via `unsafe_destructor_blind_to` (once that - has been completely implemented). ## The "eyepatch" attribute -Add a new attribute, `unsafe_destructor_blind_to(ARG)` (the "eyepatch"). +Add a new attribute, `#[may_dangle]` (the "eyepatch"). The eyepatch is similar to `unsafe_destructor_blind_to_params`: it is -attached to the destructor[1](#footnote1), and it is meant +part of the `Drop` implementation, and it is meant to assert that a destructor is guaranteed not to access certain kinds of data accessible via `self`. -The main difference is that the eyepatch has a single required -parameter, `ARG`. This is the place where you specify exactly *what* +The main difference is that the eyepatch is applied to a single +generic parameter: `#[may_dangle] ARG`. +This specifies exactly *what* the destructor is blind to (i.e., what will dropck treat as inaccessible from the destructor for this type). -There are two things one can put the `ARG` for a given eyepatch: one -of the type parameters for the type, or one of the lifetime parameters -for the type.[2](#footnote2) - -### Examples stolen from the Rustonomicon +There are two things one can supply as the `ARG` for a given eyepatch: +one of the type parameters for the type, +or one of the lifetime parameters +for the type. + +When used on a type, e.g. `#[may_dangle] T`, the programmer is +asserting the only uses of values of that type will be to move or drop +them. Thus, no fields will be accessed nor methods called on values of +such a type (apart from any access performed by the destructor for the +type when the values are dropped). This ensures that no dangling +references (such as when `T` is instantiated with `&'a u32`) are ever +accessed in the scenario where `'a` has the same lifetime as the value +being currently destroyed (and thus the precise order of destruction +between the two is unknown to the compiler). + +When used on a lifetime, e.g. `#[may_dangle] 'a`, the programmer is +asserting that no data behind a reference of lifetime `'a` will be +accessed by the destructor. Thus, no fields will be accessed nor +methods called on values of type `&'a Struct`, ensuring that again no +dangling references are ever accessed by the destructor. + +## Require `unsafe` on Drop implementations using the eyepatch + +The final detail is to add an additional check to the compiler +to ensure that any use of `#[may_dangle]` on a `Drop` implementation +imposes a requirement that that implementation block use +`unsafe impl`.[2](#footnote1) + +This reflects the fact that use of `#[may_dangle]` is a +programmer-provided assertion about the behavior of the `Drop` +implementation that must be valided manually by the programmer. +It is analogous to other uses of `unsafe impl` (apart from the +fact that the `Drop` trait itself is not an `unsafe trait`). + +### Examples adapted from the Rustonomicon [nomicon dropck]: https://doc.rust-lang.org/nightly/nomicon/dropck.html @@ -169,8 +270,7 @@ Example of eyepatch on a lifetime parameter:: ```rust struct InspectorA<'a>(&'a u8, &'static str); -impl<'a> Drop for InspectorA<'a> { - #[unsafe_destructor_blind_to('a)] +unsafe impl<#[may_dangle] 'a> Drop for InspectorA<'a> { fn drop(&mut self) { println!("InspectorA(_, {}) knows when *not* to inspect.", self.1); } @@ -184,8 +284,7 @@ use std::fmt; struct InspectorB(T, &'static str); -impl Drop for InspectorB { - #[unsafe_destructor_blind_to(T)] +unsafe impl<#[may_dangle] T: fmt::Display> Drop for InspectorB { fn drop(&mut self) { println!("InspectorB(_, {}) knows when *not* to inspect.", self.1); } @@ -202,8 +301,7 @@ To generalize `RawVec` from the [motivation](#motivation) with an code), we would now write: ```rust -impl Drop for RawVec { - #[unsafe_destructor_blind_to(T)] +unsafe impl<#[may_dangle]T, A:Allocator> Drop for RawVec { /// Frees the memory owned by the RawVec *without* trying to Drop its contents. fn drop(&mut self) { [... free memory using self.alloc ...] @@ -211,7 +309,7 @@ impl Drop for RawVec { } ``` -The use of `unsafe_destructor_blind_to(T)` here asserts that even +The use of `#[may_dangle] T` here asserts that even though the destructor may access borrowed data through `A` (and thus dropck must impose drop-ordering constraints for lifetimes occurring in the type of `A`), the developer is guaranteeing that no access to @@ -232,9 +330,7 @@ If we wanted to generalize this type a bit, we might write: ```rust struct InspectorC<'a,'b,'c>(&'a str, &'b str, &'c str); -impl<'a,'b,'c> Drop for InspectorC<'a,'b,'c> { - #[unsafe_destructor_blind_to('a)] - #[unsafe_destructor_blind_to('c)] +unsafe impl<#[may_dangle] 'a, 'b, #[may_dangle] 'c> Drop for InspectorC<'a,'b,'c> { fn drop(&mut self) { println!("InspectorA(_, {}, _) knows when *not* to inspect.", self.1); } @@ -319,7 +415,7 @@ including when `D` == `InspectorC<'a,'name,'c>`). [prototype]: #prototype pnkfelix has implemented a proof-of-concept -[implementation][pnkfelix prototype] of this feature. +[implementation][pnkfelix prototype] of the `#[may_dangle]` attribute. It uses the substitution machinery we already have in the compiler to express the semantics above. @@ -329,20 +425,15 @@ Here we note a few limitations of the current prototype. These limitations are *not* being proposed as part of the specification of the feature. -1. The eyepatch is not attached to the -destructor in the current [prototype][pnkfelix prototype]; it is -instead attached to the `struct`/`enum` definition itself. - -2. The eyepatch is only able to accept a type -parameter, not a lifetime, in the current -[prototype][pnkfelix prototype]; it is instead attached to the -`struct`/`enum` definition itself. +2. The compiler does not yet enforce (or even +allow) the use of `unsafe impl` for `Drop` implementations that use +the `#[may_dangle]` attribute. Fixing the above limitations should just be a matter of engineering, not a fundamental hurdle to overcome in the feature's design in the context of the language. -[pnkfelix prototype]: https://github.com/pnkfelix/rust/commits/fsk-nonparam-blind-to-indiv +[pnkfelix prototype]: https://github.com/pnkfelix/rust/commits/dropck-eyepatch # Drawbacks [drawbacks]: #drawbacks @@ -359,82 +450,11 @@ could check the assertions being made by the programmer, rather than trusting them. (pnkfelix has some thoughts on this, which are mostly reflected in what he wrote in the [RFC 1238 alternatives][].) -## Attributes lack hygiene -[attributes-lack-hygiene]: #attributes-lack-hygiene - -As noted by arielb1, putting type parameter identifiers into attributes -is not likely to play well with macro hygiene. - -Here is a concrete example: - -```rust -struct Yell2(A, B); - -macro_rules! make_yell2a { - ($A:ident, $B:ident) => { - impl<$A:Debug,$B:Debug> Drop for Yell2<$A,$B> { - #[unsafe_destructor_blind_to(???)] // <---- - fn drop(&mut self) { - println!("Yell1(_, {:?})", self.1); - } - } - } -} - -make_yell2a!(X, Y); -``` - -Here is the issue: In the above, what does one put in for the `???` to -say that we are blind to the first type parameter to `Yell2`? -`#[unsafe_destructor_blind_to(A)` would be nonsense, becauase in the instantiation of the macro, `$A` will be mapped to the identifier `X`. so perhaps we should write it is blind to `X` -- but to me one big point of macro hygiene is that a macro definition should not have to build in knowledge of the identifiers chosen at the usage site, and this is the opposite of that. - -(I don't think `#[unsafe_destructor_blind_to($A)` works, because our attribute system operates at the same meta-level that macros operate at , but I would be happy to be proven wrong.) - ----- - -Despite my somewhat dire attitude above, I don't think this is a significant problem in the short term. This sort of macro is probably rare, and the combination of this macro with UGEH is doubly so. You cannot define a destructor multiple times for the same type, so it seems weird to me to abstract this code construction at this particular level. - - [RFC 1238 alternatives]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md#continue-supporting-parametricity # Alternatives [alternatives]: #alternatives -## unsafe_destructor_blind_to(T1, T2, ...) - -The eyepatch could take multiple arguments, rather than requiring a -distinct instance of the attribute for each parameter that we are -blind to. - -However, I think that each usage of the attribute needs to be -considered, since it represents a separate "attack vector" where -unsoundness can be introduced, and therefore it deserves more than -just a comma and a space added to the program text when it is added. - -(I only weakly support the latter position; it is obviously easy -to support this form if that is deemed desirable.) - -## Use a blacklist not a whitelist -[blacklist-not-whitelist]: #use-a-blacklist-not-a-whitelist - -The `unsafe_destructor_blind_to` attribute acts as a whitelist of -parameters that we are telling dropck to ignore in its analysis -of this destructor. - -We could instead add a way to list the lifetimes and/or -type-expressions (e.g. parameters, projections from parameters) that -the destructor may access (and thus treat that list as a blacklist of -parameters that dropck needs to *include* in its analysis). - -arielb1 first suggested this as an attribute form -[here][blacklist attribute], but then provided a different formulation -of the idea by expressing it as a [`where`-clause][blacklist where] on -the `fn drop` method (which is what I will show in the next section). - -[blacklist attribute]: https://github.com/rust-lang/rfcs/pull/1327#issuecomment-149302743 - -[blacklist where]: https://github.com/rust-lang/rfcs/pull/1327#issuecomment-149329351 - ## Make dropck "see again" via (focused) where-clauses (This alternative carries over some ideas from @@ -480,10 +500,8 @@ avoids a number of problems that the eyepatch attribute has. Advantages of fn-drop-with-where-clauses: - * It completely sidesteps the [hygiene issue][attributes-lack-hygiene]. - - * If the eyepatch attribute is to be limited to identifiers (type - parameters) and lifetimes, then this approach is more expressive, + * Since the eyepatch attribute is to be limited to type and lifetime + parameters, this approach is more expressive, since it would allow one to put type-projections into the constraints. @@ -542,7 +560,7 @@ this RFC rather than waiting for a sound compiler analysis): to write that `T` is parametric (e.g. `T: ?Special` in the [RFC 1238 alternatives]). Even then, we would still need the compiler changes suggested by this RFC, and at that point hopefully the task would be for the programmer to mechanically - replace occurrences of `#[unsafe_destructor_blind_to(T)` with `T: ?Special` + replace occurrences of `#[may_dangle] T` with `T: ?Special` (and then see if the library builds). In other words, I see the form suggested by this RFC as being a step *towards* @@ -558,11 +576,6 @@ If we do nothing, then we cannot add `Vec` soundly. # Unresolved questions [unresolved]: #unresolved-questions -Is there any issue with writing `'a` in an attribute like -`#[unsafe_destructor_blind_to('a)]`? (The prototype, as mentioned -[above](#footnote2), does not currently accept lifetime parameter -inputs, so I do not know the answer off hand. - Is the definition of the drop-check rule sound with this `patched(D)` variant? (We have not proven any previous variation of the rule sound; I think it would be an interesting student project though.) From fd51b8578ede81bef785e2290bd13187ea27185d Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Thu, 23 Jun 2016 23:41:33 +0200 Subject: [PATCH 5/8] added feature gate for attributes on generics --- text/0000-dropck-param-eyepatch.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index c5e5c6eedb0..2a1c7593555 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -1,4 +1,4 @@ -- Feature Name: dropck_eyepatch +- Feature Name: dropck_eyepatch, generic_param_attrs - Start Date: 2015-10-19 - RFC PR: (leave this empty) - Rust Issue: (leave this empty) @@ -19,6 +19,9 @@ and type paramters). Atop that capability, this RFC proposes adding a holds data that must not be accessed during the dynamic extent of that `drop` invocation. +As a side-effect, enable adding attributes to the formal declarations +of generic type and lifetime parameters. + [RFC 1238]: https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md [RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md @@ -140,7 +143,7 @@ storage for [cyclic graph structures][dropck_legal_cycles.rs]). 1. Add the ability to attach attributes to syntax that binds formal lifetime or type parmeters. For the purposes of this RFC, the only place in the syntax that requires such attributes are `impl` - blocks, as in `impl Drop for Type { ... }` + blocks, as in `impl Drop for Type { ... }` 2. Add a new fine-grained attribute, `may_dangle`, which is attached to the binding sites for lifetime or type parameters on an `Drop` @@ -163,6 +166,8 @@ storage for [cyclic graph structures][dropck_legal_cycles.rs]). This is a simple extension to the syntax. +It is guarded by the feature gate `generic_param_attrs`. + Constructions like the following will now become legal. Example of eyepatch attribute on a single type parameter: @@ -212,6 +217,8 @@ unsafe impl<'a, X, Y> Drop for Foo<'a, #[may_dangle] X, Y> { Add a new attribute, `#[may_dangle]` (the "eyepatch"). +It is guarded by the feature gate `dropck_eyepatch`. + The eyepatch is similar to `unsafe_destructor_blind_to_params`: it is part of the `Drop` implementation, and it is meant to assert that a destructor is guaranteed not to access certain kinds From ef11dc096a7d10ae4157f193eddaec0c2314c295 Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Thu, 23 Jun 2016 23:46:52 +0200 Subject: [PATCH 6/8] remove now-meaningless paragraph --- text/0000-dropck-param-eyepatch.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index 2a1c7593555..b72c658d784 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -464,10 +464,6 @@ reflected in what he wrote in the [RFC 1238 alternatives][].) ## Make dropck "see again" via (focused) where-clauses -(This alternative carries over some ideas from -[the previous section][blacklist-not-whitelist], but it stands well on -its own as something to consider, so I am giving it its own section.) - The idea is that we keep the UGEH attribute, blunt hammer that it is. You first opt out of the dropck ordering constraints via that, and then you add back in ordering constraints via `where` clauses. From aa9fbb6db9ba39f62399056cbd502d92c7dd8622 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 11 Jul 2016 12:33:25 +0200 Subject: [PATCH 7/8] add links to RFC PR and tracking issue in Rust repo. --- text/0000-dropck-param-eyepatch.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-dropck-param-eyepatch.md b/text/0000-dropck-param-eyepatch.md index c5e5c6eedb0..d6fe2fbc3be 100644 --- a/text/0000-dropck-param-eyepatch.md +++ b/text/0000-dropck-param-eyepatch.md @@ -1,7 +1,7 @@ - Feature Name: dropck_eyepatch - Start Date: 2015-10-19 -- RFC PR: (leave this empty) -- Rust Issue: (leave this empty) +- RFC PR: [rust-lang/rfcs#1327](https://github.com/rust-lang/rfcs/pull/1327) +- Rust Issue: [rust-lang/rust#34761](https://github.com/rust-lang/rust/issues/34761) # Summary [summary]: #summary From 82afb34bd73ca4c3a555d533d5d25fb8659f60c1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 11 Jul 2016 12:34:12 +0200 Subject: [PATCH 8/8] rename file with RFC PR. --- ...000-dropck-param-eyepatch.md => 1327-dropck-param-eyepatch.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-dropck-param-eyepatch.md => 1327-dropck-param-eyepatch.md} (100%) diff --git a/text/0000-dropck-param-eyepatch.md b/text/1327-dropck-param-eyepatch.md similarity index 100% rename from text/0000-dropck-param-eyepatch.md rename to text/1327-dropck-param-eyepatch.md