Stream: t-compiler/wg-nll

Topic: #56754 re-triaging NLL-deferred issues


pnkfelix (Dec 19 2018 at 14:58, on Zulip):

Well, its taken me a week to get around to it, but I'm going to spend the next ~hour doing NLL-deferred triaging

pnkfelix (Dec 19 2018 at 14:58, on Zulip):

I'll log my actions here. Oh I guess I'll notify @WG-compiler-nll about it, too.

pnkfelix (Dec 19 2018 at 15:00, on Zulip):

Here is a link the work queue I'll be using. Wow, 21 open issues; I'm so used to the hundreds of issues open for T-compiler, this looks like a cake walk. :smile:

pnkfelix (Dec 19 2018 at 15:01, on Zulip):

'"lifetime of borrowed pointer outlives lifetime of captured variable" error message very unclear' #42574

pnkfelix (Dec 19 2018 at 15:02, on Zulip):

This was marked as NLL-deferred just to put it on some work queue. It is an NLL-fixed-by-NLL, but it requires full #![feature(nll)] (not just migration mode).

pnkfelix (Dec 19 2018 at 15:04, on Zulip):

Nonetheless, the main work item remaining here (I think) is to put in tests for the behavior in our various borrowck modes (ast, migrate, mir). Its already tagged with E-needstest. As a matter of overall policy, I think the task of making the tests in a case like this (diagnostic quality) is P-low (though if someone wants to argue for P-medium, I'm all ears.)

pnkfelix (Dec 19 2018 at 15:05, on Zulip):

In any case I'm going to mark it P-low and assign it to myself, since it seems like a good task for zombie felix to do

pnkfelix (Dec 19 2018 at 15:07, on Zulip):

next: "Double-indirection with Cow causes inconsistent behavior of the borrow checker" #43058

pnkfelix (Dec 19 2018 at 15:10, on Zulip):

This is already tagged with E-needstest (and E-mentor, since @nikomatsakis put in some mentorship notes). It was basically an expressiveness issue (i.e. it would be NLL-complete if NLL didn't actually fix it. But it does fix it.). I'm going to add NLL-fixed-by-NLL. And assign it to myself. and mark it P-low, by same logic that I used in #42574

pnkfelix (Dec 19 2018 at 15:11, on Zulip):

next: "error "cannot bind by-move into a pattern guard" (+ others) is too imprecise and should be removed in favor of MIR borrowck" #45600

pnkfelix (Dec 19 2018 at 15:13, on Zulip):

this is now fixed by NLL, which (when enabled) suggests adding #![feature(bind_by_move_pattern_guards)] when running the given test (and then when you add that, the test compiles and runs).

pnkfelix (Dec 19 2018 at 15:23, on Zulip):

ah the same issue had a second example that argued that we should allow by-move and by-ref patterns in the same arm

pnkfelix (Dec 19 2018 at 15:23, on Zulip):

(or should I say "match candidate" more precisely? anyway...)

pnkfelix (Dec 19 2018 at 15:24, on Zulip):

tagging with NLL-complete, P-medium, for the second example in the issue.

pnkfelix (Dec 19 2018 at 15:25, on Zulip):

(though the check in question may not even live in NLL. I.e. this might be trivial to fix, outside of the NLL code base. Not sure yet.)

pnkfelix (Dec 19 2018 at 15:25, on Zulip):

next: "match arm bindings have weird lifetimes" #46525

pnkfelix (Dec 19 2018 at 15:26, on Zulip):

I don't remember what effect my changes to match had on cases like the one in this issue.

pnkfelix (Dec 19 2018 at 15:26, on Zulip):

I know that @Ariel Ben-Yehuda has written notes elsewhere on similar oddities with how match bindings are handled.

pnkfelix (Dec 19 2018 at 15:27, on Zulip):

Certainly seems like I'm the right person to address this, at least in the short term.

pnkfelix (Dec 19 2018 at 15:27, on Zulip):

tagging P-medium and assigning to self.

pnkfelix (Dec 19 2018 at 15:28, on Zulip):

oh and in terms of the tags... I guess since this is getting into details of the code we generate for a given match expression, the most relevant NLL-tag is NLL-complete. I'll tag it with that.

pnkfelix (Dec 19 2018 at 15:29, on Zulip):

(If someone wants to argue that #46525 should be given P-high priority, I'm open to hearing an argument for that.)

pnkfelix (Dec 19 2018 at 15:29, on Zulip):

next "MIR borrowck doesn't accept the example of iterating and updating a mutable reference" #46589

pnkfelix (Dec 19 2018 at 15:30, on Zulip):

this is one of the cases that @davidtwco has been iterating on.

pnkfelix (Dec 19 2018 at 15:31, on Zulip):

I'm personally not convinced that the remaining work here merits P-high. Its certainly nice to have, but in terms of triage, I'd call it P-medium.

pnkfelix (Dec 19 2018 at 15:31, on Zulip):

and its obviously NLL-complete.

pnkfelix (Dec 19 2018 at 15:32, on Zulip):

next: "figure out how to integrate constants and the MIR type checker" #46702

pnkfelix (Dec 19 2018 at 15:32, on Zulip):

I don't like how many question marks I have attached to my mental model of this issue.

pnkfelix (Dec 19 2018 at 15:33, on Zulip):

(I also am not sure how @davidtwco 's other work interacts here.)

pnkfelix (Dec 19 2018 at 15:34, on Zulip):

In any case, since the worry is about failing to enforce constraints from lifetime annotations, I'm thinking that makes this NLL-sound. The only question is whether it is P-high or P-medium.

pnkfelix (Dec 19 2018 at 15:35, on Zulip):

call me crazy but I think given our migration strategy, identifying the severity of the problem here (if any) sounds like a P-high problem to me.

pnkfelix (Dec 19 2018 at 15:36, on Zulip):

Unless of course the code that is accepted by this was also accepted by AST-borrowck...

pnkfelix (Dec 19 2018 at 15:37, on Zulip):

I'm going to tag this as P-high and nominate it for discussion at tonight's @WG-compiler-nll meeting, with the intent that someone should get assigned the task of evaluating whether this is indeed a problem or not.

davidtwco (Dec 19 2018 at 15:38, on Zulip):

@pnkfelix I don't think there is a meeting tonight, the calendar invite was cancelled by @nikomatsakis due to holidays.

pnkfelix (Dec 19 2018 at 15:38, on Zulip):

Oh I thought that was for next weeks' meeting

davidtwco (Dec 19 2018 at 15:38, on Zulip):

Unless that was for a date sometime in future and I misread.

pnkfelix (Dec 19 2018 at 15:38, on Zulip):

let me look

pnkfelix (Dec 19 2018 at 15:39, on Zulip):

meeting tonight is still on my calendar

pnkfelix (Dec 19 2018 at 15:39, on Zulip):

next week's is not on my calendar

davidtwco (Dec 19 2018 at 15:39, on Zulip):

Yeah, you're right, my bad. Should have read that closer.

davidtwco (Dec 19 2018 at 15:40, on Zulip):

this is one of the cases that @davidtwco has been iterating on.

This has a waiting-on-bors PR.

davidtwco (Dec 19 2018 at 15:40, on Zulip):

(I also am not sure how @davidtwco 's other work interacts here.)

I don't know if it does, I've not looked at that issue at all - only messed with constants w/r/t type annotations.

pnkfelix (Dec 19 2018 at 16:04, on Zulip):

next: "Is an InlineAsm output a potential use that needs to activate a borrow?" #46891

pnkfelix (Dec 19 2018 at 16:04, on Zulip):

the running theory here is that its impossible to actually run into the relevant case here under our "limited" definition of two-phase borrows.

pnkfelix (Dec 19 2018 at 16:05, on Zulip):

Resolving the questions here strikes me as NLL-sound. But my understanding is that inline-asm is still an unstable feature. So it can be P-medium rather than P-high, IMO.

pnkfelix (Dec 19 2018 at 16:07, on Zulip):

next: "two-phase-borrows need a specification" #46901

pnkfelix (Dec 19 2018 at 16:07, on Zulip):

i'm going to mark this P-high and assign it to self. Documenting two-phase borrows would be a good side-project as I work through issue #56254

pnkfelix (Dec 19 2018 at 16:08, on Zulip):

I don't know if it really falls into any of our P-buckets NLL-buckets.

pnkfelix (Dec 19 2018 at 16:09, on Zulip):

but since its P-high and I'll do it, I won't worry about the NLL-bucket

pnkfelix (Dec 19 2018 at 16:12, on Zulip):

next: "NLL / MIR-borrowck regression in getopts" #48225

pnkfelix (Dec 19 2018 at 16:16, on Zulip):

Seems NLL complete. Also seems to work now. Assigned to self, P-medium, E-needstest.

pnkfelix (Dec 19 2018 at 16:18, on Zulip):

next: "Tracking issue for generalized two-phase borrows" #49434

pnkfelix (Dec 19 2018 at 16:18, on Zulip):

wrote a comment linking the other two aforementioned 2PB issues

pnkfelix (Dec 19 2018 at 16:19, on Zulip):

and marking as NLL-complete, P-medium. (Since I don't think generalized 2PB is terribly high priority. This might even be P-low....)

pnkfelix (Dec 19 2018 at 16:20, on Zulip):

next: "rustc panics on compile-fail/borrowck/two-phase-reservation-sharing-interference" #50128

pnkfelix (Dec 19 2018 at 16:21, on Zulip):

Hmm it seems like this was "resolved" by PR #49891 in that it removed the nll_beyond revision from the test in question.

pnkfelix (Dec 19 2018 at 16:21, on Zulip):

I think I'll just close this one then.

pnkfelix (Dec 19 2018 at 16:23, on Zulip):

next: "consider fixing common regression with expansion of 2-phase borrows" #51915

pnkfelix (Dec 19 2018 at 16:23, on Zulip):

I'll have this CC #49434 (since I figure that's a decent umbrella issue for any generalization of 2PB). Otherwise, its P-medium, NLL-complete (which it already has as a tag.)

pnkfelix (Dec 19 2018 at 16:24, on Zulip):

next: "Two-phase borrows don't allow mutably splitting an array based on its length" #53723

pnkfelix (Dec 19 2018 at 16:25, on Zulip):

Again, seems good to just CC #49434, P-medium, NLL-complete.

pnkfelix (Dec 19 2018 at 16:27, on Zulip):

next: "[nll] _ patterns should not count as borrows" #53114

pnkfelix (Dec 19 2018 at 16:28, on Zulip):

this is already marked as NLL-complete, and the most urgent part of it was already spawned off into #54003 (which remains unaddressed, but it will be triaged separately is the point).

pnkfelix (Dec 19 2018 at 16:28, on Zulip):

but I'm not 100% sure we have unit tests for all the cases here...?

pnkfelix (Dec 19 2018 at 16:30, on Zulip):

left a comment saying double-checking our testing situation is P-high, and assigned that subtask to self.

pnkfelix (Dec 19 2018 at 16:30, on Zulip):

hmm this issue is already assigned to matthewjasper

pnkfelix (Dec 19 2018 at 16:30, on Zulip):

well I'll add myself

pnkfelix (Dec 19 2018 at 16:32, on Zulip):

Okay I have to go home now, I got the the set of NLL-deferred issues down to 7 issues which seems like good progress for today.

nikomatsakis (Dec 19 2018 at 20:23, on Zulip):

call me crazy but I think given our migration strategy, identifying the severity of the problem here (if any) sounds like a P-high problem to me.

seems ok

pnkfelix (Dec 21 2018 at 14:14, on Zulip):

okay resuming this task

pnkfelix (Dec 21 2018 at 14:14, on Zulip):

next: "[nll] move 2-phase borrows into MIR lowering" #53198

pnkfelix (Dec 21 2018 at 14:15, on Zulip):

this may or may not fold into an effort to determine+document the "final" specification for two-phase borrows (2PB)

pnkfelix (Dec 21 2018 at 14:16, on Zulip):

there are definitely open questions

pnkfelix (Dec 21 2018 at 14:18, on Zulip):

and some conversation here

pnkfelix (Dec 21 2018 at 14:23, on Zulip):

after reflection: the 2PB specification effort needs to at least be aware of #53198. In particular, it should decide whether we want to enusre we leave the door open for that implementation strategy, or just potentially close it forever.

pnkfelix (Dec 21 2018 at 14:26, on Zulip):

and since that specification effort (#46901) is P-high, I am likewise marking #53198 as P-high.

pnkfelix (Dec 21 2018 at 14:26, on Zulip):

next: "NLL: document specs for (new) semantics in rust ref (incl. deviations from RFC)" #54129

pnkfelix (Dec 21 2018 at 14:27, on Zulip):

I'd say that #46901 is a subtask of #54129

pnkfelix (Dec 21 2018 at 14:28, on Zulip):

but #54129 is not as high priority. It doesn't strike me as something that must land any time soon; its just a nice-to-have.

pnkfelix (Dec 21 2018 at 14:28, on Zulip):

So I'm tagging #54129 as P-medium

pnkfelix (Dec 21 2018 at 14:29, on Zulip):

next: "#[thread_local] static mut is allowed to have 'static lifetime" #54366

pnkfelix (Dec 21 2018 at 14:29, on Zulip):

thread locals are unstable feature

pnkfelix (Dec 21 2018 at 14:30, on Zulip):

let me see whether there is a tracking issue for stabilizing them...

pnkfelix (Dec 21 2018 at 14:31, on Zulip):

ah its already listed on the stabilization issue for thread_local (#29594)

centril (Dec 21 2018 at 14:31, on Zulip):

@pnkfelix when you say: > In particular, it should decide whether we want to enusre we leave the door open for that implementation strategy, or just potentially close it forever.

... I haven't read the issue #53198 in detail (skimmed it)...
... does it get potentially closed forever because of some change to the typesystem / operational semantics such that it cannot be changed later?
... if so, T-lang should be brought in

pnkfelix (Dec 21 2018 at 14:34, on Zulip):

yeah, there are some examples that are on the comment thread for #53198 of code that is accepted today that we expect would be rejected by the semantics that arises via the rewrite-based 2PB

pnkfelix (Dec 21 2018 at 14:34, on Zulip):

now, its possible that those examples are not worth supporting

pnkfelix (Dec 21 2018 at 14:34, on Zulip):

and its also possible that the rewrite could be revised to support them

pnkfelix (Dec 21 2018 at 14:35, on Zulip):

and its also possible that the static semantics for NLL could be extended (in some way simpler than NLL) that would make those examples compile even under the proposed rewrite

pnkfelix (Dec 21 2018 at 14:36, on Zulip):

but it also might be premature to bring in T-lang at this point

pnkfelix (Dec 21 2018 at 14:36, on Zulip):

i.e. there may be some amount of pre-triage/documentation/survey that NLL team members and/or RalfJung could do

pnkfelix (Dec 21 2018 at 14:37, on Zulip):

before we start bringing this up at T-lang meeting(s) ?

pnkfelix (Dec 21 2018 at 14:37, on Zulip):

@centril ^

centril (Dec 21 2018 at 14:37, on Zulip):

@pnkfelix OK; from your description it seems like T-lang should be brought in at some point before any irreversible action is taken (e.g. changing semantics in a user observable way or finalizing documentation)

pnkfelix (Dec 21 2018 at 14:37, on Zulip):

sure, that makes sense

centril (Dec 21 2018 at 14:37, on Zulip):

(either dynamic or static semantics...)

centril (Dec 21 2018 at 14:38, on Zulip):

aight; thanks -- I'll let you return to your triage :slight_smile:

pnkfelix (Dec 21 2018 at 14:39, on Zulip):

back to re-triaging "#[thread_local] static mut is allowed to have 'static lifetime" #54366 ...

pnkfelix (Dec 21 2018 at 14:40, on Zulip):

alexcrichton made the following comment regarding stabilization of thread_local:

I've personally been under the impression that we're holding out on stabilizing this for as long as possible. We've stabilized very few (AFAIK) platform-specific attributes like this and I at least personally haven't ever looked to hard into stabilizing this.

pnkfelix (Dec 21 2018 at 14:40, on Zulip):

based on that, I'm going to tag #54366 as P-medium

pnkfelix (Dec 21 2018 at 14:48, on Zulip):

next: "NLL: need 3rd round of review comparing .stderr and .nll.stderr output" #54528

pnkfelix (Dec 21 2018 at 14:49, on Zulip):

I still do want to go through the work items on the cards here. But its definitely not high priority. P-medium.

pnkfelix (Dec 21 2018 at 14:50, on Zulip):

next: "'approximate check' for outlives relations can miss matches" #54572

pnkfelix (Dec 21 2018 at 14:52, on Zulip):

I don't think there's been any change on the status here. Its still pretty low priority. I think I'm going to mark it as P-low and wait for @nikomatsakis to yell at me for choosing such a low priority.

pnkfelix (Dec 21 2018 at 14:53, on Zulip):

next: "Goal: Accept partial initialization + use of records created via such" #54987

pnkfelix (Dec 21 2018 at 14:54, on Zulip):

I think @centril was working on an RFC related to this.

centril (Dec 21 2018 at 14:54, on Zulip):

@pnkfelix Yeah; https://github.com/Centril/rfcs/pull/16 -- I left some TODOs for you / @nikomatsakis

pnkfelix (Dec 21 2018 at 14:54, on Zulip):

but its definitely not high priority. And until I see people from community clamoring for it, I'm not sure its even P-medium.

pnkfelix (Dec 21 2018 at 14:55, on Zulip):

@centril do you want me to link from #54987 to that PR ?

centril (Dec 21 2018 at 14:55, on Zulip):

@pnkfelix seems P-low... or; it's also blocked by an RFC so not even P-anything ;)

pnkfelix (Dec 21 2018 at 14:55, on Zulip):

right; I prefer to explictly mark as P-low so that we know we don't need to think about it further during future triage.

centril (Dec 21 2018 at 14:55, on Zulip):

@pnkfelix sure, why not

pnkfelix (Dec 21 2018 at 14:57, on Zulip):

next (and last): "NLL error on closure, but not on equivalent function" #55526

pnkfelix (Dec 21 2018 at 14:57, on Zulip):

I've had quite a saga personally with #55526

pnkfelix (Dec 21 2018 at 14:58, on Zulip):

I think the discrepancy under discussion there may be worth discussing with T-lang...

pnkfelix (Dec 21 2018 at 15:00, on Zulip):

but I don't see a reason to change the P-label (which was P-medium; see aforementioned saga where it went from P-high to P-medium over past 1.5 months)

pnkfelix (Dec 21 2018 at 15:01, on Zulip):

and that's it; no more NLL-deferreds.

centril (Dec 21 2018 at 15:01, on Zulip):

@pnkfelix do you want to I-nominate the last one for T-lang or is that premature?

pnkfelix (Dec 21 2018 at 15:02, on Zulip):

premature I think

pnkfelix (Dec 21 2018 at 15:02, on Zulip):

or at least

pnkfelix (Dec 21 2018 at 15:02, on Zulip):

the main thing that T-lang may want to discuss is the hypothetical change to AST-borrowck

pnkfelix (Dec 21 2018 at 15:03, on Zulip):

so that it will reject the code

pnkfelix (Dec 21 2018 at 15:03, on Zulip):

when no return type is provided

pnkfelix (Dec 21 2018 at 15:03, on Zulip):

which is an expected side-effect of PR #55988

centril (Dec 21 2018 at 15:03, on Zulip):

@pnkfelix is this related to this weirdness btw? https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=824c389a9ee6f01ec519a2cbcfc9ebef

centril (Dec 21 2018 at 15:04, on Zulip):

(4-5 does not typeck but it should presumably do so)

pnkfelix (Dec 21 2018 at 15:04, on Zulip):

yes I think this is related

pnkfelix (Dec 21 2018 at 15:05, on Zulip):

which reminds me

pnkfelix (Dec 21 2018 at 15:05, on Zulip):

I think that there is an important task here

pnkfelix (Dec 21 2018 at 15:05, on Zulip):

to actually ensure our test suite captures the current various behaviors

pnkfelix (Dec 21 2018 at 15:05, on Zulip):

so that we're aware of future changes

centril (Dec 21 2018 at 15:05, on Zulip):

@pnkfelix but do you agree that 4-5 is a bug?

pnkfelix (Dec 21 2018 at 15:06, on Zulip):

well

pnkfelix (Dec 21 2018 at 15:06, on Zulip):

it might not be a bug... I'm not sure

pnkfelix (Dec 21 2018 at 15:06, on Zulip):

you can still add an explicit type annotation to get it to work again

pnkfelix (Dec 21 2018 at 15:06, on Zulip):

like so: https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=d002240a9cb91a613a529b26f2af990b

pnkfelix (Dec 21 2018 at 15:07, on Zulip):

so its basically an issue with how we use (limited) contextual information to infer the type of a closure

pnkfelix (Dec 21 2018 at 15:07, on Zulip):

and I think people assume certain transformations are valid (like the one you are demonstrating here) but the transformations just ... aren't.

centril (Dec 21 2018 at 15:08, on Zulip):

@pnkfelix sure, or with impl for<'a> Fn(&'a u8) -> u8 more generally...

pnkfelix (Dec 21 2018 at 15:08, on Zulip):

don't think that works

pnkfelix (Dec 21 2018 at 15:09, on Zulip):

(or maybe you just need to enable certain features for it)

pnkfelix (Dec 21 2018 at 15:09, on Zulip):

(mine worked on stable)

centril (Dec 21 2018 at 15:09, on Zulip):

@pnkfelix not yet ;) but let x: impl ... = expr; is on its way

pnkfelix (Dec 21 2018 at 15:09, on Zulip):

still even this breaks: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2015&gist=51d8cc3f965e47e56a700cdaf4bcbd3b

pnkfelix (Dec 21 2018 at 15:10, on Zulip):

assuming you don't want to also change signature of fn bar

centril (Dec 21 2018 at 15:10, on Zulip):

oh yeah; true

pnkfelix (Dec 21 2018 at 15:10, on Zulip):

anyway

pnkfelix (Dec 21 2018 at 15:10, on Zulip):

it seems like this may be a different but related issue

pnkfelix (Dec 21 2018 at 15:10, on Zulip):

where the fundamental thing is the question of "how does type/region inference work for closures???"

pnkfelix (Dec 21 2018 at 15:10, on Zulip):

right?

centril (Dec 21 2018 at 15:10, on Zulip):

right

centril (Dec 21 2018 at 15:11, on Zulip):

@pnkfelix it seems to me that in this instance, because the binding x is otherwise unconstrained and there's no surrounding contextual information that it should have the most general type it can have and that is to have for<'a> ...

pnkfelix (Dec 21 2018 at 15:14, on Zulip):

I think @nikomatsakis has suggested doing this change

pnkfelix (Dec 21 2018 at 15:14, on Zulip):

they've used an analogy with how ML inference of type-polymorphic let-bindings works

centril (Dec 21 2018 at 15:15, on Zulip):

@pnkfelix yeah; you'd get the same results in Haskell

centril (Dec 21 2018 at 15:16, on Zulip):

@pnkfelix I'm working on proposal(s) for for<'a: 'b>, for<T: Trait> and generic closures anyways so I could work that in somewhere

pnkfelix (Dec 21 2018 at 15:21, on Zulip):

Context of where niko suggested ML-style inference: https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/subject/.2355526.20error.20on.20closure.20but.20not.20on.20.22equiv.22.20fn/near/151010804

pnkfelix (Dec 21 2018 at 15:22, on Zulip):

@centril ^

Last update: Nov 21 2019 at 13:05UTC