Stream: t-compiler/wg-nll

Topic: weekly meeting August 28


nikomatsakis (Aug 28 2018 at 19:33, on Zulip):

:wave: hello dear friends in @WG-compiler-nll

nikomatsakis (Aug 28 2018 at 19:34, on Zulip):

paper triage document is here

pnkfelix (Aug 28 2018 at 19:35, on Zulip):

any chance @Matthew Jasper is around tonight? I'm not sure what time zone they are in, but I figured I'd check.

Matthew Jasper (Aug 28 2018 at 19:35, on Zulip):

I'm here

pnkfelix (Aug 28 2018 at 19:36, on Zulip):

okay great. there were some questions that came up when @nikomatsakis and I were chatting earlier today

pnkfelix (Aug 28 2018 at 19:36, on Zulip):

(they aren't on the agenda; we should probably go through the meeting agenda, and then if there's time after that I can pick your brain.)

Matthew Jasper (Aug 28 2018 at 19:37, on Zulip):

I'll be around after the meeting, so that should be fine.

nikomatsakis (Aug 28 2018 at 19:37, on Zulip):

so let's see.. overall status...

nikomatsakis (Aug 28 2018 at 19:37, on Zulip):

perf is basically looking good, apart from UCD (and memory use)

nikomatsakis (Aug 28 2018 at 19:37, on Zulip):

@Wesley Wiser's branch addresses the memory use problem

nikomatsakis (Aug 28 2018 at 19:38, on Zulip):

@mikhail-m1 was looking at what I think is the best fix for UCD perf

nikomatsakis (Aug 28 2018 at 19:38, on Zulip):

so it's essentially down to correctness (sound, complete) + diagnostics

lqd (Aug 28 2018 at 19:38, on Zulip):

(and njn has a quick fix PR for parts of ucd)

nikomatsakis (Aug 28 2018 at 19:39, on Zulip):

this is the full list of milestone issues -- some are unassigned. I've marked some of them with I-nominated and left some notes in the paper

nikomatsakis (Aug 28 2018 at 19:40, on Zulip):

it's not clear that all of them belong on the milestone

pnkfelix (Aug 28 2018 at 19:40, on Zulip):

there are effectively three relevant milestones remaining at this point, right?

pnkfelix (Aug 28 2018 at 19:40, on Zulip):

RC, RC2, and Release. Do I have that right?

pnkfelix (Aug 28 2018 at 19:41, on Zulip):

