Update drop-related stuff based on improvements to the language #35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC @aturon @nikomatsakis @pnkfelix @arielb1 I SUMMON THEE, COUNCIL OF DROPCK |
arielb1
reviewed
Aug 2, 2017
| The general rule for when it's safe to apply the dropck eyepatch to a type parameter | ||
| `T` is that the destructor must only do things to values of type `T` that could be | ||
| done with *all* types. Basically: we can move (or copy) the values around, take | ||
| references to them, get their size/align, and drop them. Just to be clear |
There was a problem hiding this comment.
You can only drop them if you have a PhantomData<T>. A struct Foo<T>(*const T) can't drop a T.
There was a problem hiding this comment.
This is discussed in the next section, but I should emphasize this isn't quite true here
There was a problem hiding this comment.
I would remove the "and drop them" from this list (because it is not valid unconditionally) and add it to the second section.
So the exposition is:
- You can always do "fully parametric" things from the eyepatch.
- But you can't just drop things blindly.
- So if you do want to drop things, add a
PhantomDataand the compiler will make sure you can.
|
Rebased, for #32 |
arielb1
reviewed
Aug 5, 2017
|
|
||
| [Unique]: phantom-data.html | ||
| Should using Option be unacceptable, [`ManuallyDrop`][ManuallyDrop] is always |
There was a problem hiding this comment.
eng: this is awkward. "However, if using an Option is unacceptable..." would be better
arielb1
reviewed
Aug 5, 2017
| At Rust 1.0, these flags were stored in the actual values that needed to | ||
| be tracked. This was a big mess and you had to worry about it in unsafe code. | ||
|
|
||
| As of Rust 1.13, these flags are stored seperately on the stack, so you no |
There was a problem hiding this comment.
"Starting from", rather than "As of". The flags will not go back into actual values in a way you have to care about.
arielb1
reviewed
Aug 5, 2017
| @@ -79,5 +78,8 @@ if condition { | |||
| } | |||
| ``` | |||
|
|
|||
There was a problem hiding this comment.
Maybe add the traditional example of "when drop flags are useful":
let buf;
let result: &str = if let Some(name) = your_name {
buf = format!("Hello, {}", name);
&buf
} else {
"Hello, World!"
};
use(result);
// `buf` is only initialized, and only dropped, on if `name` is Some.
arielb1
reviewed
Aug 5, 2017
| 8 | } | ||
| | ^ `days` dropped here while still borrowed | ||
| | | ||
| = note: values in a scope are dropped in the opposite order they are created |
There was a problem hiding this comment.
This is actually a limitation of lexical lifetimes - inspector lives before days is defined, so because lifetimes form a tree it must live until after days is destroyed. It will be removed the moment we get NLL.
Because "confusing" lifetime/destructor are half "lexical lifetimes are sometimes stupid" and half "deterministic drop order + purely-intraprocedural analysis mean this is actually unsound", and these come with different rules on how to avoid them, I think it's good to note which is which.
arielb1
reviewed
Aug 5, 2017
| is therefore uninteresting. | ||
| * If a type is generic, and doesn't have a destructor, its generic arguments | ||
| must must live *at least* as long as it. | ||
| * If a type is generic, and *does* have a destructor, its generic arguments |
There was a problem hiding this comment.
Please please no "strictly"/"at least" language. That is (A) has no chance of working with NLL and (B) is a bad way of thinking about things. (The "at least" part only exists because of the limitation of lexical lifetimes I've talked about above, it has no relation to soundness).
There was a problem hiding this comment.
The relevant distinction is drop validity. Relevant IRC logs:
Aug 02 00:12:27 <arielby> Gankro: no
Aug 02 00:12:36 <arielby> associated items have to be outlived by the projection
Aug 02 00:12:37 <arielby> so if you have
Aug 02 00:12:42 <arielby> <Foo as Trait<'a>>::Item
Aug 02 00:12:47 <arielby> that has to outlive 'a, not 'static
Aug 02 00:13:44 <Gankro> arielby: if dropck makes sure T doesn't dangle, does it need to care about any projection <T as ???>::???
Aug 02 00:13:59 <arielby> Gankro: no
Aug 02 00:14:07 <Gankro> ok good
Aug 02 00:14:16 <arielby> if dropck needs to make sure a projection doesn't dangle, it makes sure that projection doesn't dangle
Aug 02 00:14:22 <arielby> (e.g. because the projection is a field)
Aug 02 00:14:28 <Gankro> arielby: are you familiar with the details of the eyepatch?
Aug 02 00:14:34 <arielby> Gankro: yeah
Aug 02 00:15:25 <arielby> do you have a question about it?
Aug 02 00:15:30 <Gankro> arielby: is there a case where simply having #[may_dangle] and #[may_dangle_but_will_be_dropped] (covering the case where PhantomData<T> is used) wouldn't be sufficient?
Aug 02 00:15:42 * Thog has quit (Quit: Bye!)
Aug 02 00:15:47 <arielby> sufficient?
Aug 02 00:16:17 <Gankro> arielby: so let's say you have...
Aug 02 00:16:19 <arielby> sufficient for what
Aug 02 00:16:33 <Gankro> struct Foo<#[may_dangle] T> { x: T }
Aug 02 00:16:34 <Gankro> and
Aug 02 00:16:55 <Gankro> struct Foo<#[may_dangle} T: U> { x: <T as U>::Item }
Aug 02 00:17:08 <arielby> the latter is useless
Aug 02 00:17:15 <arielby> I mean, the #[may_dangle] part is useless ATM
Aug 02 00:17:21 <Gankro> arielby: say more?
Aug 02 00:17:44 <arielby> first, #[may_dangle] goes on the *drop impl*
Aug 02 00:17:57 <Gankro> arielby: oh yes sorry, let's say it is
Aug 02 00:17:58 <arielby> and your `Foo` is a "dropck" type even without a drop impl
Aug 02 00:18:14 <arielby> because `<T as U>::Item` might have a destructor
Aug 02 00:18:29 <arielby> a dropck destructor
Aug 02 00:19:11 <arielby> Gankro: actually it's not useleses
Aug 02 00:19:13 * modocache (modocache@moz-a2hth7.j0qm.7vpd.010d.2620.IP) has joined
Aug 02 00:19:18 <arielby> because <T as U>::Item might be normalized in concrete cases
Aug 02 00:19:51 <arielby> the thing dropck is based on is basically a `T is-drop-valid-for 'a` predicate
Aug 02 00:19:58 <Gankro> arielby: is there a case where Foo1 should compile but Foo2 shouldn't?
Aug 02 00:20:10 <arielby> Gankro: yeah
Aug 02 00:20:17 <arielby> Foo<T> for a generic T is a dropck type
Aug 02 00:20:32 <arielby> (Foo<T> is-drop-valid 'a <-> T: 'a)
Aug 02 00:20:34 * infinity0_ has quit (Ping timeout: 121 seconds)
Aug 02 00:20:37 * infinity0 (infinity0@moz-m4o6ba.ecodis.net) has joined
Aug 02 00:20:50 <arielby> rather than
Aug 02 00:20:57 <arielby> Foo<T> is-drop-valid 'a <-> T is-drop-valid 'a
Aug 02 00:21:21 <arielby> Gankro: do you understand the "is-drop-valid" predicate?
Aug 02 00:21:29 <Gankro> arielby: I don't think so
Aug 02 00:21:32 <arielby> it's basically a weakening of the outlives predicate
Aug 02 00:21:39 <arielby> normally, in Rust, before you call a function on a type
Aug 02 00:21:46 <arielby> you must prove that the type outlives the current callsite
Aug 02 00:21:48 <arielby> right?
Aug 02 00:22:03 <Gankro> yes
Aug 02 00:22:14 <arielby> and "outlives" is a "syntactic" rule based on the type
Aug 02 00:22:22 <arielby> yes?
Aug 02 00:22:36 <arielby> so `&'a u32: 'a` but not `&'a u32: 'static`
Aug 02 00:22:42 <arielby> yes?
Aug 02 00:22:47 <arielby> that's the "RFC1214 outlives"
Aug 02 00:22:47 <Gankro> arielby: in that it just checks the generic params?
Aug 02 00:22:57 <arielby> Gankro: yeah
Aug 02 00:23:06 <Gankro> ok sure
Aug 02 00:23:15 * modocache has quit (Ping timeout: 121 seconds)
Aug 02 00:23:16 <arielby> now, that can't work for drops
Aug 02 00:23:25 <arielby> you want to drop an `&'a u32` local outside of `'a`
Aug 02 00:23:32 * modocache (modocache@moz-oce7o3.j0qm.7vpd.010d.2620.IP) has joined
Aug 02 00:23:35 <arielby> right?
Aug 02 00:23:36 * Thog (Thog@moz-llidlj.rev.poneytelecom.eu) has joined
Aug 02 00:23:40 <Gankro> arielby: well, on the boundary, but sure
Aug 02 00:23:49 <arielby> Gankro: in MIR there's no "on the boundary"
Aug 02 00:24:00 <arielby> there's an EndRegion for the scope and then a drop
Aug 02 00:24:06 <Gankro> ok
Aug 02 00:24:16 <arielby> (also in pre-MIR, but it's a little bit less explicit)
Aug 02 00:24:46 <arielby> so we want a predicate, which I'll call `is-drop-valid`, which is a weaker version of outlives
Aug 02 00:24:55 <arielby> and tells us that dropping is valid (even if calling an arbitrary function isn't)
Aug 02 00:25:12 <arielby> of course, if a drop is a no-op, then a type is always drop-valid, even if it is not in-scope in any way
Aug 02 00:25:25 <arielby> &'a u32 is-drop-valid 'static
Aug 02 00:25:36 * nagisa has quit (Ping timeout: 121 seconds)
Aug 02 00:25:46 <arielby> right?
Aug 02 00:25:48 <Gankro> right
Aug 02 00:26:04 <arielby> so the "user-facing" part of dropck is when a type is drop-valid related to the lifetimes within it
Aug 02 00:26:28 <Gankro> sounds good
Aug 02 00:27:10 <arielby> niko uses "in-scope" to mean "outlives the current lifetime"
Aug 02 00:27:22 <arielby> so `T in-scope` <-> `T outlives 'current`
Aug 02 00:27:28 <arielby> (that's a shorthand)
Aug 02 00:27:38 * modocache has quit (Ping timeout: 121 seconds)
Aug 02 00:27:41 <arielby> if a type is drop-valid iff it is in-scope, then it's a "dropck type"
Aug 02 00:27:47 <arielby> and trying to create cycles with it will fail
Aug 02 00:27:52 <arielby> from a user POV
Aug 02 00:28:02 * ubsandroid has quit (Ping timeout: 121 seconds)
Aug 02 00:28:27 <SimonSapin> are unbound lifetimes effectively the same as 'static, because of lifetime subtyping?
Aug 02 00:28:34 <arielby> SimonSapin: unbound lifetimes?
Aug 02 00:28:49 <Gankro> arielby: foo<'a>() -> &'a u32
Aug 02 00:28:55 <SimonSapin> yes
Aug 02 00:29:20 <arielby> that's not a type "because of RFC 447/i32330"
Aug 02 00:29:25 <arielby> it's a type-scheme
Aug 02 00:29:57 <arielby> but it can be used as an `foo() -> &'static u32`
Aug 02 00:30:29 <arielby> and a `foo() -> &'static u32` "can be used as a" `foo<'a>() -> &'a u32`
Aug 02 00:30:36 <arielby> in the cases I know of
Aug 02 00:30:38 <arielby> why are you asking?
Aug 02 00:30:49 <Gankro> arielby: what's an example of a dropck type
Aug 02 00:31:09 <arielby> Gankro: Vec<(), Allocator<'a>>
Aug 02 00:31:15 <arielby> it accesses the allocator during its destructor
Aug 02 00:31:28 <Gankro> ok
Aug 02 00:31:38 <arielby> also, Box<Trait+'a>
Aug 02 00:31:43 <arielby> because we have no idea what the destructor does
Aug 02 00:31:48 <Gankro> fair
Aug 02 00:32:12 <arielby> so the dropck RFCs are basically around how we determine whether a type is drop-valid
Aug 02 00:32:24 <arielby> whether and when
Aug 02 00:32:42 <arielby> and the eyepatch rule is
Aug 02 00:32:54 * modocache (modocache@moz-l4avvk.j0qm.7vpd.010d.2620.IP) has joined
Aug 02 00:33:01 <arielby> first, a type is dropck valid only if all of its fields are dropck-valid
Aug 02 00:33:06 <arielby> (because dropping it drops its fields)
Aug 02 00:33:10 <arielby> that applies for all types
Aug 02 00:33:31 <arielby> second, if a type has a safe drop impl, it is dropck valid only if it is in-scope
Aug 02 00:33:55 <arielby> because the safe drop impl can call "everything", which can assume that the type is in scope
Aug 02 00:34:14 * pcwalton (pcwalton@moz-m5ejvh.sfo1.mozilla.net) has joined
Aug 02 00:34:14 * ChanServ gives channel operator status to pcwalton
Aug 02 00:34:15 <arielby> third, if the type has #[may_dangle], the type author can decide what they want to assume
Aug 02 00:34:50 <arielby> "I want to be able to assume that my allocator is in-scope, but I only care that my field is drop-valid"
Aug 02 00:35:01 <Gankro> arielby: the fields are still queried even if you may_dangle everything
Aug 02 00:35:10 <arielby> Gankro: IIRC yeah
Aug 02 00:35:45 <arielby> yeah
Aug 02 00:36:05 <arielby> e.g. for `Vec<T>` you *do* want `T` to be drop-valid
Aug 02 00:36:19 <arielby> (while for something like Ref<T> you don't care whether `T` is drop-valid at all)
Aug 02 00:37:01 * modocache has quit (Ping timeout: 121 seconds)
Aug 02 00:37:12 <arielby> any more questions?
Aug 02 00:37:15 <arielby> I need to write this somewhere
Aug 02 00:38:44 * sleffy (sleffy@moz-6h8duu.ca.comcast.net) has joined
Aug 02 00:39:00 <Gankro> arielby: so I'm trying to decide if the "you (potentially) need PhantomData if you use #[may_dangle]" rule can be replaced with just having #may_dangle_also_i_drop_this to explicitly declare that
Aug 02 00:39:33 * modocache (modocache@moz-3aegnv.j0qm.7vpd.010d.2620.IP) has joined
Aug 02 00:40:13 <Gankro> arielby: and I think you can't, because you could have <`&'a T` as Printer>::IPrintThisWhenIDrop that you need to tell the compiler about?
Aug 02 00:41:10 * htmldrum (htmldrum@moz-rbu.hbi.152.120.IP) has joined
Aug 02 00:41:51 * Diggsey (uid120933@moz-773k9v.hathersage.irccloud.com) has joined
Aug 02 00:42:06 <arielby> Gankro: sure
Aug 02 00:42:11 <arielby> if you have
Aug 02 00:42:15 * rkruppe has quit (Quit: ChatZilla 0.9.93 [Firefox 54.0/20170613080547])
Aug 02 00:42:18 <arielby> Foo(*const T)
Aug 02 00:42:36 <arielby> then a #[may_dangle_also_i_drop_this] will work
Aug 02 00:42:38 <arielby> but if you have
Aug 02 00:42:44 <arielby> Foo(*const <T as Foo>::Bar)
Aug 02 00:42:45 <arielby> you need
Aug 02 00:42:50 <arielby> also_i_drop(<T as Foo>::Bar)
Aug 02 00:43:34 * modocache has quit (Ping timeout: 121 seconds)
Aug 02 00:48:47 * Tidurrr has quit (Client exited)
Aug 02 00:50:33 * htmldrum has quit (Ping timeout: 121 seconds)
Aug 02 00:51:42 <Gankro> arielby: hrm, that's disappointing
Aug 02 00:52:22 <arielby> yea
Aug 02 00:52:35 <Gankro> arielby: it feels like all your dropck annotations should be together
Aug 02 00:52:39 <arielby> yea
Aug 02 00:53:05 <arielby> if we can reify the drop-valid-at predicate
Aug 02 00:53:11 <arielby> then you could have
Aug 02 00:53:37 <arielby> impl<'a, #[may_dangle] T> Drop('a) for Foo<T> where T: Drop('a)
Aug 02 00:53:57 <arielby> where Drop('a) is the reification
Aug 02 00:54:11 * jdm has quit (Quit: )
Aug 02 00:54:13 <Gankro> arielby: and then it could be T::Item: Drop('a) ?
Aug 02 00:54:18 Gankro gandro geofft ggherdov_ gjnoonan gkholkar gsingh93 Guest28739 gus
Aug 02 00:54:20 <arielby> Gankro: yeah
Aug 02 00:54:31 <arielby> or maybe just have `T: is Drop` as the reification?
Aug 02 00:54:32 <arielby> so
Aug 02 00:54:44 <arielby> impl<#[may_dangle] T> Drop for T where T::Item: Drop
Aug 02 00:54:47 * pkmoore has quit (Ping timeout: 121 seconds)
Aug 02 00:54:58 <arielby> or "is Drop", because "T::Item: Drop" has a (useless) meaning
Aug 02 00:55:28 <Gankro> arielby: also isn't this necessary if your destructor uses specialization anyway? Like you literally can't say in the contents whether you drop T::Item
Aug 02 00:55:45 <arielby> Gankro: don't use specialization and lifetimes
Aug 02 00:55:47 <arielby> it doesn't work
Aug 02 00:55:53 <Manishearth> eddyb: is removing that match arm a valid fix ?
Aug 02 00:56:26 <eddyb> Manishearth: no
Aug 02 00:56:27 <Gankro> arielby: no say it's <'a, T: 'a>, and you specialize for T: PrintOnDrop
Aug 02 00:56:32 <eddyb> Manishearth: you have to make it => None
Aug 02 00:56:39 <eddyb> Manishearth: I think if you remove it you get an ICE or something
Aug 02 00:56:47 <arielby> Gankro: then you have a `T: 'a` req on the type
Aug 02 00:56:47 <eddyb> Manishearth: lemme double check
Aug 02 00:56:55 <arielby> so you don't need any other req
Aug 02 00:57:04 <arielby> as long as you don't `#[may_dangle] 'a` anywhere
Aug 02 00:57:25 <eddyb> Manishearth: oh it goes to the default case with no elision
Aug 02 00:57:38 <eddyb> Manishearth: anyway has to be => None
Aug 02 00:58:43 <Manishearth> eddyb: er, i meant None-ing it sorry :)
Aug 02 00:59:10 <eddyb> Manishearth: yeah
Aug 02 00:59:17 <eddyb> Manishearth: it's what happens for trait methods
Aug 02 00:59:49 <arielby> Gankro: so if we had `T: @Drop` we wouldn't need PhantomData for dropck
Aug 02 01:03:46 <Gankro> arielby: would it still make sense to look at fields just to infer those when possible?
Aug 02 01:04:00 <arielby> Gankro: for safe destructors we have to
Aug 02 01:04:05 <arielby> and I think we'll do it for unsafe types too
Aug 02 01:10:11 * harrisonclarke (harrisoncla@moz-7vs.bl6.251.207.IP) has joined
Aug 02 01:10:26 <Gankro> arielby: I thought for safe destructors it becomes syntactic
Aug 02 01:10:43 <arielby> Gankro: I meant, for "no destructors"
Aug 02 01:10:46 <arielby> for types with no dtors
Aug 02 01:10:48 <Gankro> Right ok
Aug 02 01:11:12 <Gankro> And then associated items are irrelevant
Aug 02 01:11:14 <arielby> OTOH, the fields are going to be dropped "by default"
Aug 02 01:11:29 <arielby> so we still want to force them to be live
Aug 02 01:11:51 <Gankro> arielby: right but that's just recursively validating that type, and doesn't involve the outter type at all?
Aug 02 01:12:04 <arielby> Gankro: what do you mean
Aug 02 01:12:10 <arielby> if you have Vec<T>(T::Item)
Aug 02 01:12:14 <arielby> it will look just at T::Item
Aug 02 01:12:19 <arielby> it won't start randomly looking at traits
Aug 02 01:12:39 <arielby> or "non-randomly" looking at traits
Aug 02 01:12:41 <arielby> just at fields
Aug 02 01:13:01 <Gankro> arielby: basically I mean validation NoDtor<T>(T) is the same as validating T on its own
Aug 02 01:14:02 <arielby> yeah
Aug 02 01:14:29 <Gankro> arielby: ok, thanks for the chat, g2g!
Aug 02 01:14:47 <Gankro> arielby: would you be interested in reviewing nomicon stuff about this?
Aug 02 01:14:52 <Gankro> (still writing)
Aug 02 01:14:55 <arielby> sure
Aug 02 01:14:58 <Gankro> dope
Aug 02 01:15:41 <arielby> ping/mail me when it's ready
Aug 02 01:16:14 * ubsandroid (ubsan@moz-405htv.8c04.7k6a.fb90.2607.IP) has joined
**** ENDING LOGGING AT Wed Aug 2 01:17:52 2017
|
Hey thanks for the feedback! It seems like concepts from non-lexical lifetimes will be important here, so I'm gonna sit on correcting this until that's further along, and I have a better understanding. Been bitten by trying to preempt "the future" too many times. |
arielb1
reviewed
Aug 5, 2017
| own structure, drop all of the `T`'s they contain, and free themselves. This is | ||
| why it's sound for them to apply the eyepatch to their parameters. | ||
|
|
||
|
|
|
@gankro whats up with this PR? |
|
wow i don't remember this pr very well! uhh I think I'm still waiting on NLL's to get more well-defined? |
|
Now that NonNull is “riding the trains”, could that part of this revision be put in a new commit? |
|
I'm going to close this PR until nll stuff is ready to go. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
*mut, as Unique is unlikely to be stabilized and unnecessary for safety