Stream: t-compiler

Topic: weekly meeting 2020-02-20 #54818


pnkfelix (Feb 20 2020 at 12:53, on Zulip):

Hi @T-compiler/meeting ; the triage meeting will be starting in 2 hours 7 minutes

pnkfelix (Feb 20 2020 at 12:54, on Zulip):

I will be doing pre-triage in a parallel topic

pnkfelix (Feb 20 2020 at 12:55, on Zulip):

this week we have scheduled check-in's from WG-async-await and WG-diagnostics

pnkfelix (Feb 20 2020 at 12:57, on Zulip):

Hmm I don't know if either of the WG-async-await leads ( @nikomatsakis and @Taylor Cramer ) will be around at check-in time.

nikomatsakis (Feb 20 2020 at 12:57, on Zulip):

@tmandry is the co-lead now, probably web site needs to be updated

pnkfelix (Feb 20 2020 at 12:57, on Zulip):

meanwhile, for WG-diagnostics: @oli and @Esteban Küber , will one of you be available to provide checkin info for WG-diagnostics?

nikomatsakis (Feb 20 2020 at 14:57, on Zulip):

I may be late for meeting today. A few announcements I wanted to make. First, we scheduled upcoming design meetings.

nikomatsakis (Feb 20 2020 at 14:58, on Zulip):

Second, I just created a topic for pre-discussion on triage in #t-compiler/wg-meta > focused and efficient triage compiler-team#247

pnkfelix (Feb 20 2020 at 15:02, on Zulip):

Hi @T-compiler/meeting! Add a :wave: emoji to show you're here :)

pnkfelix (Feb 20 2020 at 15:04, on Zulip):

so, lets spend five minutes on ...

pnkfelix (Feb 20 2020 at 15:04, on Zulip):

Announcements

simulacrum (Feb 20 2020 at 15:04, on Zulip):
pnkfelix (Feb 20 2020 at 15:04, on Zulip):
pnkfelix (Feb 20 2020 at 15:05, on Zulip):

"If a PR may break a build but there is no reason to not roll it up, but is deemed unlikely to, then ..."

that's too many buts for me :)

simulacrum (Feb 20 2020 at 15:06, on Zulip):

heh, complex conditionals indeed

pnkfelix (Feb 20 2020 at 15:06, on Zulip):

what is MIR migrate mode ?

centril (Feb 20 2020 at 15:07, on Zulip):

NLL migrate mode presumably

pnkfelix (Feb 20 2020 at 15:08, on Zulip):

oh, in case its not clear from nikos' message:

pnkfelix (Feb 20 2020 at 15:08, on Zulip):
Santiago Pastorino (Feb 20 2020 at 15:09, on Zulip):

would be nice to add the rollups guidance to forge as XAMPPRocky pointed out

centril (Feb 20 2020 at 15:09, on Zulip):

@Santiago Pastorino it's already there

centril (Feb 20 2020 at 15:09, on Zulip):

it was largely copied from the forge

Santiago Pastorino (Feb 20 2020 at 15:09, on Zulip):

ahh cool :)

pnkfelix (Feb 20 2020 at 15:10, on Zulip):

okay, so, onto the meeting itself

pnkfelix (Feb 20 2020 at 15:10, on Zulip):

I have a hackmd here for today's meeting

pnkfelix (Feb 20 2020 at 15:10, on Zulip):

first up, we have five beta nominations

pnkfelix (Feb 20 2020 at 15:10, on Zulip):

beta-nom 1/5 "Revert "Remove checked_add in Layout::repeat“” #69241

centril (Feb 20 2020 at 15:11, on Zulip):

This is not fixing the root cause right?

pnkfelix (Feb 20 2020 at 15:11, on Zulip):

so this is interesting because its working around an LLVM bug

pnkfelix (Feb 20 2020 at 15:11, on Zulip):

it definitely seems weird to isolate this one thing

centril (Feb 20 2020 at 15:12, on Zulip):

Still; better to paper over to avoid a known case of unsoundness than nothing imo

pnkfelix (Feb 20 2020 at 15:12, on Zulip):

but if it does the trick, and this change wasn't super crucial to performance in the first place