(I want to make sure we're all on the same page)

nikomatsakis (Aug 28 2018 at 19:41, on Zulip):

that is correct

nikomatsakis (Aug 28 2018 at 19:41, on Zulip):

RC is still a week or two out I think?

pnkfelix (Aug 28 2018 at 19:41, on Zulip):

so it behooves us

pnkfelix (Aug 28 2018 at 19:42, on Zulip):

to probably deliberately move some number of these from RC to RC2 (at least)

pnkfelix (Aug 28 2018 at 19:42, on Zulip):

right? I suspect @nikomatsakis was already doing some of that

nikomatsakis (Aug 28 2018 at 19:43, on Zulip):

it may make sense, yeah

nikomatsakis (Aug 28 2018 at 19:43, on Zulip):

we can also wait to do that until closer

pnkfelix (Aug 28 2018 at 19:43, on Zulip):

Well

pnkfelix (Aug 28 2018 at 19:43, on Zulip):

I guess if all the issues are equally important

pnkfelix (Aug 28 2018 at 19:43, on Zulip):

then yes, it doesn't matter when we do that triaging

pnkfelix (Aug 28 2018 at 19:43, on Zulip):

but if some are more important than others ...

nikomatsakis (Aug 28 2018 at 19:44, on Zulip):

yeah, it's fine to move to RC2, then we can always start in early if we finish everything :)

nikomatsakis (Aug 28 2018 at 19:44, on Zulip):

should we look at the "nominated" things?

pnkfelix (Aug 28 2018 at 19:44, on Zulip):

I would like to look at the nominated things, yes

nikomatsakis (Aug 28 2018 at 19:44, on Zulip):

ok first one was How to split up user-based type annotations? (#47184)

nikomatsakis (Aug 28 2018 at 19:45, on Zulip):

though I think we left some comments in thread

pnkfelix (Aug 28 2018 at 19:45, on Zulip):

this is the (fixed) list, right?

nikomatsakis (Aug 28 2018 at 19:45, on Zulip):

the main thing to figure out was whether this can be split up

nikomatsakis (Aug 28 2018 at 19:45, on Zulip):

er, yes; I was going off the notes in the paper

pnkfelix (Aug 28 2018 at 19:45, on Zulip):

or I guess you are probably using the link from the paper doc

pnkfelix (Aug 28 2018 at 19:45, on Zulip):

good idea

nikomatsakis (Aug 28 2018 at 19:45, on Zulip):

in particular some of the issues are grouped together

nikomatsakis (Aug 28 2018 at 19:46, on Zulip):

the main thing to figure out was whether this can be split up

I think the answer to this is yes but i'm not sure if it makes sense to do so — in particular the constant stuff feels pretty separate

nikomatsakis (Aug 28 2018 at 19:47, on Zulip):

I guess I could try to split out mentoring instructions, if someone thought they might be up for said work (@davidtwco, are you looking for tasks right now? :)

nikomatsakis (Aug 28 2018 at 19:47, on Zulip):

/me just noticed the words "lazy"

davidtwco (Aug 28 2018 at 19:47, on Zulip):

I will need something to fix aforementioned laziness, yes.

nikomatsakis (Aug 28 2018 at 19:47, on Zulip):

well maybe we should look at the next few though

nikomatsakis (Aug 28 2018 at 19:48, on Zulip):

e.g., there are a couple of tricky correctness-related problems:

nikomatsakis (Aug 28 2018 at 19:49, on Zulip):

I guess the atty one -- @Matthew Jasper you think you'll have time for that?

lqd (Aug 28 2018 at 19:49, on Zulip):

@pnkfelix I can do the minimization for #52934 if you want to do more important things

nikomatsakis (Aug 28 2018 at 19:49, on Zulip):

I saw you opened a topic for us to discuss the best fix :)

nikomatsakis (Aug 28 2018 at 19:50, on Zulip):

(oh, meta point, I just realized I have a hard stop today at 16:00 -- in 10 minutes, that is)

pnkfelix (Aug 28 2018 at 19:51, on Zulip):

@lqd sure, feel free to add yourself to the assignees list for #52934. I'm going to keep it assigned to me but I'll try to ping anyone else who's assigned.

nikomatsakis (Aug 28 2018 at 19:51, on Zulip):

I was curious to "make a decision" about two-phase borrows

lqd (Aug 28 2018 at 19:51, on Zulip):

what else can I help with ?

Matthew Jasper (Aug 28 2018 at 19:51, on Zulip):

I should be able to get a fix up for it soon if we can agree an approach. I was going to combine it with a diagnostic improvement, but that can wait.

pnkfelix (Aug 28 2018 at 19:52, on Zulip):

I was curious to "make a decision" about two-phase borrows

as in #53723 ?

nikomatsakis (Aug 28 2018 at 19:52, on Zulip):

in particular, the question is whether we think it's worth trying to do any expansions, yeah

pnkfelix (Aug 28 2018 at 19:52, on Zulip):

/me thinks we should focus on other things

pnkfelix (Aug 28 2018 at 19:52, on Zulip):

the borrowck=migrate mode gives us breathing room here

pnkfelix (Aug 28 2018 at 19:52, on Zulip):

in terms of future improvements

nikomatsakis (Aug 28 2018 at 19:52, on Zulip):

certainly an expanded form would fix basically all of the "unfortunate" regressions we've seen on crates.io — but I think most of them have "complicating factors" that would require an involved fix

nikomatsakis (Aug 28 2018 at 19:52, on Zulip):

in terms of future improvements

that's an interesting point

nikomatsakis (Aug 28 2018 at 19:53, on Zulip):

I hadn't fully considered that

pnkfelix (Aug 28 2018 at 19:53, on Zulip):

i.e. ICE's kill us

pnkfelix (Aug 28 2018 at 19:53, on Zulip):

and performance kills us

pnkfelix (Aug 28 2018 at 19:53, on Zulip):

in the sense that borrowck=migrate doesn't mitigate those things

nikomatsakis (Aug 28 2018 at 19:53, on Zulip):

yes

pnkfelix (Aug 28 2018 at 19:53, on Zulip):

and of course NLL-sound kills us too

pnkfelix (Aug 28 2018 at 19:53, on Zulip):

but in some ways we (as in the Rust compiler) has always had soundness bugs

pnkfelix (Aug 28 2018 at 19:54, on Zulip):

so I'm willing to let some of those slip through to be fixed later

nikomatsakis (Aug 28 2018 at 19:54, on Zulip):

I think my playbook would be something like this:

lqd (Aug 28 2018 at 19:54, on Zulip):

i.e. ICE's kill us

do we want to do another crater run now that the previous one's ICEs are fixed ?

nikomatsakis (Aug 28 2018 at 19:54, on Zulip):

and of course NLL-sound kills us too

we're not really aware of any of these, right?

nikomatsakis (Aug 28 2018 at 19:55, on Zulip):

do we want to do another crater run now that the previous one's ICEs are fixed

I think we do, but we might want to wait until the user type annot thing is closer to done

pnkfelix (Aug 28 2018 at 19:55, on Zulip):

respecting type annotations is the big NLL-sound issue I know of

nikomatsakis (Aug 28 2018 at 19:55, on Zulip):

ah, ok, yes

nikomatsakis (Aug 28 2018 at 19:55, on Zulip):

in particular — I think the fix for that will involve touch some "widely used" code

nikomatsakis (Aug 28 2018 at 19:55, on Zulip):

and could create ICEs

pnkfelix (Aug 28 2018 at 19:55, on Zulip):

(and you've already heard my crazy workarounds for the remaining type annotation cases)

Matthew Jasper (Aug 28 2018 at 19:55, on Zulip):

Also #53695, depending on what we decide.

nikomatsakis (Aug 28 2018 at 19:55, on Zulip):

/me not ready to surrender yet

pnkfelix (Aug 28 2018 at 19:56, on Zulip):

ah shall we move on to #53695

pnkfelix (Aug 28 2018 at 19:56, on Zulip):

?

pnkfelix (Aug 28 2018 at 19:56, on Zulip):

(that is, put a pin in the 2PB discussion?)

nikomatsakis (Aug 28 2018 at 19:56, on Zulip):

I think we settled it

nikomatsakis (Aug 28 2018 at 19:57, on Zulip):

the only open question for me w/r/t 2PB is how much confusion we can mitigate with improved diagnostics

nikomatsakis (Aug 28 2018 at 19:57, on Zulip):

(I'm gonna have to run, but I'll try to follow along)

nikomatsakis (Aug 28 2018 at 19:59, on Zulip):

"2PB == two-phase borrows", for those who might not have realized :)

nikomatsakis (Aug 28 2018 at 20:00, on Zulip):

ok that meeting is starting a bit late ;)

