Stream: t-compiler/wg-nll

Topic: weekly meeting 2019.03.06


pnkfelix (Mar 06 2019 at 20:31, on Zulip):

hi @WG-compiler-nll

pnkfelix (Mar 06 2019 at 20:32, on Zulip):

so our paper is here as usual

pnkfelix (Mar 06 2019 at 20:32, on Zulip):

We have "three" nominated issues from today's triage, but we have the option of essentially skipping the first two of them

pnkfelix (Mar 06 2019 at 20:33, on Zulip):

“NLL compile-time performance regression” #58178 was nominated because I wanted to assign someone to it

pnkfelix (Mar 06 2019 at 20:33, on Zulip):

but @csmoe volunteered to take it

pnkfelix (Mar 06 2019 at 20:33, on Zulip):

so while I mention it just to let people know that they can feel free to also jump on and help out there if they like

pnkfelix (Mar 06 2019 at 20:33, on Zulip):

it doesn't really need further discussion

pnkfelix (Mar 06 2019 at 20:33, on Zulip):

and then “user type annotations are captured post normalization” #54940

pnkfelix (Mar 06 2019 at 20:34, on Zulip):

is just me pinging @nikomatsakis in a not-subtle-at-all way

pnkfelix (Mar 06 2019 at 20:34, on Zulip):

