Stream: t-compiler/wg-nll

Topic: weekly meeting July 10


pnkfelix (Jul 10 2018 at 19:31, on Zulip):

Hi @everyone its about time to start the weekly meeting

davidtwco (Jul 10 2018 at 19:31, on Zulip):

o/

lqd (Jul 10 2018 at 19:31, on Zulip):

:wave:

pnkfelix (Jul 10 2018 at 19:32, on Zulip):

If you have status to report please do add it to the triage Paper, though perhaps everyone has already done so...

pnkfelix (Jul 10 2018 at 19:34, on Zulip):

So lets see, I've been gone for a while

pnkfelix (Jul 10 2018 at 19:34, on Zulip):

You all did some great stuff in my absence

pnkfelix (Jul 10 2018 at 19:34, on Zulip):

I see that there was a lot of triaging effort

lqd (Jul 10 2018 at 19:34, on Zulip):

(welcome back again :)

pnkfelix (Jul 10 2018 at 19:35, on Zulip):

and the performance numbers are looking much better. Still not great, of course...

Santiago Pastorino (Jul 10 2018 at 19:35, on Zulip):

welcome back @pnkfelix :)

pnkfelix (Jul 10 2018 at 19:35, on Zulip):

At least that's my reading of: https://perf.rust-lang.org/nll-dashboard.html

lqd (Jul 10 2018 at 19:35, on Zulip):

there have been a couple crater runs

Santiago Pastorino (Jul 10 2018 at 19:36, on Zulip):

wow 2,246.69% for tuple stress

lqd (Jul 10 2018 at 19:36, on Zulip):

it was 12000% :)

pnkfelix (Jul 10 2018 at 19:36, on Zulip):

@lqd right so maybe lets talk about the crater runs

lqd (Jul 10 2018 at 19:36, on Zulip):

and there's some in-progress work from @DPC but unfortunately awaiting review/help from niko

lqd (Jul 10 2018 at 19:37, on Zulip):

@pnkfelix sure

lqd (Jul 10 2018 at 19:37, on Zulip):

so as I mentioned in the dedicated topic, we have done 2 runs

lqd (Jul 10 2018 at 19:37, on Zulip):

one without 2 phase borrows and one with them

DPC (Jul 10 2018 at 19:37, on Zulip):

@lqd yeah some more work is left on that PR.

pnkfelix (Jul 10 2018 at 19:37, on Zulip):

the dedicated topic is of course the one titled: crater-run-and-analysis

lqd (Jul 10 2018 at 19:37, on Zulip):

the 1st one uncovered another ICE IIC, otherwise they were all known

lqd (Jul 10 2018 at 19:37, on Zulip):

for the 2nd run (nll-3) no new ICEs

pnkfelix (Jul 10 2018 at 19:38, on Zulip):

okay that 's good news

lqd (Jul 10 2018 at 19:38, on Zulip):

one of the known ICE was even fixed since the runs started

lqd (Jul 10 2018 at 19:38, on Zulip):

there were lots of "regressions" though

pnkfelix (Jul 10 2018 at 19:38, on Zulip):

which one is the new ICE ? I.e. what is its issue number?

pnkfelix (Jul 10 2018 at 19:38, on Zulip):