nikomatsakis (Aug 28 2018 at 20:00, on Zulip):

what about this "removing warnings (#52768)"

nikomatsakis (Aug 28 2018 at 20:01, on Zulip):

that warning is certainly an "impl detail" we should not expose to users

nikomatsakis (Aug 28 2018 at 20:01, on Zulip):

@pnkfelix can you say a bit more about this?

But on the other hand, my diagnostic review (#52663) is revealing a couple of cases where a diagnostic is missing useful info that we used to report, and I think they correspond to these region errors that we are not reporting.

pnkfelix (Aug 28 2018 at 20:02, on Zulip):

boy I wish I had included more context there

pnkfelix (Aug 28 2018 at 20:02, on Zulip):

but I think the summary is that

pnkfelix (Aug 28 2018 at 20:02, on Zulip):

we had some nice errors that were only reported in AST-borrowck at the time

pnkfelix (Aug 28 2018 at 20:03, on Zulip):

since then, we've turned back on at least one of those nice errors, I think ... maybe only in borrowck=migrate mode? Not sure

pnkfelix (Aug 28 2018 at 20:03, on Zulip):

let me see if I can find a specific link from #52663

pnkfelix (Aug 28 2018 at 20:03, on Zulip):

oh right I think the devil is literally in the <details> here

pnkfelix (Aug 28 2018 at 20:04, on Zulip):

src/test/ui/issue-13058.nll.stderr was one I noted

pnkfelix (Aug 28 2018 at 20:05, on Zulip):

this is the comment collecting them in that issue: https://github.com/rust-lang/rust/issues/52663#issuecomment-409031734

pnkfelix (Aug 28 2018 at 20:05, on Zulip):

so the questions I have are: 1. What does NLL currently do for those two cases in that comment, and 2. What does borrowck=migrate do for them.

Matthew Jasper (Aug 28 2018 at 20:07, on Zulip):

src/test/ui/issue-13058.nll.stderr is probably because NLL (or any MIR generation?) doesn't run if there are type errors.

pnkfelix (Aug 28 2018 at 20:07, on Zulip):

so consider this case: https://play.rust-lang.org/?gist=8851ff9938c45d9180bd71cb584d6fbc&version=nightly&mode=debug&edition=2015

pnkfelix (Aug 28 2018 at 20:08, on Zulip):

On nightly for play, AST-borrowck includes message "help: you can add a constraint to the return type to make it last less than 'static and match the anonymous lifetime #1 defined on the method body"

pnkfelix (Aug 28 2018 at 20:08, on Zulip):

under NLL, it does not include that

Matthew Jasper (Aug 28 2018 at 20:08, on Zulip):

static-return-lifetime-infered.nll.stderr is indeed a regression.

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

BUT

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

2018 edition: https://play.rust-lang.org/?gist=7c173cea49b300b334029b61c00bed6b&version=nightly&mode=debug&edition=2018

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

i.e. borrowck=migrate

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

includes the nice help

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

So I don't think this needs to be a high priority item to resolve

pnkfelix (Aug 28 2018 at 20:09, on Zulip):

because of whatever hack I put in for borrowck=migrate

pnkfelix (Aug 28 2018 at 20:10, on Zulip):

Ill transcribe this info onto #52768

lqd (Aug 28 2018 at 20:15, on Zulip):

I guess it could be useful to see how the wontfix/2PB error cases from the latest crater run would look with borrowck=migrate

pnkfelix (Aug 28 2018 at 20:16, on Zulip):

has anyone been doing crater runs with 2018 edition turned on?

lqd (Aug 28 2018 at 20:16, on Zulip):

there's one running IIRC, @Pietro Albini right ?

pnkfelix (Aug 28 2018 at 20:16, on Zulip):

because that should have that same effect (plus other stuff), right?

lqd (Aug 28 2018 at 20:16, on Zulip):

sure

Pietro Albini (Aug 28 2018 at 20:17, on Zulip):

yep, one edition crater run should be running, let me check

Pietro Albini (Aug 28 2018 at 20:17, on Zulip):

yep it's running (I hate logging into the server to check legacy runs...)

lqd (Aug 28 2018 at 20:17, on Zulip):

@Pietro Albini thank you

lqd (Aug 28 2018 at 20:19, on Zulip):

do we know when it will end approximately ?

lqd (Aug 28 2018 at 20:19, on Zulip):

(that is, I can manually check with borrowck=migrate until the results are available)

lqd (Aug 28 2018 at 20:21, on Zulip):

@pnkfelix did you want to talk about more things ?

Pietro Albini (Aug 28 2018 at 20:21, on Zulip):

hmm. in 3/4 days it should be finished, but I'm not sure (there is no progress indicator for legacy runs, and it's running on a new temporary machine)

