Stream: t-compiler/wg-nll

Topic: weekly meeting June 19


pnkfelix (Jun 19 2018 at 19:33, on Zulip):

hi @everyone

davidtwco (Jun 19 2018 at 19:33, on Zulip):

o/

nikomatsakis (Jun 19 2018 at 19:33, on Zulip):

are others having problems with the paper doc?

nikomatsakis (Jun 19 2018 at 19:34, on Zulip):

I am :)

nikomatsakis (Jun 19 2018 at 19:34, on Zulip):

(paper has been a bit unreliable over last couple of days for me)

pnkfelix (Jun 19 2018 at 19:34, on Zulip):

((but we are also sitting in a cafe sharing the same questionable wifi))

Santiago Pastorino (Jun 19 2018 at 19:34, on Zulip):

I'm not, I think

davidtwco (Jun 19 2018 at 19:34, on Zulip):

It worked after I refreshed, but initially I got a 500.

nikomatsakis (Jun 19 2018 at 19:35, on Zulip):

do you see my edits?

nikomatsakis (Jun 19 2018 at 19:35, on Zulip):

e.g. a list of possible issues?

nikomatsakis (Jun 19 2018 at 19:35, on Zulip):

it seems to die each time I try to edit, I fear

Santiago Pastorino (Jun 19 2018 at 19:35, on Zulip):
  - nmatsakis: working on query performance still
    - one PR landed
nikomatsakis (Jun 19 2018 at 19:35, on Zulip):

yeah, not those

davidtwco (Jun 19 2018 at 19:35, on Zulip):

No, don't think mine has gone through either. I only see what Santiago just posted.

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

getting 500 now

nikomatsakis (Jun 19 2018 at 19:36, on Zulip):

I added a list with two issues

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

it's definitely having issues

nikomatsakis (Jun 19 2018 at 19:36, on Zulip):

I think paper is dead :( :(

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

yes

nikomatsakis (Jun 19 2018 at 19:36, on Zulip):

I guess we can share briefly important status msgs here :)

pnkfelix (Jun 19 2018 at 19:36, on Zulip):

niko and i were just looking at the NLL-diagnostics list

nikomatsakis (Jun 19 2018 at 19:36, on Zulip):

I'm still working on perf, almost at the end of my patch sequence

nikomatsakis (Jun 19 2018 at 19:37, on Zulip):

(btw cc @Matthew Jasper, you around by any chance?)

nikomatsakis (Jun 19 2018 at 19:38, on Zulip):

ok, well, I think at this point the major focus is to hack on diagnostics issues...

nikomatsakis (Jun 19 2018 at 19:38, on Zulip):

@David Wood, do you still think you'll have time to pursue that region stuff?

nikomatsakis (Jun 19 2018 at 19:38, on Zulip):

we could try to setup a time to sync again

nikomatsakis (Jun 19 2018 at 19:38, on Zulip):

(e.g., tomorrow)

davidtwco (Jun 19 2018 at 19:39, on Zulip):

@nikomatsakis I've definitely got time, just need some guidance on improving the bare-bones categorization that's currently there and then I can look into using those to get the error we want.

pnkfelix (Jun 19 2018 at 19:40, on Zulip):

@pnkfelix is curious about it (the "region stuff",aka issue 51188) because he has been looking into whether there is some low-hanging fruit in related areas there.

nikomatsakis (Jun 19 2018 at 19:40, on Zulip):

ok — I skimmed the PR and it "seemed fine" but I need to actually bring that stuff back into my head a bit I guess

nikomatsakis (Jun 19 2018 at 19:40, on Zulip):

I will make a point to do that later today

pnkfelix (Jun 19 2018 at 19:40, on Zulip):

but if there is a broader solution in the works, then @pnkfelix would be happy to focus elsewhere

pnkfelix (Jun 19 2018 at 19:41, on Zulip):

So to me my main question is what big tasks remain for diagnostics

davidtwco (Jun 19 2018 at 19:41, on Zulip):

Is the issue @pnkfelix just assigned to me similar to the stuff being discussed in the ui/underscore-lifetime/dyn-trait-underscore.rs topic? Or something new?

nikomatsakis (Jun 19 2018 at 19:41, on Zulip):

I think it is similar =)

nikomatsakis (Jun 19 2018 at 19:42, on Zulip):

that's why we assigned it

nikomatsakis (Jun 19 2018 at 19:42, on Zulip):

I think @Santiago Pastorino that https://github.com/rust-lang/rust/issues/51268 might be a good "next issue"