pnkfelix (Feb 20 2020 at 15:12, on Zulip):

then I'm fine with a backport.

Santiago Pastorino (Feb 20 2020 at 15:13, on Zulip):

doesn't worth a FIXME there to easily revisit it later?

pnkfelix (Feb 20 2020 at 15:13, on Zulip):

centril said:

This is not fixing the root cause right?

but yes, as noted in #69225, the root cause is LLVM mis-optimization. We will likely address that via an upgrade to LLVM 10 (see PR #67759 )

centril (Feb 20 2020 at 15:13, on Zulip):

seems fine to add a fixme, but that seems separate from backport question though

Santiago Pastorino (Feb 20 2020 at 15:13, on Zulip):

yep

centril (Feb 20 2020 at 15:14, on Zulip):

( @Santiago Pastorino if you want to file such a PR "revisit ..." I'll happily r+ )

pnkfelix (Feb 20 2020 at 15:14, on Zulip):

I think as long as we track the follow-up work in some way, I don't particularly care whether we do it via a FIXME or just an (assigned) issue.

pnkfelix (Feb 20 2020 at 15:14, on Zulip):

(I literally mean "I don't care" as in I'm fine with such a modification.)

centril (Feb 20 2020 at 15:15, on Zulip):

sounds like beta-accepted?

pnkfelix (Feb 20 2020 at 15:15, on Zulip):

anyway it seems like we have beta-accepted, yeah

pnkfelix (Feb 20 2020 at 15:15, on Zulip):

beta nom 2/5: “Do not ICE when encountering yield inside async block” #69175

centril (Feb 20 2020 at 15:17, on Zulip):

bug -> delay_span_bug basically; seems fine

pnkfelix (Feb 20 2020 at 15:17, on Zulip):

yeah this seems like a clear beta-accept

pnkfelix (Feb 20 2020 at 15:18, on Zulip):

beta-nom 3/5: ""Account for bounds and asociated items when denying _"" #69148

pnkfelix (Feb 20 2020 at 15:19, on Zulip):

this ... seems good ...

centril (Feb 20 2020 at 15:19, on Zulip):

although fiddling with astconv makes me more nervous :P

pnkfelix (Feb 20 2020 at 15:20, on Zulip):

its a bit heavier than the previous PR we were looking at. :)

pnkfelix (Feb 20 2020 at 15:20, on Zulip):

honestly

pnkfelix (Feb 20 2020 at 15:20, on Zulip):

I might be fine with just letting this lie?

pnkfelix (Feb 20 2020 at 15:20, on Zulip):

as in, let it ride the trains

centril (Feb 20 2020 at 15:20, on Zulip):

@Esteban Küber how confident are you about the change? :D

pnkfelix (Feb 20 2020 at 15:20, on Zulip):

let me check the issues in Question

pnkfelix (Feb 20 2020 at 15:21, on Zulip):

see if there is some big reason to rush a backport, rather than let it conitnue to ICE

nikomatsakis (Feb 20 2020 at 15:21, on Zulip):

/me arrives and skims the PR

nikomatsakis (Feb 20 2020 at 15:21, on Zulip):

I was thinking the same thing

centril (Feb 20 2020 at 15:21, on Zulip):

it's sorta unstable anyways

centril (Feb 20 2020 at 15:21, on Zulip):

err, wrong PR... I take that back

nikomatsakis (Feb 20 2020 at 15:21, on Zulip):

the ICE is pretty bad

nikomatsakis (Feb 20 2020 at 15:22, on Zulip):

but it has to do with people using _ in bounds?

pnkfelix (Feb 20 2020 at 15:22, on Zulip):

ah I see, yeah okay this regressed in PR #67597, as reported in issue #68801

nikomatsakis (Feb 20 2020 at 15:23, on Zulip):

My inclination is still to let it ride the trains

nikomatsakis (Feb 20 2020 at 15:23, on Zulip):

How long until next release / beta branch?

pnkfelix (Feb 20 2020 at 15:23, on Zulip):

anyone want to argue the other side? Is @Esteban Küber around to push for a backport?

centril (Feb 20 2020 at 15:23, on Zulip):

@nikomatsakis March 12th 2020 UTC

nikomatsakis (Feb 20 2020 at 15:23, on Zulip):

(because plausibly we could see if we get experience from nightly)

pnkfelix (Feb 20 2020 at 15:24, on Zulip):

lets decline for backport

pnkfelix (Feb 20 2020 at 15:24, on Zulip):

but state on ticket that if someone wants to fight us, we've got our fists ready

pnkfelix (Feb 20 2020 at 15:24, on Zulip):

(I'm joking, but my point is, I don't want to act like its a final decision?)

pnkfelix (Feb 20 2020 at 15:25, on Zulip):

or at least that we are reasonable people.

pnkfelix (Feb 20 2020 at 15:25, on Zulip):

anyway

nikomatsakis (Feb 20 2020 at 15:25, on Zulip):

seems good. "current decision: no backport because too big, but let us know if you want us to revisit the question"

pnkfelix (Feb 20 2020 at 15:26, on Zulip):

beta nom 4/5: "Fix MIR typeck soundness holes" #69145

pnkfelix (Feb 20 2020 at 15:28, on Zulip):

(sorry, I got distracted by the shiny code and didn't post the emoji votes as fast as I should have)

nikomatsakis (Feb 20 2020 at 15:28, on Zulip):

ugh, I guess I'm behind on reviewing that (haven't had any time for reviews this week)

pnkfelix (Feb 20 2020 at 15:28, on Zulip):

oh this hasn't even merged, heh.

centril (Feb 20 2020 at 15:28, on Zulip):

would be good to get it in nightly soon yeah; I think it's important that we backport this as well

centril (Feb 20 2020 at 15:28, on Zulip):

(including stable backport)

pnkfelix (Feb 20 2020 at 15:29, on Zulip):

we'

pnkfelix (Feb 20 2020 at 15:29, on Zulip):

we'll discuss stable backports in a minute

pnkfelix (Feb 20 2020 at 15:29, on Zulip):

but yeah I think this looks good for beta

pnkfelix (Feb 20 2020 at 15:29, on Zulip):

@nikomatsakis would you prefer to table this until after it lands on nightly?

centril (Feb 20 2020 at 15:29, on Zulip):

perhaps @pnkfelix has more time to review it?

pnkfelix (Feb 20 2020 at 15:29, on Zulip):

(given that we don't branch beta until march, right?)

nikomatsakis (Feb 20 2020 at 15:30, on Zulip):

why is it imp't to stable backport it?

pnkfelix (Feb 20 2020 at 15:30, on Zulip):

yeah I can do the review, I pretty much already have done so, effectively.

nikomatsakis (Feb 20 2020 at 15:30, on Zulip):

that is, is there some particular reason?

centril (Feb 20 2020 at 15:30, on Zulip):

@nikomatsakis it's a soundness hole; allowing code to compile that shouldn't

nikomatsakis (Feb 20 2020 at 15:30, on Zulip):

or because soundness

centril (Feb 20 2020 at 15:30, on Zulip):

and could cause forward compat issues if we don't plug it quickly

pnkfelix (Feb 20 2020 at 15:31, on Zulip):

I'm not sure soundness alone is always enough to justify backports

pnkfelix (Feb 20 2020 at 15:31, on Zulip):

we have lots of soundness holes

nikomatsakis (Feb 20 2020 at 15:31, on Zulip):

is it a recent regression, or long-standing problem?

pnkfelix (Feb 20 2020 at 15:31, on Zulip):

its always a tradeoff, is all I'm saying

centril (Feb 20 2020 at 15:31, on Zulip):

@nikomatsakis yes, recent regression

centril (Feb 20 2020 at 15:31, on Zulip):

introduced in 1.41.0

nikomatsakis (Feb 20 2020 at 15:31, on Zulip):

(anyway it seems like a fairly self-contained patch, just trying to understand full situation)

nikomatsakis (Feb 20 2020 at 15:31, on Zulip):

ok

pnkfelix (Feb 20 2020 at 15:32, on Zulip):

So, beta-approved then? Or do you want to wait a week?

pnkfelix (Feb 20 2020 at 15:32, on Zulip):

(assuming that you and or I manage to actually win the review race within a week)

nikomatsakis (Feb 20 2020 at 15:33, on Zulip):

I propose we land it on nightly first, but I think I can review it today

pnkfelix (Feb 20 2020 at 15:33, on Zulip):

ok we'll wait

nikomatsakis (Feb 20 2020 at 15:33, on Zulip):

butI think it seems backportble

pnkfelix (Feb 20 2020 at 15:33, on Zulip):

beta-nom 5/5: “Make conflicting_repr_hints a deny-by-default c-future-compat lint” #68586

centril (Feb 20 2020 at 15:33, on Zulip):

(I'd like to highlight that waiting makes stable backport harder.)

pnkfelix (Feb 20 2020 at 15:34, on Zulip):

ah, there is a point release being proposed, isn't there?

centril (Feb 20 2020 at 15:34, on Zulip):

@pnkfelix it's pretty likely yes

pnkfelix (Feb 20 2020 at 15:34, on Zulip):

that does change the calculus here a bit

nikomatsakis (Feb 20 2020 at 15:34, on Zulip):

at the same time, backporting to insta-table without even trying on nightly...

pnkfelix (Feb 20 2020 at 15:35, on Zulip):

(probably should nclude note of such things in the initial announcements at meeting...)

nikomatsakis (Feb 20 2020 at 15:35, on Zulip):

(that said I just read the PR now and I feel pretty good about it :)

pnkfelix (Feb 20 2020 at 15:35, on Zulip):

@nikomatsakis I'd say we get the nightly landed first, and any approval here is predicated on that?

pnkfelix (Feb 20 2020 at 15:35, on Zulip):

as in, we don't want to wait a week. but it hopefully could wait a day?

centril (Feb 20 2020 at 15:36, on Zulip):

sounds fair

simulacrum (Feb 20 2020 at 15:36, on Zulip):

We are just three weeks out so I'd say it's pretty now or never wrt to a point release, that said, I also feel uncomfortable with direct to stable

simulacrum (Feb 20 2020 at 15:36, on Zulip):

Point release probably Thursday next week or so

pnkfelix (Feb 20 2020 at 15:36, on Zulip):

okay well maybe lets finish discssion of beta-noms

simulacrum (Feb 20 2020 at 15:36, on Zulip):

We'll decide tomorrow in release meeting

pnkfelix (Feb 20 2020 at 15:36, on Zulip):

we'll come back to this when we do the stable-noms

nikomatsakis (Feb 20 2020 at 15:36, on Zulip):

(fyi I just r+'d)

pnkfelix (Feb 20 2020 at 15:37, on Zulip):

so, how about this #68586 ...

pnkfelix (Feb 20 2020 at 15:37, on Zulip):

seems like a simple oversight

nikomatsakis (Feb 20 2020 at 15:37, on Zulip):

this resolves the 2 regressions that @**simulacrum** reported here?

pnkfelix (Feb 20 2020 at 15:37, on Zulip):

(or did we explicitly reject this from being backported ...?)

nikomatsakis (Feb 20 2020 at 15:38, on Zulip):

backport+ from me

centril (Feb 20 2020 at 15:38, on Zulip):

so, how about this #68586 ...

I thought it had made it into beta but I was wrong about the repr hint one

pnkfelix (Feb 20 2020 at 15:38, on Zulip):

@centril do you recall why you removed the T-compiler and beta-nominated labels? I'm guessing it was just ... what does one call it, butter fingers?

centril (Feb 20 2020 at 15:38, on Zulip):

(was right on the edge)

pnkfelix (Feb 20 2020 at 15:39, on Zulip):

oh oh I see

pnkfelix (Feb 20 2020 at 15:39, on Zulip):

okahy

pnkfelix (Feb 20 2020 at 15:39, on Zulip):

beta-accepted then

centril (Feb 20 2020 at 15:39, on Zulip):

(fwiw, beta crater run came in and it was like 2 regressions, so in hindsight I think leaving it as an error would have been fine too)

pnkfelix (Feb 20 2020 at 15:40, on Zulip):

I think we need to have a discussion about how to interpret crater results

pnkfelix (Feb 20 2020 at 15:40, on Zulip):

i..e I think of it as intended to expose bugs

pnkfelix (Feb 20 2020 at 15:41, on Zulip):

but I'm not sure its a great idea to treat it as justification for "backport is unnecessary". Especially when we're talking about a non-risky backport...

centril (Feb 20 2020 at 15:41, on Zulip):

@pnkfelix not making that argument :slight_smile:

pnkfelix (Feb 20 2020 at 15:42, on Zulip):

centril said:

(fwiw, beta crater run came in and it was like 2 regressions, so in hindsight I think leaving it as an error would have been fine too)

don't know how else to read the above, honestly.

pnkfelix (Feb 20 2020 at 15:42, on Zulip):

unless you mean the whole PR was unnecessary

centril (Feb 20 2020 at 15:42, on Zulip):

@pnkfelix before it was landed as a lint on nightly I mean

centril (Feb 20 2020 at 15:42, on Zulip):

but let's move on

pnkfelix (Feb 20 2020 at 15:42, on Zulip):

which is indeed an entirely different argument, though I think my response applieed there as well.

pnkfelix (Feb 20 2020 at 15:42, on Zulip):

okay so,

pnkfelix (Feb 20 2020 at 15:42, on Zulip):

that's all the beta noms

pnkfelix (Feb 20 2020 at 15:43, on Zulip):

let do stable noms

pnkfelix (Feb 20 2020 at 15:43, on Zulip):

stable-nom 1/3 : "Revert "Remove checked_add in Layout::repeat“” #69241

nikomatsakis (Feb 20 2020 at 15:43, on Zulip):

seems as safe as it gets

pnkfelix (Feb 20 2020 at 15:44, on Zulip):

I guess this makes as much sense as the beta backport

pnkfelix (Feb 20 2020 at 15:44, on Zulip):

approved then

nikomatsakis (Feb 20 2020 at 15:44, on Zulip):

apart from "edited a comment"

nikomatsakis (Feb 20 2020 at 15:44, on Zulip):

and those tend to make my build fail more often than not thanks to rustdoc mis-interpreting something as a test ;)

pnkfelix (Feb 20 2020 at 15:44, on Zulip):

stable-nom 2/3 “Do not ICE when encountering yield inside async block” #69175

simulacrum (Feb 20 2020 at 15:45, on Zulip):

I need to run unfortunately, o/

pnkfelix (Feb 20 2020 at 15:46, on Zulip):

okay I guess #69175 is stable accepted too

pnkfelix (Feb 20 2020 at 15:48, on Zulip):

okay so we're back to #69145

pnkfelix (Feb 20 2020 at 15:48, on Zulip):

stable-nom 3/3: “Fix MIR typeck soundness holes” #69145

pnkfelix (Feb 20 2020 at 15:49, on Zulip):

at this point I think we probably are justified in backporting this to stable, with a note that we want the backport to wait until after it lands in nightly?

pnkfelix (Feb 20 2020 at 15:49, on Zulip):

(right?)

pnkfelix (Feb 20 2020 at 15:50, on Zulip):

okay accepted for stable backport

pnkfelix (Feb 20 2020 at 15:50, on Zulip):

(and likewise, beta backport)

nikomatsakis (Feb 20 2020 at 15:50, on Zulip):

sorry, was hesitating :)

nikomatsakis (Feb 20 2020 at 15:50, on Zulip):

but I agree it seems "ok"

centril (Feb 20 2020 at 15:50, on Zulip):

raised to p=10 & set to rollup=never

nikomatsakis (Feb 20 2020 at 15:51, on Zulip):

my hesitation is based on experience in a nightly and not the PR itself, in other words

pnkfelix (Feb 20 2020 at 15:52, on Zulip):

as in, its just bad mojo to backport without having landed in nightly, right?

pnkfelix (Feb 20 2020 at 15:52, on Zulip):

because things you didn't expect always come up, right?

pnkfelix (Feb 20 2020 at 15:52, on Zulip):

anyway that's all the backports and we're a bit short on time

centril (Feb 20 2020 at 15:53, on Zulip):

the PR is probably going to land in 4 hours anyways :P

pnkfelix (Feb 20 2020 at 15:53, on Zulip):

there are three PR's marked waiting on team

pnkfelix (Feb 20 2020 at 15:55, on Zulip):

there's been ongoing discussion about this one, right? "Print nicer async/await trait errors for generators in any place in the error 'stack'" #67116

pnkfelix (Feb 20 2020 at 15:55, on Zulip):

but I think its largely back in the the PR author's hands now, right?

pnkfelix (Feb 20 2020 at 15:55, on Zulip):

As in, we should probably revise the tag to say its not waiting on us anymore ?

pnkfelix (Feb 20 2020 at 15:57, on Zulip):

as for "Rename asm! to llvm_asm! " #68404, I assume that the team its waiting on there is T-lang ?

pnkfelix (Feb 20 2020 at 15:57, on Zulip):

okay well we are almost out of time

pnkfelix (Feb 20 2020 at 15:58, on Zulip):

there was an issue i wanted help prioritizing: "Significant performance regression on the encoding benchmark" #69197

nikomatsakis (Feb 20 2020 at 15:58, on Zulip):

(erp, sorry, I have to run but

nikomatsakis (Feb 20 2020 at 15:58, on Zulip):

pnkfelix said:

as for "Rename asm! to llvm_asm! " #68404, I assume that the team its waiting on there is T-lang ?

yes

nikomatsakis (Feb 20 2020 at 15:58, on Zulip):

pnkfelix said:

but I think its largely back in the the PR author's hands now, right?

yes

centril (Feb 20 2020 at 15:58, on Zulip):

I don't know why it's waiting on T-Lang

centril (Feb 20 2020 at 15:58, on Zulip):

it doesn't seem like a language team issue

centril (Feb 20 2020 at 15:59, on Zulip):

If t-compiler wants to rename the macro then it seems like their call

nikomatsakis (Feb 20 2020 at 16:00, on Zulip):

that confuses me, it is a language feature (albeit an unstable, highly experimental one)

nikomatsakis (Feb 20 2020 at 16:00, on Zulip):

however, I have to run

pnkfelix (Feb 20 2020 at 16:00, on Zulip):

okay well that's an hour.

pnkfelix (Feb 20 2020 at 16:00, on Zulip):

and we didn't get to the WG checkins. :sad:

nikomatsakis (Feb 20 2020 at 16:01, on Zulip):

re: the perf regression, I don't know how to prioritize -- but I feel that in general we should be tracking those like other regressions, so probably I would consider it p-high (but I didn't yet see how big of a regr it is)

pnkfelix (Feb 20 2020 at 16:01, on Zulip):

(on the plus side, I don't think anyone was available from the relevant WG's, apart from @nikomatsakis , who has already said they have to run)

nikomatsakis (Feb 20 2020 at 16:01, on Zulip):

89% max..seems imp't

pnkfelix (Feb 20 2020 at 16:01, on Zulip):

so, thanks for attending all!

centril (Feb 20 2020 at 16:01, on Zulip):

(It's not a language feature that was accepted by an RFC nor intended for stabilization; it's perma-unstable and similar to other rustc_ in my mind)

pnkfelix (Feb 20 2020 at 16:01, on Zulip):

that is, thanks for attending all in @T-compiler/meeting

pnkfelix (Feb 20 2020 at 16:02, on Zulip):

nikomatsakis said:

89% max..seems imp't

okay P-high then

centril (Feb 20 2020 at 16:04, on Zulip):

@eddyb could I assign https://github.com/rust-lang/rust/issues/69306 to you?

eddyb (Feb 20 2020 at 16:10, on Zulip):

maybe @varkor has time for it? I'm already deep in several unfinished things

centril (Feb 20 2020 at 16:11, on Zulip):

@eddyb yeah, very fair

varkor (Feb 20 2020 at 16:11, on Zulip):

I can look into it, but it might not be soon — I've got too much going on IRL at the moment

centril (Feb 20 2020 at 16:11, on Zulip):

I'll assign you then @varkor

Last update: May 29 2020 at 17:50UTC