(I cannot tell from a quick skim of https://github.com/rust-lang/rust/issues/52217 )

lqd (Jul 10 2018 at 19:38, on Zulip):

unfortunately I myself don't know, niko said so from simulacrum's report

lqd (Jul 10 2018 at 19:39, on Zulip):

air quotes on regressions because a lot of them are actually in upstream crates

lqd (Jul 10 2018 at 19:39, on Zulip):

eg servo's url = 200-300 regr

lqd (Jul 10 2018 at 19:40, on Zulip):

I've started looking at both the runs and started filing issues both for rustc bugs and diagnostics improvements

pnkfelix (Jul 10 2018 at 19:40, on Zulip):

okay good.

lqd (Jul 10 2018 at 19:40, on Zulip):

filtered nll-3 so we can dispatch some of the triage

lqd (Jul 10 2018 at 19:40, on Zulip):

the root regressions' deps are done

lqd (Jul 10 2018 at 19:40, on Zulip):

root regressions with local errors not yet, but I started

lqd (Jul 10 2018 at 19:40, on Zulip):

and that's pretty much it :)

pnkfelix (Jul 10 2018 at 19:41, on Zulip):

okay

lqd (Jul 10 2018 at 19:41, on Zulip):

(the crater topic has a summary issue from sunday with all relevant links, Paper has what I've done since sunday)

lqd (Jul 10 2018 at 19:42, on Zulip):

I could use another pair of eyes on the analyses though

pnkfelix (Jul 10 2018 at 19:42, on Zulip):

@lqd Okay I'll try to add that to my own TODO list

lqd (Jul 10 2018 at 19:42, on Zulip):

esp as some are weird to me, halfway between crate bugs, or feature requests of expansions to 2 phase borrows, etc

lqd (Jul 10 2018 at 19:43, on Zulip):

@pnkfelix awesome thank you

pnkfelix (Jul 10 2018 at 19:43, on Zulip):

So I think I want to take a moment to summarize my understanding of where we are right now

pnkfelix (Jul 10 2018 at 19:43, on Zulip):

which is probably going to sound somewhat similar to where we were before I went on leave

pnkfelix (Jul 10 2018 at 19:43, on Zulip):

As I see it, there are currently four sizable efforts in front of us

pnkfelix (Jul 10 2018 at 19:44, on Zulip):

Three of which one might argue block an attempt to release NLL as part of the next edition preview

pnkfelix (Jul 10 2018 at 19:45, on Zulip):

Those efforts, as I understand them, are: 1. Bringing NLL performance into a "reasonable" distance from AST-borrowck. (What is reasonable? We haven't committed to any target % that I know of.)

pnkfelix (Jul 10 2018 at 19:45, on Zulip):

2. Bringing NLL error diagnostic quality up to par with that of AST borrowck.

pnkfelix (Jul 10 2018 at 19:46, on Zulip):

3. Implementing the -Z borrowck=migrate mode that @Santiago Pastorino is working on (and then turning it on)

lqd (Jul 10 2018 at 19:46, on Zulip):

I think we also have specific GH issues marked for the next preview

lqd (Jul 10 2018 at 19:46, on Zulip):

eg maybe some of the known ICEs

pnkfelix (Jul 10 2018 at 19:47, on Zulip):

4. Moving ahead with Polonius, with the hope that it will provide the more expressive form of NLL that we all have been hoping for without the same performance impact that we originally saw

pnkfelix (Jul 10 2018 at 19:47, on Zulip):

In case its not clear, the three issues that I see as "blocking" releasing NLL as part of an edition preview are 1, 2, and 3.

pnkfelix (Jul 10 2018 at 19:47, on Zulip):

@lqd That is good that they are appropriately marked. Are you aware of any that do not fall into categories 1, 2, or 3?

pnkfelix (Jul 10 2018 at 19:48, on Zulip):

I guess I could just skim them myself, so if you don't know offhand, that's okay, I'll find out. :)

lqd (Jul 10 2018 at 19:48, on Zulip):

I don't know from the top of my head I'll check

pnkfelix (Jul 10 2018 at 19:48, on Zulip):

anyway, I just wanted to lay all that out, mostly to say: Does that jibe with everyone elses' understanding of the situation?

lqd (Jul 10 2018 at 19:49, on Zulip):

doesn't seem like they fall outside of 1 2 3 https://github.com/rust-lang/rust/milestone/52

Matthew Jasper (Jul 10 2018 at 19:49, on Zulip):

Fixing ICEs is 2?

lqd (Jul 10 2018 at 19:49, on Zulip):

it's my understanding of the work ahead, but I'm not sure about the "blocking" aspect though

pnkfelix (Jul 10 2018 at 19:50, on Zulip):

@Matthew Jasper ah that's a good point

Santiago Pastorino (Jul 10 2018 at 19:50, on Zulip):

@pnkfelix what's Polonius plan?

pnkfelix (Jul 10 2018 at 19:51, on Zulip):

@Matthew Jasper hmm. ICE's aren't really a diagnostic quality thing. Or rather, they are at best the worst kind of diagnostic.

Santiago Pastorino (Jul 10 2018 at 19:51, on Zulip):

it's a thing that will happen after edition 2018, right?

pnkfelix (Jul 10 2018 at 19:51, on Zulip):

because all you know is "hmm my code broke the compiler"

Santiago Pastorino (Jul 10 2018 at 19:51, on Zulip):

or that's uncertain?

lqd (Jul 10 2018 at 19:51, on Zulip):

then a point 5. fixing ICEs and bugs ?

pnkfelix (Jul 10 2018 at 19:51, on Zulip):

@Santiago Pastorino From what I understand, Polonius is not near enough to completion for us to attempt to include it in the initial 2018 edition.

Santiago Pastorino (Jul 10 2018 at 19:52, on Zulip):

:+1:

pnkfelix (Jul 10 2018 at 19:52, on Zulip):

@Santiago Pastorino but I also see no reason that we could not deploy it sometime during the lifecycle of the edition; it does not need to wait for a 2021 edition or whatever.

Santiago Pastorino (Jul 10 2018 at 19:52, on Zulip):

yeah

lqd (Jul 10 2018 at 19:52, on Zulip):

if it's released during the next 3 years will it technically be a part of rust 2018 :)

Santiago Pastorino (Jul 10 2018 at 19:52, on Zulip):

I didn't follow exactly what you mean in point 4.

pnkfelix (Jul 10 2018 at 19:53, on Zulip):

@lqd So, I know that some bugs are things that Niko has said "this should not block us from releasing NLL as part of the 2nd preview"

pnkfelix (Jul 10 2018 at 19:53, on Zulip):

@Santiago Pastorino what I meant in Point 4 is that there is still people working on Polonius itself. At least in theory.

lqd (Jul 10 2018 at 19:53, on Zulip):

@pnkfelix btw do you know if those in the preview 2 milestone are some of these non blocking ones ?

pnkfelix (Jul 10 2018 at 19:54, on Zulip):

@Santiago Pastorino but such work is not meant to be coupled with the initial NLL that we deploy in the edition preview

Santiago Pastorino (Jul 10 2018 at 19:54, on Zulip):

makes sense

pnkfelix (Jul 10 2018 at 19:55, on Zulip):

@lqd I don't know offhand, but I would hope not.

lqd (Jul 10 2018 at 19:55, on Zulip):

alright

lqd (Jul 10 2018 at 19:56, on Zulip):

we can sync up with niko next week on that

pnkfelix (Jul 10 2018 at 19:56, on Zulip):

@lqd that is, I would hope that any bug where Niko left a comment of the form "this need not block the edition preview" or "this is [just] a RC blocker" would be on the "Rust 2018 release" milestone

lqd (Jul 10 2018 at 19:56, on Zulip):

yeah we can verify this later no worries

pnkfelix (Jul 10 2018 at 19:56, on Zulip):

@Santiago Pastorino how are you feeling about the -Z borrowck=migrate task?

lqd (Jul 10 2018 at 19:57, on Zulip):

what's the plan for this week ? I was myself thinking of continuing triaging (if someone wants to help do not hesitate)

pnkfelix (Jul 10 2018 at 19:57, on Zulip):

the reason I ask is that, the other tasks on the list (1, 2, and now I guess 5 as well...), they are made up of many "small" bugs, and its possible that future rounds of triage could ... "reprioritize" them to be something for a later milestone. Maybe...

Santiago Pastorino (Jul 10 2018 at 19:57, on Zulip):

@pnkfelix I didn't do much today, tomorrow I will focus on it

lqd (Jul 10 2018 at 19:58, on Zulip):

but if there are different tasks I can help as well, just let me know

Santiago Pastorino (Jul 10 2018 at 19:58, on Zulip):

the main problem is that I need to buffer all the errors on mir borrowck

pnkfelix (Jul 10 2018 at 19:58, on Zulip):

(I don't know how realistic such hypothesized reprioritization actually is; Niko is good about accurately capturing the situation for each bug)

Santiago Pastorino (Jul 10 2018 at 19:58, on Zulip):

that happens in a LOT of places

Santiago Pastorino (Jul 10 2018 at 19:58, on Zulip):

right now ...

Santiago Pastorino (Jul 10 2018 at 19:58, on Zulip):
[santiago@archlinux rust1 (z-borrowck-migrate)]$ rg "\.emit\(\)" src/librustc_mir
src/librustc_mir/transform/qualify_consts.rs
178:            err.emit();
198:            err.emit();
485:                    err.emit()
516:                                    err.emit();
666:                            err.emit();
763:                            err.emit();
786:                        .emit();
805:                    err.emit();
936:                            err.emit();
963:                    err.emit();
1005:                            .emit();
1058:                    err.emit();

src/librustc_mir/borrow_check/mod.rs
294:        err.emit();
327:                .emit();
1365:                .emit();
1883:        err.emit();

src/librustc_mir/transform/check_unsafety.rs
407:    db.emit();
449:                    .emit();

src/librustc_mir/monomorphize/collector.rs
511:        diag.emit();

src/librustc_mir/borrow_check/nll/mod.rs
342:        err.emit();
349:        err.emit();

src/librustc_mir/hair/pattern/check_match.rs
146:                    ).emit();
152:                    ).emit();
241:                    err.emit();
304:            diag.emit();
333:                        err.emit();
390:                                        .emit();
421:                                            .emit();
441:                            err.emit();
503:                .emit();
537:                .emit();
542:                .emit();
548:                    .emit();
606:                    .emit();
617:                    .emit();
648:                        .emit();

src/librustc_mir/hair/pattern/mod.rs
391:                                err.emit();

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
257:        diag.emit();
Santiago Pastorino (Jul 10 2018 at 19:58, on Zulip):

all those cases are missing on my branch

Santiago Pastorino (Jul 10 2018 at 19:59, on Zulip):

after that I guess would be easy :)

Santiago Pastorino (Jul 10 2018 at 19:59, on Zulip):

I still need to see how to pass a buffer or reuse something on all those places

pnkfelix (Jul 10 2018 at 19:59, on Zulip):

@Santiago Pastorino okay, that answers my next question

Santiago Pastorino (Jul 10 2018 at 19:59, on Zulip):

:)