davidtwco (Jun 19 2018 at 19:42, on Zulip):

Cool, just checking :thumbs_up:

nikomatsakis (Jun 19 2018 at 19:42, on Zulip):

same "general area" of the code as what you were just doing

Santiago Pastorino (Jun 19 2018 at 19:42, on Zulip):

same "general area" of the code as what you were just doing

:+1:

pnkfelix (Jun 19 2018 at 19:43, on Zulip):

For diagnostics, some of the work remaining are relatively small fixes to papercuts, such as bugs like https://github.com/rust-lang/rust/issues/51512 which @nikomatsakis just put some great mentoring instructions on

nikomatsakis (Jun 19 2018 at 19:44, on Zulip):

great I don't know

lqd (Jun 19 2018 at 19:45, on Zulip):

(less important status update: opened a couple of polonius PRs, investigated the visit_mir origins, tried removing 2k unused cfg edges from clap and somehow got slower results :)

nikomatsakis (Jun 19 2018 at 19:45, on Zulip):

ah, yes, lqd -- the visit_mir thing --

nikomatsakis (Jun 19 2018 at 19:46, on Zulip):

that was good detective work :) we should see if we can fix that

pnkfelix (Jun 19 2018 at 19:46, on Zulip):

what's the answer?

pnkfelix (Jun 19 2018 at 19:46, on Zulip):

(or in what Zulip thread can we find the answer?)

nikomatsakis (Jun 19 2018 at 19:46, on Zulip):

the theory was that it is this code

davidtwco (Jun 19 2018 at 19:46, on Zulip):

Yeah, I'd be interested in seeing what it was since I couldn't get it to repro here.

nikomatsakis (Jun 19 2018 at 19:46, on Zulip):
    for local in temporary_used_locals {
        let assignments = mbcx.mir.find_assignments(local);
        for location in assignments {
            for moi in &mbcx.move_data.loc_map[location] {
                let mpi = &mbcx.move_data.moves[*moi].path;
                let path = &mbcx.move_data.move_paths[*mpi];
                debug!("assignment of {:?} to {:?}, adding {:?} to used mutable set",
                       path.place, local, path.place);
                if let Place::Local(user_local) = path.place {
                    mbcx.used_mut.insert(user_local);
                }
            }
        }
}
nikomatsakis (Jun 19 2018 at 19:46, on Zulip):

so part of the unused mut lint

lqd (Jun 19 2018 at 19:47, on Zulip):

@pnkfelix specifically, mbcx.mir.find_assignments(local)

nikomatsakis (Jun 19 2018 at 19:47, on Zulip):

tl;dr find_assignments is not cheap

nikomatsakis (Jun 19 2018 at 19:47, on Zulip):

this is an O(n^2) loop

nikomatsakis (Jun 19 2018 at 19:47, on Zulip):

it could easily be linear

nikomatsakis (Jun 19 2018 at 19:48, on Zulip):

so this is a concrete easy to fix bug

nikomatsakis (Jun 19 2018 at 19:48, on Zulip):

for somebody who wants to tackle a perf bug :)

nikomatsakis (Jun 19 2018 at 19:48, on Zulip):

I or @pnkfelix can try to write up somethig, but the basic idea is this:

Santiago Pastorino (Jun 19 2018 at 19:48, on Zulip):

I can help there if nobody wants to do it :)

nikomatsakis (Jun 19 2018 at 19:48, on Zulip):

today we are iterating over all variables and, for each variable, over the MIR

lqd (Jun 19 2018 at 19:49, on Zulip):

same

nikomatsakis (Jun 19 2018 at 19:49, on Zulip):

we would rather iterate once over the MIR and collect this data

nikomatsakis (Jun 19 2018 at 19:49, on Zulip):

ok, let's file an issue related to this

Matthew Jasper (Jun 19 2018 at 19:50, on Zulip):

@nikomatsakis I'm here.

nikomatsakis (Jun 19 2018 at 19:50, on Zulip):

@Matthew Jasper I wanted to check in on https://github.com/rust-lang/rust/issues/51217

pnkfelix (Jun 19 2018 at 19:52, on Zulip):

Okay great; it sounds like fixing that O(n^2) loop will take care of the 7% listed on the clap-rs perf results on https://paper.dropbox.com/doc/NLL-Performance-Tracking-Doc-dRlUMiWT8eexf80FTrYj8

pnkfelix (Jun 19 2018 at 19:53, on Zulip):

of course the real hog there is type-check-internal, which @nikomatsakis is still looking at

