-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Dropck Eyepatch RFC. #1327
Dropck Eyepatch RFC. #1327
Conversation
attributes taking IDENTIFIERS as parameters? how unhygienic are you willing to go? /justjoking. Anyway, it would be nice to put arbitrary types (most importantly, projections) in the attribute (but this requires it to be a blacklist instead of a whitelist): trait TypeContext {
type Arena: Copy + Allocator;
type Ty: Copy + TypeFoldable<Self>;
type SubstRef: Copy + TypeFoldable<Self>;
fn subst<T: TypeFoldable<Self>>(t: T, s: self::SubstRef) -> T;
}
struct TyList<TCX: TypeContext> {
d: Unique<[TCX::Ty]>,
a: TCX::Arena
}
impl<TCX: TypeContext> Drop for DataSlice<C> {
#[unsafe_destructor_accesses_only(TCX::Arena)]
fn drop(&mut self) {
unsafe { self.a.deallocate(d) };
}
} |
I didn't really read the RFC throughly. I would prefer to add |
Actually, the "structural-outlives" is just me misinterpreting the RFC. It still could be clearer about that. |
It may be cleaner (but less ergonomic) to just allow putting where-clauses (of course, clauses implied by impl<T, A:Allocator> Drop for RawVec<T, A> {
#[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 {
[... free memory using self.alloc ...]
}
} OTOH, this won't really work because of early vs. late-bound lifetimes. |
This is actually a fairly significant drawback to the approach. |
Except for the hygiene and projection problems, the "negative-sense" approach is nicer ergonomically. However, I think the "positive-sense" approach is much cleaner from a semantic standpoint. It meshes nicely with RFC1214. A model for dropckTerminology: outlives and alive
Problem statementWe are mainly interested in variants of the following example:
We have to regionck this code, which means assigning concrete regions to 'λ and 'μ. In this example, we can basically let them be equal without losing generality. Naïvely, we might think we could replace Another way of seeing this is that because the references are cyclic, one of the destructors will be called after the destructor was called on the other. dropck and the escape hatchThe observation that allows dropck to work is that many destructors don't really care about the value they hold. For example, the drop glue for references, being completely trivial, runs just as well if the reference is dead. We basically want a Obviously, the drop glue for types with a arbitrary user-defined destructor calls On the other hand, the drop glue for types without destructors just calls the drop-glue of their contents. We can therefore extend The first interesting case is types, like The second case is what this RFC is about, types that require their content to be destruction-valid, but in addition require some type parameter, for example an allocator, to be alive. These are a combination of the previous cases, and behave similarly. dealing with dead typesWhile dead type parameters are certainly dangerous, it is still possible to write sound generic programs that deal with them. I will discuss this tomorrow. |
dealing with dead typesThe primary soundness issue raised by dropck is that functions (the destructor itself, and potentially generic functions called by it) are called with types and lifetimes that are not actually alive. Now lifetimes that are not alive are very dangerous - a reference with that lifetime points to dead data, and accessing it will cause UB and bugs that are hard to debug and easy to exploit. Types that are not alive, however, are only dangerous because of the dead lifetimes they can contain. As the only way a lifetime can be extracted from a type in Rust is through a function call (even associated types can't do it!), we only need to ensure these are safe. For this, we differentiate between functions that are still accessible from when the type was alive and newly-created functions. The function pointers that were accessible from when type was alive are certainly a problem, as was discovered in rust-lang/rust#26656. Because of the way type parameters work, they are only dangerous if their signature contains dead types (informally, dead generic parameters can be "truncated" back to life - but this is still a problem without dropck, though it would be nice if someone formalized it), so you must ensure no old function with such signature is called. A similar problem would exist with trait objects, except that the current formulation of destruction-validity forces them to be alive. The other problematic case is methods from trait-refs implied by where-clauses in the destructor ( Free functions and methods that come from trait-refs selected by impls can be proven safe inductively |
Currently all lifetime and type parameters are alive, right? In the last RFC I proposed doing analogous to impl<T: ?Alive, A: Allocator> Drop for RawVec<T, A> {
fn drop(&mut self) {
[... free memory using self.alloc ...]
}
} Unless I am missing something, this gives us nice semantics, no hygiene problem, and the better ergonomics of a dead list. Win! |
Tagging as T-lang and assigning to pnkfelix (but feel free to correct!) |
We already have |
So, I find aspects of the alternatives proposed in these comments appealing. In particular, in the long term I would like us to support something like @Ericson2314 's There is still the question of whether we wait for such a thing to be implemented, or if we accept something dirtier like (If I was sure that I could implement @arielb1 's |
Glad to here you like it! If we go the temporarily-dirty route, it might be convenient to still use my syntax to avoid the hygiene issues. |
@Ericson2314 I would only want to adopt your syntax when we actually enforce the constraint it implies, these other short term approaches all have the word "unsafe" in an attribute name because they are not doing the analysis that I believe |
Well, can use a different identifier, |
So it's taken me a while to read the comments on this RFC. My feeling is that, like @pnkfelix, I like the However, when we tried to dig into just what limitations we should impose, it seemed harder. Part of this was that we were attempting to permit helper fns that could also use This is not to say I don't like the With respect to @arielb1's suggestion, I feel less good. Partly this is because that suggestion seems to dial back to maximum unsafety (using the attribute) and then let you layer safety back on, versus the original proposal, which degrades from safe down to more unsafe. I guess that if we were going to make this a safe check, I might feel differently -- that is, perhaps we could fashion something where you add Certainly the problem of hygiene is real, though i'm not sure how real. The hygiene algorithm does tag all identifiers (afaik) with colors and so on, so it could be that we could make the "names in attributes" things work. In any case, longer term, if we did adopt an attribute-based solution, I feel like it'd be nicer to annotate the type parameter declaration EDIT: Minor tweaks. |
So I looked into adding (The rest of this comment is basically an elaboration of that last sentence. In other words, I'm spelling out an example of a potential misgiving that @nikomatsakis alluded to above.) In the discussion here, I'm going to make some assumptions about what In the long term, the reason I see for machinery like Consider a case like this: struct Pair<T>(T, fn(&T) -> String);
#[cfg(sound_helper)]
impl<T: ?Alive> Pair<T> {
fn call_it(&self) {
println!("Pair says Hi");
}
}
impl<T: ?Alive> Drop for Pair<T> {
fn drop(&mut self) {
self.call_it();
}
} The above is an example of sound code. We would like to be able to structure code into helpers like this and to have the compiler check that the helpers are sound. Now consider: #[cfg(unsound_helper)]
impl<T: ?Alive> Pair<T> {
fn call_it(&self) {
println!("Pair says {}", self.1(&self.0));
}
} My objective is to have a system that would reject the above program: the callback held in The question: Is I had thought the answer was "yes, it is reasonable." But that was when I thought we would somewhat magically get the checks we desired out of our existing machinery ... In particular, I had hoped that just the presence of the bound footnote: Perhaps I am being too naive or narrow-minded; i.e. maybe extending the type-checker to ensure that if
|
On a totally separate note: this morning I thought of a hack to address the hygiene issue: in addition to allowing the attribute to refer to the types by name, also allow the attribute to reference a type by its index in the generic parameter list. E.g.: the impl<T: fmt::Display> Drop for InspectorB<T> {
#[unsafe_destructor_blind_to(0)]
fn drop(&mut self) {
println!("InspectorB(_, {}) knows when *not* to inspect.", self.1);
}
} (Ugly, you say? Well, yes: But the above syntax is not required outside of a macro body, so its use should be very rare.) But what about lifetimes, I hear you cry? Yeah, I didn't think of them either until I reviewed the RFC to find the example above. Well, we could do this (here taking the 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('0)]
#[unsafe_destructor_blind_to('2)]
fn drop(&mut self) {
println!("InspectorA(_, {}, _) knows when *not* to inspect.", self.1);
}
} (Boy if you thought the first example was ugly, I bet your eyes are melting now.) |
I think the hygiene issues, while an interesting point, may be overblown. IMO the best way to do this attribute would be to attach it to the type parameter itself (As an aside, It's worth pointing out that we do not treat type parameter names hygienically now: http://is.gd/XnMSQs, so this is all in some theoretical universe where that is fixed, and yet where we cannot make the macro system aware of identifiers in attributes for some reason.) Even then, there is no nesting of definitions, so to have a "false equality" you would have to have a macro that is generating the contents of the attribute (i.e., the macro author wrote I think all i'm saying here is that using indices feels like overkill, if we want to take this approach we should just adopt it now, but plan on replacing it with attributes on the type parameters themselves. If for some reason we can't make that possible, then we can debate the hygiene question. |
We said this on IRC, but I wanted to write it down for the record. When I last entertained the idea of |
@pnkfelix Three things come to mind:
|
Is there any connection between |
@glaebhoerl don't remember that / wasn't there for it, but going by the name, it might be related. As I understand it:
Because lifetimes are erased, it is fine to instantiate all |
@pnkfelix had a thought I wanted to jot down. Assuming we opt for an attribute here like |
So I finally had a chance to sit down and work on this some more. I have prototyped a new version that still uses attributes, but attaches them directly to the generic type parameter, like so: unsafe impl<#[may_dangle] X, Y: fmt::Debug> Drop for Foo<X, Y> {
fn drop(&mut self) { /* cannot access contents of `X`, but may access contents of `Y` */ }
} (Well, its almost at the point of the above; I haven't implemented the part where it requires an |
(where `ARG` is either a formal lifetime parameter `'a` or a formal type parmeter `T`).
TODO: the new kind of attribute (on generic parameter binding sites) should have its own, separate, feature gate. |
Is there any fundamental reason that this needs to be handled via an attribute as opposed to an additional bound? As in, I am not sure if "parametric" is the right term here. Parametricity is a relational property, i.e. it is expressed in terms of how |
This is still supposed to be a temporary fix, right? |
@RalfJung my take is that when its a local assertion where the programmer is responsible for checking the correctness w.r.t. the actual implementation of the
Then I am left with a choice between Why do I prefer an attribute for right now?
|
@Ericson2314 : yes, this is entirely meant to be temporary. Hopefully we can independently work on a |
I agree.
Thanks. Maybe the temporary nature of this attribute should be stated in the RFC? |
The lang team is bringing this RFC into its final comment period 💬 To reiterate some of the recent discussion, the proposal in this RFC is intended as a temporary solution (along the lines of |
Huzzah! The @rust-lang/lang team has decided to accept this RFC. Again we stress that the solution described here is temporary and will not be stabilized as-is. |
Can anyone explain in layman's terms why this (or the previous "blunt attribute") is needed? I don't think the RFC properly explains that. |
I'm not sure it counts as a layman's explanation, but there was some really good discussion of this and related issues in this internals thread, and the other discussions linked from there. In particular this comment by @RalfJung, and the ensuing discussion about drop-desugarings, were the first time I felt like I had gotten an idea of what all this is fundamentally about. |
@glaebhoerl Appreciate the links, but I still don't get the basic idea I'm afraid. Why do we want to allow "dead" types and lifetimes? What can we then do with them? And why specifically in a |
if anyone is reading the dialogue above and is similarly confused about the underlying motivation, I think the previous RFCs did dedicate significant space to explaining why you want to allow types with dangling references. First, the original Sound Generic Drop RFC (RFC 769) explained why some destructors would access borrowed data: The same RFC noted, as part of its drawbacks, that it did not include a way for a type to specify explicitly (as part of its API) that its borrowed data was not accessed from its destructor: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#drawbacks Then, the followup Non-Parametric Drop Check RFC (RFC 1238) explained why some types should allow one to assert that their borrowed data is never accessed from their destructors: |
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.
unsafe impl
requirementrendered