pnkfelix (Aug 28 2018 at 20:22, on Zulip):

@pnkfelix did you want to talk about more things ?

well there were like 10 nominated issues i think

lqd (Aug 28 2018 at 20:23, on Zulip):

(it would also be interesting to build servo with NLLs)

pnkfelix (Aug 28 2018 at 20:23, on Zulip):

these are the nominated issues: https://github.com/rust-lang/rust/issues?utf8=%E2%9C%93&q=+label%3AI-nominated+label%3AA-NLL+

pnkfelix (Aug 28 2018 at 20:23, on Zulip):

we've touched on some of these already

pnkfelix (Aug 28 2018 at 20:24, on Zulip):

e.g. everything that is about 2PB, I think we agreed we can afford to let that slide to later

pnkfelix (Aug 28 2018 at 20:24, on Zulip):

sorry let me narrow focus to just the open nominated issues: https://github.com/rust-lang/rust/issues?q=label%3AI-nominated+label%3AA-NLL+is%3Aopen

pnkfelix (Aug 28 2018 at 20:25, on Zulip):

so I think the main thing I wanted to talk about was #53595 #53695

davidtwco (Aug 28 2018 at 20:25, on Zulip):

I'm not convinced that was what you intended to link to.

lqd (Aug 28 2018 at 20:25, on Zulip):