pnkfelix (Jul 10 2018 at 19:59, on Zulip):

(namely, I was wondering how you were planning to rewrite each of those calls to emit())

Santiago Pastorino (Jul 10 2018 at 20:00, on Zulip):
-            err.emit();
+            self.errors_buffer.push(err);
pnkfelix (Jul 10 2018 at 20:00, on Zulip):

Its possible that you might be able to do something like self.signal(err)

lqd (Jul 10 2018 at 20:00, on Zulip):

santiago do you have the context we talked about before available at those locations ?

pnkfelix (Jul 10 2018 at 20:01, on Zulip):

and then have self carry some boolean that decides whether to immediately emit or to buffer.

Santiago Pastorino (Jul 10 2018 at 20:01, on Zulip):

@lqd that's basically the issue, I don't know what do I have

Santiago Pastorino (Jul 10 2018 at 20:01, on Zulip):

it may be very easy

Santiago Pastorino (Jul 10 2018 at 20:01, on Zulip):

I may need to pass some stuff around

pnkfelix (Jul 10 2018 at 20:01, on Zulip):

at the very least, that would allow the initial rewrite of those emit() calls to look trivial ... if an appropriate context is already lying around

Santiago Pastorino (Jul 10 2018 at 20:01, on Zulip):

Its possible that you might be able to do something like self.signal(err)