nikomatsakis (Jun 19 2018 at 19:53, on Zulip):

Ok I filed https://github.com/rust-lang/rust/issues/51641

nikomatsakis (Jun 19 2018 at 19:53, on Zulip):

to represent that loop

nikomatsakis (Jun 19 2018 at 19:53, on Zulip):

so we should assign either @lqd or @Santiago Pastorino — maybe @lqd as they've been tracking it thus far?

Matthew Jasper (Jun 19 2018 at 19:54, on Zulip):

I'm probably going to get to it after the other issues that I assigned myself (which are just waiting to be rebased over https://github.com/rust-lang/rust/pull/51275). I should have something running locally by next week.

nikomatsakis (Jun 19 2018 at 19:54, on Zulip):

I'm a bit torn on whether to fold that loop into some other walk or what — it feels silly to have Yet Another Walk of the MIR

lqd (Jun 19 2018 at 19:54, on Zulip):

@nikomatsakis I'm not in the GH orgs yet IIRC, otherwise, sure :)

nikomatsakis (Jun 19 2018 at 19:54, on Zulip):

oh, didn't I invite you?

nikomatsakis (Jun 19 2018 at 19:54, on Zulip):

I can fix that...

nikomatsakis (Jun 19 2018 at 19:55, on Zulip):

you are @lqd also on GH, right?

lqd (Jun 19 2018 at 19:55, on Zulip):

yeah

nikomatsakis (Jun 19 2018 at 19:56, on Zulip):

so yeah at this point, other than type-check-internal and this...

nikomatsakis (Jun 19 2018 at 19:56, on Zulip):

it's not clear what else to do on perf; there is perhaps a bit more

nikomatsakis (Jun 19 2018 at 19:56, on Zulip):

njn was doing some investigations for example

nikomatsakis (Jun 19 2018 at 19:57, on Zulip):

they found a hot loop or two

nikomatsakis (Jun 19 2018 at 19:57, on Zulip):

my guess is that we'll want to wait until this work lands and reprofile

pnkfelix (Jun 19 2018 at 19:58, on Zulip):

Right I don't think we've identified any other hot spots to attack

pnkfelix (Jun 19 2018 at 19:59, on Zulip):

from our profiling

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

@nikomatsakis assign it to @lqd

nikomatsakis (Jun 19 2018 at 19:59, on Zulip):

done

lqd (Jun 19 2018 at 19:59, on Zulip):

@Santiago Pastorino alright I just did

pnkfelix (Jun 19 2018 at 19:59, on Zulip):

So I just want to double-check

nikomatsakis (Jun 19 2018 at 19:59, on Zulip):

I guess then the next thing to focus on is prob diagnostics?

pnkfelix (Jun 19 2018 at 19:59, on Zulip):

Since the two main pillars are diagnostics and performance

pnkfelix (Jun 19 2018 at 19:59, on Zulip):

yeah

nikomatsakis (Jun 19 2018 at 20:00, on Zulip):

(I was wondering recently if we should be putting more energy into various "sound/complete" checks — e.g., trying to bootstrap with NLL enabled, crater runs etc)

nikomatsakis (Jun 19 2018 at 20:00, on Zulip):

but I think keeping our focus on diagnostics+perf is still prob right

pnkfelix (Jun 19 2018 at 20:00, on Zulip):

Regarding diagnostics, what "major architectural efforts" are underway or necessary?

pnkfelix (Jun 19 2018 at 20:01, on Zulip):

I.e., I know about the small papercuts that should be easy to fix

pnkfelix (Jun 19 2018 at 20:01, on Zulip):

and I just finished what I regard as one "major rewrite" in my change to check_access_permissions

nikomatsakis (Jun 19 2018 at 20:01, on Zulip):

hmm. well, I think the work that @David Wood and I have been doing on regions probably counts

pnkfelix (Jun 19 2018 at 20:01, on Zulip):

Right, that's one

nikomatsakis (Jun 19 2018 at 20:01, on Zulip):

we were talking about the possibility of threading span information through on each Place

nikomatsakis (Jun 19 2018 at 20:01, on Zulip):

that would be pretty major

nikomatsakis (Jun 19 2018 at 20:02, on Zulip):

but I don't know whether we consider that necessary? it seems like eventually it may be, depending on how precise we want our spans to be

pnkfelix (Jun 19 2018 at 20:02, on Zulip):

what would that span represent?

pnkfelix (Jun 19 2018 at 20:02, on Zulip):

some first initialization of the place?