is that the good link ?

pnkfelix (Aug 28 2018 at 20:25, on Zulip):

(which .... I don't think was discussed yet ... apart from my mentioning it above)

pnkfelix (Aug 28 2018 at 20:25, on Zulip):

ugh

pnkfelix (Aug 28 2018 at 20:26, on Zulip):

fixed

lqd (Aug 28 2018 at 20:26, on Zulip):

is this the one you wanted to discuss with @Matthew Jasper ?

pnkfelix (Aug 28 2018 at 20:26, on Zulip):

no

pnkfelix (Aug 28 2018 at 20:27, on Zulip):

i mean, we do want to get @Matthew Jasper 's input on this too

pnkfelix (Aug 28 2018 at 20:27, on Zulip):

but the questions I was alluding to earlier were about something else

Matthew Jasper (Aug 28 2018 at 20:27, on Zulip):

The solution here is pretty clear, it's just whether we think that this should be allowed.

pnkfelix (Aug 28 2018 at 20:28, on Zulip):

well, I'm also wondering : 1. by when do we need a decision on this, and 2. who should make the decision?

pnkfelix (Aug 28 2018 at 20:28, on Zulip):

Do we need the lang team involved? Or should we just trust our WG to make the right call here

lqd (Aug 28 2018 at 20:28, on Zulip):

what is your immediate reaction to this question ?

pnkfelix (Aug 28 2018 at 20:29, on Zulip):

Regarding the first Q, I personally don't think this has to be decided by RC. But I'd love to hear counter arguments.

lqd (Aug 28 2018 at 20:30, on Zulip):

it certainly wouldn't hurt to ask t-lang in any case

pnkfelix (Aug 28 2018 at 20:30, on Zulip):

Regarding the second Q: Well, I suppose we've been left in charge of even meatier Q's. :)

pnkfelix (Aug 28 2018 at 20:30, on Zulip):

this is just one of those ones that seems like a bikeshed type Q

Matthew Jasper (Aug 28 2018 at 20:30, on Zulip):

I don't think we need to decide. But I think that we should check if there's any disagreement about it first.

pnkfelix (Aug 28 2018 at 20:31, on Zulip):

(you know, how everyone can have an opinion on something this "simple")

pnkfelix (Aug 28 2018 at 20:31, on Zulip):

So, lets say we add the fake read. is there any downside we can anticipate?

Matthew Jasper (Aug 28 2018 at 20:33, on Zulip):

Perf, maybe?

pnkfelix (Aug 28 2018 at 20:33, on Zulip):

and should the fake read indeed occur after the whole let PAT = EXPR; is done?

Matthew Jasper (Aug 28 2018 at 20:33, on Zulip):

Well, it needs to occur at a point which is meaningfully later than the assign.

pnkfelix (Aug 28 2018 at 20:34, on Zulip):

or should it somehow be intermixed. I'm thinking of a case like let (x, y) = ...;: should the fake read of x come before the initialization of y

Matthew Jasper (Aug 28 2018 at 20:34, on Zulip):

I think that we already fail in that case

Matthew Jasper (Aug 28 2018 at 20:34, on Zulip):

(to compile)

lqd (Aug 28 2018 at 20:34, on Zulip):

@Matthew Jasper when you said earlier that the solution was clear in this case, were you thinking of the "fake read" solution as well ?