explain more what do you mean?

lqd (Jul 10 2018 at 20:01, on Zulip):

felix's suggestion sounds nice indeed

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

@Santiago Pastorino well, in the little diff you sketched out

lqd (Jul 10 2018 at 20:02, on Zulip):

(but I don't know that code don't mind me ;)

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

@Santiago Pastorino you hard coded pushing onto the field for the buffer

Santiago Pastorino (Jul 10 2018 at 20:02, on Zulip):

ahh you mean signal emits or stores depending on a bool flag?

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

@Santiago Pastorino and I'm suggesting you move that decision out-of-line

Santiago Pastorino (Jul 10 2018 at 20:02, on Zulip):

ok

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

@Santiago Pastorino exactly.

Santiago Pastorino (Jul 10 2018 at 20:02, on Zulip):

yeah sounds good

lqd (Jul 10 2018 at 20:02, on Zulip):

:thumbs_up:

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

It may not be as nice as we make it out to be

pnkfelix (Jul 10 2018 at 20:02, on Zulip):

but its worth a shot

Santiago Pastorino (Jul 10 2018 at 20:03, on Zulip):

that's probably harder though

Santiago Pastorino (Jul 10 2018 at 20:03, on Zulip):

for instance ... I have one context in Move errors and other one in other places

Santiago Pastorino (Jul 10 2018 at 20:03, on Zulip):

I guess we can talk about this later or tomorrow so we don't take over this meeting :)

pnkfelix (Jul 10 2018 at 20:03, on Zulip):

@Santiago Pastorino well, either way you'll need some sort of contextual object, even if its only a reference to the buffer itself

pnkfelix (Jul 10 2018 at 20:03, on Zulip):

sure

pnkfelix (Jul 10 2018 at 20:04, on Zulip):

I just wanted to dig a little into the task