nikomatsakis (Jun 19 2018 at 20:02, on Zulip):

well what I imagined was rather that each mir::Place would carry the span from where it appears in the source

pnkfelix (Jun 19 2018 at 20:02, on Zulip):

or its declaration point?

pnkfelix (Jun 19 2018 at 20:02, on Zulip):

hmm

nikomatsakis (Jun 19 2018 at 20:02, on Zulip):

that in turn implies that the borrowck can't use the Place internally to designate a path

nikomatsakis (Jun 19 2018 at 20:02, on Zulip):

and would instead need to use MovePathIndex or some such thing

nikomatsakis (Jun 19 2018 at 20:03, on Zulip):

so e.g. if you had let a = 22; foo(a);

nikomatsakis (Jun 19 2018 at 20:03, on Zulip):

those two uses of a would have distinct spans

nikomatsakis (Jun 19 2018 at 20:03, on Zulip):

(in the MIR)

pnkfelix (Jun 19 2018 at 20:03, on Zulip):

okay I see

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

(it occurs to me that this might tie in with the changes that @Eduard-Mihai Burtescu was talking about for changing MIR to not use Place but rather build up the paths with statements)

pnkfelix (Jun 19 2018 at 20:04, on Zulip):

this is an alternative to my pattern of passing around a (Place, Span) tuple, yes?

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

anyway I feel like this is probably out of scope right now?

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

this is an alternative to my pattern of passing around a (Place, Span) tuple, yes?

yes, it would basically replace that I guess

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

the problem is that today we have the span of the statement

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

but we can't get more precise

nikomatsakis (Jun 19 2018 at 20:04, on Zulip):

still, I feel like I've not seen a lot of issues where this seems to be the key problem?

nikomatsakis (Jun 19 2018 at 20:05, on Zulip):

perhaps because we introduce enough temporaries :)

pnkfelix (Jun 19 2018 at 20:06, on Zulip):

I'm going to skim over the NLL-diagnostics list again to double-check whether there are other issues that seem like they obviously need some other solution than the ones listed abover

nikomatsakis (Jun 19 2018 at 20:06, on Zulip):

yeah I'm skimming now

nikomatsakis (Jun 19 2018 at 20:07, on Zulip):

so is https://github.com/rust-lang/rust/issues/51175 the issue @pnkfelix that we were talking about which sort of overlaps with what @David Wood and I were doing?

pnkfelix (Jun 19 2018 at 20:07, on Zulip):

@nikomatsakis yeah probably

pnkfelix (Jun 19 2018 at 20:07, on Zulip):

I'm not sure why I gave that a separate issue

nikomatsakis (Jun 19 2018 at 20:07, on Zulip):

seems fine

davidtwco (Jun 19 2018 at 20:08, on Zulip):

So are there currently three issues based on this work? The original #46983, #51188 that was assigned to me earlier and #51175?

nikomatsakis (Jun 19 2018 at 20:08, on Zulip):

sounds right

nikomatsakis (Jun 19 2018 at 20:10, on Zulip):

this is also related https://github.com/rust-lang/rust/issues/51012

nikomatsakis (Jun 19 2018 at 20:12, on Zulip):

ok well I guess everybody has something to do :)

nikomatsakis (Jun 19 2018 at 20:12, on Zulip):

there are still a lot of diagnostic issues, it's sort of hard to tell what is easiest, just have to dig into them

pnkfelix (Jun 19 2018 at 20:13, on Zulip):

Okay when/if Paper is working again then we should all try to post status reports.

nikomatsakis (Jun 19 2018 at 20:13, on Zulip):

er right @Matthew Jasper you were also working on https://github.com/rust-lang/rust/issues/45699, right ?

nikomatsakis (Jun 19 2018 at 20:13, on Zulip):

(moves and patterns)

davidtwco (Jun 19 2018 at 20:13, on Zulip):

I think it might be, at least, I think mine has went in now.

nikomatsakis (Jun 19 2018 at 20:13, on Zulip):

did that result in a PR?

Matthew Jasper (Jun 19 2018 at 20:14, on Zulip):

not yet, should be up soon.

pnkfelix (Jun 19 2018 at 20:27, on Zulip):

(in case it wasn't clear, I'm done talking for this meeting. :smile: )

lqd (Jun 19 2018 at 20:40, on Zulip):

I'll keep trying to update the Paper doc, it's half-working right now apparently

lqd (Jun 19 2018 at 20:53, on Zulip):

(at least Dropbox is working on it)

Last update: Nov 21 2019 at 14:40UTC