Matthew Jasper (Aug 28 2018 at 20:34, on Zulip):

It's just let [mut] x = ... where this happens. (quickly goes to check)

Matthew Jasper (Aug 28 2018 at 20:35, on Zulip):

Yes

pnkfelix (Aug 28 2018 at 20:35, on Zulip):

I have to admit I find the fake read ... a bit hacky here

lqd (Aug 28 2018 at 20:36, on Zulip):

do we have an idea on how much work would this be ? ie, if perf does turn out to regress, would we lose a couple hours or days ?

pnkfelix (Aug 28 2018 at 20:36, on Zulip):

it just doesn't feel right to me. Its only a gut reaction

lqd (Aug 28 2018 at 20:37, on Zulip):

(and/or if t-lang disagrees with this solution)

Jake Goulding (Aug 28 2018 at 20:38, on Zulip):

there's nothing incorrect about the code, right? Isn't this fake read akin to adding a lint?

pnkfelix (Aug 28 2018 at 20:38, on Zulip):

@Jake Goulding yeah I think that is part of why my gut is reacting

pnkfelix (Aug 28 2018 at 20:38, on Zulip):

like, if we're accepting the code

    let x;
    {
        let z = 0;
        x = &z
    };

then why not accept the form where its in the initializer?

Matthew Jasper (Aug 28 2018 at 20:39, on Zulip):

It depends on the exact order that things happen in MIR. Whether this is a good enough reason to disallow it is perhaps up for debate.

pnkfelix (Aug 28 2018 at 20:40, on Zulip):

ah true. We did after all, for example, put in FalseEdges in order to keep our options open in terms of what MIR we generate

pnkfelix (Aug 28 2018 at 20:40, on Zulip):

So its possible this is analogous to that ... ?

Matthew Jasper (Aug 28 2018 at 20:41, on Zulip):

There's also let _ = ...(or any pattern that doesn't bind and values or read any discriminant), but that seems more reasonable.

pnkfelix (Aug 28 2018 at 20:41, on Zulip):

still, I think I'd need to see examples of the variants here, i.e. the cases where we accept and where we reject that otherwise look similar

pnkfelix (Aug 28 2018 at 20:41, on Zulip):

The let _ = ...; is a separate can of worms I think

pnkfelix (Aug 28 2018 at 20:42, on Zulip):

or at least its one that I believe @nikomatsakis made a dedicated issue for somwhere...

Matthew Jasper (Aug 28 2018 at 20:42, on Zulip):

I guess I'll write up cases we do and don't allow in the issue.

nikomatsakis (Aug 28 2018 at 20:43, on Zulip):

sorry, had to drop off. I don't have a strong opinion here but I think writing up various examples and then cc'ing a wider group to get their take seems fine

nikomatsakis (Aug 28 2018 at 20:44, on Zulip):

in particular the differences when using initializer vs when using block were interesting to me

nikomatsakis (Aug 28 2018 at 20:44, on Zulip):

I think I tend towards "this is ok" personally but.. yeah.

nikomatsakis (Aug 28 2018 at 20:44, on Zulip):

not sure yet

pnkfelix (Aug 28 2018 at 20:45, on Zulip):

or at least its one that I believe @nikomatsakis made a dedicated issue for somwhere...

#53114 is what I was thinking of here.

pnkfelix (Aug 28 2018 at 20:50, on Zulip):

@nikomatsakis do you agree that we do not need to make a decision in time for RC ?

pnkfelix (Aug 28 2018 at 20:50, on Zulip):

(in which case I'd argue that we should reassign it to RC 2 milestone)

lqd (Aug 28 2018 at 20:52, on Zulip):

(sorry I have to go, bbl)

pnkfelix (Aug 28 2018 at 20:52, on Zulip):

I must myself go t osleep ... :sleepy:

lqd (Aug 28 2018 at 20:53, on Zulip):

o/

nikomatsakis (Aug 28 2018 at 20:53, on Zulip):

@nikomatsakis do you agree that we do not need to make a decision in time for RC ?

yes I do

Last update: Nov 22 2019 at 00:40UTC