(I don't expect to get an actual sketch of the plan for #54940 right now.)

pnkfelix (Mar 06 2019 at 20:35, on Zulip):

((but ... maybe it could be filled in asynchronously.... or just poke someone to say something at the T-compiler meeting tomorrow))

nikomatsakis (Mar 06 2019 at 20:35, on Zulip):

drawing up some plans around lazy norm etc is on my list for this week

nikomatsakis (Mar 06 2019 at 20:35, on Zulip):

well, at least taking some steps there. I can certainly post a few updates on the issue.

pnkfelix (Mar 06 2019 at 20:35, on Zulip):

the last issue was put up, I think, by @Matthew Jasper : “More restrictive 2 phase borrows” #58739

pnkfelix (Mar 06 2019 at 20:35, on Zulip):

what happened with the crater run there, @Matthew Jasper ? Something went wrong on the infra side and they restarted it ... and we wait another week?

pnkfelix (Mar 06 2019 at 20:36, on Zulip):

(maybe not a whole week this time?)

pnkfelix (Mar 06 2019 at 20:36, on Zulip):

Oh we got the report back 7 hours ago!

pnkfelix (Mar 06 2019 at 20:36, on Zulip):

/me looked in the morning his time, and got discouraged and didn't look at it again

nikomatsakis (Mar 06 2019 at 20:36, on Zulip):

but csmoe volunteered to take it

what is the story here? I saw some discussion, but I wasn't clear on whether we found a solution

Matthew Jasper (Mar 06 2019 at 20:37, on Zulip):

No, it was waiting for the beta crater runs. Then some other stuff happened that's best ignored. and now we have a report

pnkfelix (Mar 06 2019 at 20:37, on Zulip):

but csmoe volunteered to take it

what is the story here? I saw some discussion, but I wasn't clear on whether we found a solution

you mean for the performance regression?

nikomatsakis (Mar 06 2019 at 20:37, on Zulip):

yes

pnkfelix (Mar 06 2019 at 20:37, on Zulip):

there's no (known) solution that I know of; i think its yet-to-be determined where the remaining performance gap comes from

Matthew Jasper (Mar 06 2019 at 20:37, on Zulip):

but csmoe volunteered to take it

what is the story here? I saw some discussion, but I wasn't clear on whether we found a solution

It's still a large perf regression if you only count the wg-grammar crate. It needs someone to profile things.

nikomatsakis (Mar 06 2019 at 20:38, on Zulip):

No, it was waiting for the beta crater runs. Then some other stuff happened that's best ignored. and now we have a report

nikomatsakis (Mar 06 2019 at 20:39, on Zulip):

(that's the upshot from the report, right?)

pnkfelix (Mar 06 2019 at 20:39, on Zulip):

... that to me says "okay, we can do this if we deploy a warning cycle"

Matthew Jasper (Mar 06 2019 at 20:39, on Zulip):

I'm also not sure if the test is on the live version of perf.rlo (@simulacrum) because rutsc CI has basically never worked today.

pnkfelix (Mar 06 2019 at 20:39, on Zulip):

but I don't know if I'm interpreting that summary correctly

nikomatsakis (Mar 06 2019 at 20:39, on Zulip):

... that to me says "okay, we can do this if we deploy a warning cycle"

that was loosely my take-away too

nikomatsakis (Mar 06 2019 at 20:39, on Zulip):

certainly we can try issuing warnings -- we're still some steps away from going to hard errors on NLL-related things, right?

pnkfelix (Mar 06 2019 at 20:39, on Zulip):

I already was pushing for a warning cycle anyway

pnkfelix (Mar 06 2019 at 20:40, on Zulip):

certainly we can try issuing warnings -- we're still some steps away from going to hard errors on NLL-related things, right?

oh interesting: i was thinking of a proper lint, not something piggy-backed on migrate mode

lqd (Mar 06 2019 at 20:40, on Zulip):

@Matthew Jasper wg-grammar is not present on the prlo status page so maybe it hasn't been deployed after your benchmark was added

nikomatsakis (Mar 06 2019 at 20:41, on Zulip):

oh interesting: i was thinking of a proper lint, not something piggy-backed on migrate mode

I had sort of assumed we'd "tie it in" to migrate mode, even though it doesn't just "fall out" naturally

Matthew Jasper (Mar 06 2019 at 20:41, on Zulip):

I would have changed it to a warning even if the crater run was completely clean. Maybe I was just hoping for too much, but it feels like a lot to break.

pnkfelix (Mar 06 2019 at 20:41, on Zulip):

@Matthew Jasper what do you think, between those two options?

centril (Mar 06 2019 at 20:41, on Zulip):

I concur with this being OK; imo giving the lang team freedom here is pretty important; a warning cycle seems reasonable

Matthew Jasper (Mar 06 2019 at 20:42, on Zulip):

I've pushed a change to a warning mode.

centril (Mar 06 2019 at 20:42, on Zulip):

I nominated it for Thursday's T-Lang meeting

Matthew Jasper (Mar 06 2019 at 20:42, on Zulip):

In the same style as the migrate warnings.

pnkfelix (Mar 06 2019 at 20:42, on Zulip):

its not exactly a migrate mode thing, since one must consider cases where the previous release successfully compiled under NLL and starts failing under NLL and always failed under AST-borrowck

pnkfelix (Mar 06 2019 at 20:42, on Zulip):

that is, I don't think we can trivially piggy-back

nikomatsakis (Mar 06 2019 at 20:42, on Zulip):

confirm, it has to be a separate path in the code, but I think @Matthew Jasper already did that

nikomatsakis (Mar 06 2019 at 20:43, on Zulip):

(if I understood their comment)

Matthew Jasper (Mar 06 2019 at 20:43, on Zulip):

I couldn't find a good way to change a diagnostic to be a lint diagnostic after we've started building it.

centril (Mar 06 2019 at 20:43, on Zulip):

Question: How much does making this a warning set back the timeline on throwing out AST-borrowck and making NLL default everywhere?

nikomatsakis (Mar 06 2019 at 20:43, on Zulip):

I feel like though it's "conceptually" groupable -- the transition to NLL will "require some adjustment and fixes"

nikomatsakis (Mar 06 2019 at 20:44, on Zulip):

I couldn't find a good way to change a diagnostic to be a lint diagnostic after we've started building it.

in other words, if there are other errors, they will be errors, but this will remain a lint?

Matthew Jasper (Mar 06 2019 at 20:44, on Zulip):

Full NLL is IMO a few releases away (>= 1.38), so I think that this should be fine.

nikomatsakis (Mar 06 2019 at 20:44, on Zulip):

Question: How much does making this a warning set back the timeline on throwing out AST-borrowck and making NLL default everywhere?

I don't think there's necessarily any effect -- we still have to go through the exercise of seeing what set of crates break when turn off migrate mode. This would (potentially) increase that number.

pnkfelix (Mar 06 2019 at 20:45, on Zulip):

confirm, it has to be a separate path in the code, but I think Matthew Jasper already did that

(ah, okay; I think the code has changed since I last looked at the diff)

Matthew Jasper (Mar 06 2019 at 20:45, on Zulip):

No, the warning is suppressed by hard errors, and other migrated errors. (I think it's maybe also suppressed by lints, which I should go fix).

pnkfelix (Mar 06 2019 at 20:46, on Zulip):

I'm confused... why would you suppress this if there are other errors emitted?

centril (Mar 06 2019 at 20:47, on Zulip):

(1.38 => 26 September 2019, I think)

Matthew Jasper (Mar 06 2019 at 20:48, on Zulip):

Well, one of the other errors might be from the activation of this borrow, in which case a warning gives potential for confusion with no benefit.

nikomatsakis (Mar 06 2019 at 20:49, on Zulip):

No, the warning is suppressed by hard errors, and other migrated errors. (I think it's maybe also suppressed by lints, which I should go fix).

Suppressed entirely? (as opposed to being converted to an error?)

nikomatsakis (Mar 06 2019 at 20:49, on Zulip):

I suppose @Matthew Jasper we could create one error + one lint initially, then suppress one or the other :)

Matthew Jasper (Mar 06 2019 at 20:50, on Zulip):

(1.38 => 26 September 2019, I think)

And that's assuming CI gets its act together and I can get migrate mode on the 2015 edition this cycle.

nikomatsakis (Mar 06 2019 at 20:50, on Zulip):

I feel like it might be surprising to have the message not appear at all -- in that case, fixing the other errors that you see could cause this "new error" to crop up

pnkfelix (Mar 06 2019 at 20:51, on Zulip):

maybe I'll understand once I actually see examples of the scenario @Matthew Jasper is worried about

Matthew Jasper (Mar 06 2019 at 20:51, on Zulip):

Well, rustc is already like that (although I guess we're moving away from that)

pnkfelix (Mar 06 2019 at 20:52, on Zulip):

Well, rustc is already like that (although I guess we're moving away from that)

yeah I think that's been regarded as a bug or at least architectural limitation, not a goal

pnkfelix (Mar 06 2019 at 20:52, on Zulip):

having said that, I'm pretty excited

pnkfelix (Mar 06 2019 at 20:53, on Zulip):

it will be great to get this in. (right? or at least @RalfJ will be happy...)

Matthew Jasper (Mar 06 2019 at 20:54, on Zulip):

Yes, it's the last PR that isn't r+'d blocking migrate on 2015 edition

RalfJ (Mar 06 2019 at 20:55, on Zulip):

I'm happy that I will be happy thought I am not sure what about :D

Matthew Jasper (Mar 06 2019 at 20:55, on Zulip):

(it would be the last PR full stop if CI didn't break. :upside_down: )

Matthew Jasper (Mar 06 2019 at 20:55, on Zulip):

/rant

pnkfelix (Mar 06 2019 at 20:57, on Zulip):

well, anyway: @Matthew Jasper , what do you think about revising the code so that it doesn't silence the warning?

Matthew Jasper (Mar 06 2019 at 20:57, on Zulip):

Also, while we're discussing this. Do we agree that we should hard error with -Zborrowck=mir, with#![feature(nll)], and on the 2015 edition.

pnkfelix (Mar 06 2019 at 20:58, on Zulip):

I don't know about 2015 edition yet. But the other two cases sound good

centril (Mar 06 2019 at 20:58, on Zulip):

And that's assuming CI gets its act together

There are 259 problems with that assumption.

Matthew Jasper (Mar 06 2019 at 20:58, on Zulip):

well, anyway: Matthew Jasper , what do you think about revising the code so that it doesn't silence the warning?

Except when it's literally a duplicate.

pnkfelix (Mar 06 2019 at 20:58, on Zulip):

(I think i'd prefer to just keep 2018 and 2015 in sync in terms of their use of migrate mode)

pnkfelix (Mar 06 2019 at 20:58, on Zulip):

well, anyway: Matthew Jasper , what do you think about revising the code so that it doesn't silence the warning?

Except when it's literally a duplicate.

it wouldn't be the first time we've let duplicate diagnostics through. But I think we do work to remove those now

Matthew Jasper (Mar 06 2019 at 20:59, on Zulip):

Duplicating an error as a warning feels really bad though

pnkfelix (Mar 06 2019 at 20:59, on Zulip):

I got 259 problems but the bors aint one :musical_notes:

nikomatsakis (Mar 06 2019 at 21:00, on Zulip):

Duplicating an error as a warning feels really bad though

I don't quite understand what this means

nikomatsakis (Mar 06 2019 at 21:01, on Zulip):

what is the duplicate scenario we are talking about?

pnkfelix (Mar 06 2019 at 21:01, on Zulip):

Well, one of the other errors might be from the activation of this borrow, in which case a warning gives potential for confusion with no benefit.

this?

Matthew Jasper (Mar 06 2019 at 21:01, on Zulip):
let x = &a;
a.method(); // warning + error?!
x;
Matthew Jasper (Mar 06 2019 at 21:02, on Zulip):

Anyway, we already deduplicate the other way around, so it shouldn't be too hard.

Matthew Jasper (Mar 06 2019 at 21:06, on Zulip):

Was there anything else that we want to discuss?

lqd (Mar 06 2019 at 21:06, on Zulip):

I was wondering about the WG permissions, the plan is for WGs to be read-only, likely in concert with the triagebot which should take care of assigning people to issues even if they don't have write access

lqd (Mar 06 2019 at 21:08, on Zulip):

I don't need such write access per se, but was still wondering; and e.g. @davidtwco was also in a "worse" situation (having r+ rights)

davidtwco (Mar 06 2019 at 21:09, on Zulip):

It's nice to be able to assign yourself to things.

davidtwco (Mar 06 2019 at 21:09, on Zulip):

I understand that this type of thing is all in transition though, with therust-lang/team effort, new working groups, pending journeypeople stuff and @triagebot.

lqd (Mar 06 2019 at 21:09, on Zulip):

this should soon be handled by the bot I assume, but as you can tell infra is fighting other fires atm

pnkfelix (Mar 06 2019 at 21:09, on Zulip):

we should definitely add some way to ask to "claim" a bug

lqd (Mar 06 2019 at 21:11, on Zulip):

@davidtwco did someone add you to "rust-push" ?

davidtwco (Mar 06 2019 at 21:12, on Zulip):

Not as far as I can tell.

lqd (Mar 06 2019 at 21:12, on Zulip):

(so that at least you're set and then others can be taken care of later)

lqd (Mar 06 2019 at 21:13, on Zulip):

maybe @pnkfelix can help with that

pnkfelix (Mar 06 2019 at 21:14, on Zulip):

I don't know what "rust-push" is but I might nonetheless have access to it. (Great power, no responsibility)

pnkfelix (Mar 06 2019 at 21:14, on Zulip):

/me goes to explore the github UI

lqd (Mar 06 2019 at 21:14, on Zulip):

this team: https://github.com/orgs/rust-lang/teams/rust-push

pnkfelix (Mar 06 2019 at 21:15, on Zulip):

ah, "People who can tag issues!"; that name is not what I expected that to mean.

pnkfelix (Mar 06 2019 at 21:17, on Zulip):

@nikomatsakis is there any reason to not add @davidtwco and @Matthew Jasper to this list? and @Santiago Pastorino for that matter?

pnkfelix (Mar 06 2019 at 21:18, on Zulip):

I'm going to just do it and wait for someone to yell at me about it later

pnkfelix (Mar 06 2019 at 21:18, on Zulip):

oh wait maybe I don't have permissions to do this

pnkfelix (Mar 06 2019 at 21:18, on Zulip):

:oops:

lqd (Mar 06 2019 at 21:20, on Zulip):

@pnkfelix on an unrelated note I'll be away next week (London) so no need to rush on #57374 / notes or anything, except if someone else wants to take it

lqd (Mar 06 2019 at 21:23, on Zulip):

and with that it's probably a good time to wish you all a nice evening/afternoon :) :wave:

nikomatsakis (Mar 07 2019 at 15:56, on Zulip):

nikomatsakis is there any reason to not add davidtwco and Matthew Jasper to this list? and Santiago Pastorino for that matter?

nope

Last update: Nov 21 2019 at 23:40UTC