Santiago Pastorino (Jul 10 2018 at 20:04, on Zulip):

@Santiago Pastorino well, either way you'll need some sort of contextual object, even if its only a reference to the buffer itself

yep

pnkfelix (Jul 10 2018 at 20:04, on Zulip):

because it was something that I know I spent some time looking at, ages ago

pnkfelix (Jul 10 2018 at 20:04, on Zulip):

and also beause

pnkfelix (Jul 10 2018 at 20:04, on Zulip):

its the one thing that I'm pretty sure cannot be "reprioritized"

pnkfelix (Jul 10 2018 at 20:05, on Zulip):

unless we did what Nicholas Nethercote suggested and just switch wholesale from AST-borrowck to NLL without running AST-borrowck at all, ever

pnkfelix (Jul 10 2018 at 20:05, on Zulip):

(but I don't see that happening, in particular for the reasons already stated by Niko here: https://github.com/rust-lang/rust/issues/46908#issuecomment-403061165 )

pnkfelix (Jul 10 2018 at 20:05, on Zulip):

so okay

pnkfelix (Jul 10 2018 at 20:06, on Zulip):

@lqd asked about plans for this week

pnkfelix (Jul 10 2018 at 20:06, on Zulip):

If I am interpreting the triage Paper correclty

pnkfelix (Jul 10 2018 at 20:07, on Zulip):

it sounds like no one is currently looking at performance stuff, apart from @David Wood

davidtwco (Jul 10 2018 at 20:07, on Zulip):

My current PR aims to reduce memory usage.

davidtwco (Jul 10 2018 at 20:07, on Zulip):

I've no idea if it achieves that because it ICEs fairly quickly.

pnkfelix (Jul 10 2018 at 20:08, on Zulip):

@David Wood right, that falls into the performance bucket in my book. :)

Santiago Pastorino (Jul 10 2018 at 20:08, on Zulip):

@pnkfelix after I finish with this thing I can jump into performance stuff

Santiago Pastorino (Jul 10 2018 at 20:08, on Zulip):

unsure what's available to do there

lqd (Jul 10 2018 at 20:09, on Zulip):

do we have actionable issues for perf ?

Santiago Pastorino (Jul 10 2018 at 20:09, on Zulip):

if there's something defined ... what @lqd have said :)

pnkfelix (Jul 10 2018 at 20:09, on Zulip):

@Santiago Pastorino @lqd that is indeed one of the problems: I'm not sure if we have any immediate ideas for something to do

Santiago Pastorino (Jul 10 2018 at 20:09, on Zulip):

or if it's just let's profile and see what's going on

lqd (Jul 10 2018 at 20:09, on Zulip):

the nll runs seemed important with what was/could be lurking there :)

pnkfelix (Jul 10 2018 at 20:10, on Zulip):

@lqd you mean the crater runs? Yes I agree

lqd (Jul 10 2018 at 20:10, on Zulip):

yeah

pnkfelix (Jul 10 2018 at 20:10, on Zulip):

@lqd do you have any notion of how much remaining triage work is left for you there?

lqd (Jul 10 2018 at 20:11, on Zulip):

20-30 maybe

lqd (Jul 10 2018 at 20:11, on Zulip):

let met get you the link

lqd (Jul 10 2018 at 20:11, on Zulip):

https://hackmd.io/2sR4vloZQhC8arJkFOFyLA?both

lqd (Jul 10 2018 at 20:12, on Zulip):

theres around 30 in there and I've done a bunch already

lqd (Jul 10 2018 at 20:12, on Zulip):

eyeballing it, probably less than 15

pnkfelix (Jul 10 2018 at 20:12, on Zulip):

okay

lqd (Jul 10 2018 at 20:12, on Zulip):

but ofc if a pro like yourself could double check that'd be reassuring :)

pnkfelix (Jul 10 2018 at 20:13, on Zulip):

So I think all these plans for this week sound good to me

lqd (Jul 10 2018 at 20:13, on Zulip):

:thumbs_up:

pnkfelix (Jul 10 2018 at 20:17, on Zulip):

okay then I'm going to call it quits for this week's meeting. (and go check on whether my son has managed to fall asleep with all this noise from outside our apartment)

lqd (Jul 10 2018 at 20:18, on Zulip):

good afternoon / evening everyone :wave:

Santiago Pastorino (Jul 10 2018 at 20:18, on Zulip):

:wave:

Last update: Nov 21 2019 at 23:25UTC