Stream: t-compiler

Topic: #54818 weekly meeting 2018-11-22


nagisa (Nov 22 2018 at 13:20, on Zulip):

Topic start.

nagisa (Nov 22 2018 at 14:57, on Zulip):

2 minutes and then some till the meeting @T-compiler.

nagisa (Nov 22 2018 at 14:58, on Zulip):

Today is thanksgiving, so at least @nikomatsakis is known to be absent (also @pnkfelix is on a vacation).

nagisa (Nov 22 2018 at 14:58, on Zulip):

Please react somehow so I know who’s participating. If there’s nobody else but me, I might just do some triage myself and call it a day.

nagisa (Nov 22 2018 at 15:00, on Zulip):

Hi all!

nagisa (Nov 22 2018 at 15:00, on Zulip):

Without a further ado…

P-high T-compiler 2 issues.

nagisa (Nov 22 2018 at 15:00, on Zulip):

NLL error on closure, but not on equivalent function #55526 no progress, seems like @nikomatsakis hadn’t an opportunity to look at this, leaving assigned to @nikomatsakis, hopefully next week either him or @pnkfelix looks at this.

nagisa (Nov 22 2018 at 15:01, on Zulip):

regression: stack overflow on macosx with xcode 6.4 #55471 no change here either. We ran a crater and found no genuine regressions. Still some perf issues and portability still needs to be worked out. I hadn’t an opportunity to get back to my portability work, maybe @Oli has anything more to say?

oli (Nov 22 2018 at 15:02, on Zulip):

I'll revert the base-stack sizes and exclude windows for now

oli (Nov 22 2018 at 15:02, on Zulip):

stacker has a PR to fix windows, but it needs a rebase and I don't understand what it does

nagisa (Nov 22 2018 at 15:03, on Zulip):

Okay. Is the stacker PR linked on the rustc PR?

oli (Nov 22 2018 at 15:03, on Zulip):

not yet, will cross-link

nagisa (Nov 22 2018 at 15:03, on Zulip):

Either way this issue won’t be "fully" fixed until we get all platforms working to some extent.

nagisa (Nov 22 2018 at 15:04, on Zulip):

/me shrugs

nagisa (Nov 22 2018 at 15:04, on Zulip):

Moving on,
Rust 2018 Release milestone 2 issues. 1 issue is T-lang, though relevant to us, I think?

nagisa (Nov 22 2018 at 15:04, on Zulip):

Issue 56128 segment id ice nightly #56143. beta-accepted. @Vadim Petrochenkov has reviewed it and left some comments.

nagisa (Nov 22 2018 at 15:04, on Zulip):

spoiler alert: this PR fixes a few issues we’ll talk about later on

mw (Nov 22 2018 at 15:05, on Zulip):

Seems to be WIP still?

mw (Nov 22 2018 at 15:05, on Zulip):

but @nikomatsakis is on it :)

nagisa (Nov 22 2018 at 15:05, on Zulip):

yeah, it seems to still need some work on it

nagisa (Nov 22 2018 at 15:06, on Zulip):

Seems in good hands either way. Does anybody know whether long weekend is a thing in the US?

nagisa (Nov 22 2018 at 15:06, on Zulip):

Best case this will land on Friday.

mw (Nov 22 2018 at 15:06, on Zulip):

"nikomatsakis added some commits 3 hours ago " :P

nagisa (Nov 22 2018 at 15:06, on Zulip):

Next is [beta] resolve: Implement edition hygiene for imports and absolute paths #56053 Crater recently failed with quite a few regressions, which seem to have been anticipated. Seems to be in good hands and covered so no action necessary unless somebody wants to discuss it somehow?

nagisa (Nov 22 2018 at 15:07, on Zulip):

This is otherwise a T-lang PR, so most of the decisions made here are for T-lang to make.

nagisa (Nov 22 2018 at 15:08, on Zulip):

But some of you might want to review the code etc.

nagisa (Nov 22 2018 at 15:08, on Zulip):

which is why I wanted to bring this to y'all’s attention.

nagisa (Nov 22 2018 at 15:09, on Zulip):

Next up,
beta-nominated Most issues reviewed previously in beta-backports thread earlier this week. There is one beta-nominated issue that is yet to be reviewed.

nagisa (Nov 22 2018 at 15:09, on Zulip):

Which is… Forward rust version number to tools #56134 Feels like T-infra/T-tools to me :slight_smile: @Oli you should also label a team along with the beta-nominated flag.

Esteban Küber (Nov 22 2018 at 15:09, on Zulip):

I did some more checking... None of the regressed crates on crates.io seem to have any reverse dependencies (except for one crate which has a reverse dependency due to the same author; the transitive & reflexive closure consists of 1 single reverse dep). The crates also don't seem to have many users. The ecosystem impact of this doesn't seem to be very notable.

I'll take a look at the code, but I'd lean on Centril's assessment there.

oli (Nov 22 2018 at 15:09, on Zulip):

ups, yea, will add teams to nominations from now on

nagisa (Nov 22 2018 at 15:10, on Zulip):

Am I correct that this is T-tools?

oli (Nov 22 2018 at 15:10, on Zulip):

yes, I tagged it as such now

nagisa (Nov 22 2018 at 15:10, on Zulip):

Great!

nagisa (Nov 22 2018 at 15:11, on Zulip):

@Esteban Küber do you want to discuss that PR any further?

nagisa (Nov 22 2018 at 15:11, on Zulip):

Personally to me 64 broken crates seems scary, regardless of what those crates are

Esteban Küber (Nov 22 2018 at 15:12, on Zulip):

@nagisa no, just pointing my agreement with the stated positions in the PR without spamming it :)

nagisa (Nov 22 2018 at 15:12, on Zulip):

Okay. Moving on then,

stable-nominated 0 issues! :high_speed_train:

nagisa (Nov 22 2018 at 15:13, on Zulip):

stable-to-beta-regressions 1 yet unreviewed issue.

nagisa (Nov 22 2018 at 15:13, on Zulip):

Beta ICE: ItemLocalIds not assigned densely #56128 the previously looked-at beta-accepted PR (#56143) fixes this. P-high.

nagisa (Nov 22 2018 at 15:13, on Zulip):

Does P-high seem appropriate to everyone?

nagisa (Nov 22 2018 at 15:15, on Zulip):

The other beta regression is the stack overflow one which also is being worked on (stacker).

nagisa (Nov 22 2018 at 15:15, on Zulip):

stable-to-nightly-regressions 2 issues.

nagisa (Nov 22 2018 at 15:15, on Zulip):

Miscompilation since rustc 1.32.0-nightly (04fdb44f5 2018-11-03) #56069 no minimal test case. Anybody wants to look at this? Smells like they might have UB unsafe code to me.

nagisa (Nov 22 2018 at 15:16, on Zulip):

Might also be worthwhile to wait for the reporter to come up with a smaller reproducer as well

nagisa (Nov 22 2018 at 15:18, on Zulip):

Wrote a comment on the issue indirectly requesting for the reproducer

nagisa (Nov 22 2018 at 15:18, on Zulip):

Panic rustc 1.31.0-nightly (96064eb61 2018-10-28) running on x86_64-pc-windows-msvc: in hir_id_validator.rs:31 #55475 seems to be fixed by previously looked-at beta-accepted PR (#56143). P-high.

nagisa (Nov 22 2018 at 15:18, on Zulip):

Any objections to P-high?

nagisa (Nov 22 2018 at 15:20, on Zulip):

/me decides to drive some conersation by interpreting +1 as an objection (not)

nagisa (Nov 22 2018 at 15:20, on Zulip):

S-waiting-on-team T-compiler 0 issues!

nagisa (Nov 22 2018 at 15:21, on Zulip):

I-nominated T-compiler 3 issues (2 of them nominated for WG-compiler-nll, not us)

nagisa (Nov 22 2018 at 15:21, on Zulip):

Compiler in 1.30.1 stable reliably panics on Windows #56095 P-high, T1 platform. Could be related to windows machines having a tendency to accumulate uptime because it doesn’t do "real" restarts.

nagisa (Nov 22 2018 at 15:21, on Zulip):

I recall mildly Windows having a debug option to start counting uptime from a very high value

mw (Nov 22 2018 at 15:21, on Zulip):

react with a :thumbs_down: if you don't dislike not misinterpreting :+1:s as objections.

nagisa (Nov 22 2018 at 15:22, on Zulip):

(also react with :+1: if you don’t disagree with P-high :slight_smile:)

mw (Nov 22 2018 at 15:22, on Zulip):

this is really a libs issue

mw (Nov 22 2018 at 15:23, on Zulip):

but we should maybe work around it nonetheless

nagisa (Nov 22 2018 at 15:23, on Zulip):

Kind of, yes.

nagisa (Nov 22 2018 at 15:23, on Zulip):

makes me wonder though, why this is coming up only now

nagisa (Nov 22 2018 at 15:23, on Zulip):

Did something change in our compiler implementation? Perhaps there was some recent windows update that broke times?

Esteban Küber (Nov 22 2018 at 15:23, on Zulip):

While we're relabelling, lets also add an O-Windows label

mw (Nov 22 2018 at 15:24, on Zulip):

because -Z self-profile hit stable?

mw (Nov 22 2018 at 15:24, on Zulip):

i.e. the code seems to always be executed, not only if -Zself-profile is enabled

nagisa (Nov 22 2018 at 15:24, on Zulip):

Ah…

mw (Nov 22 2018 at 15:24, on Zulip):

maybe

nagisa (Nov 22 2018 at 15:25, on Zulip):

well, it seems there are two courses of action then:

nagisa (Nov 22 2018 at 15:26, on Zulip):

1. Investigate what change in the compiler caused this to get triggered now, and perhaps workaround it;

mw (Nov 22 2018 at 15:26, on Zulip):

there's a suggested fix here: https://github.com/rust-lang/rust/issues/51648

nagisa (Nov 22 2018 at 15:26, on Zulip):

2. (T-libs) Figure out what causes a monotonically increasing timer to not increase in such a way.

mw (Nov 22 2018 at 15:28, on Zulip):

I think we should:
1. ask @Wesley Wiser to implement the work-around

mw (Nov 22 2018 at 15:28, on Zulip):

2. open a T-libs issue about fn elapsed() panicking unexpectedly

Esteban Küber (Nov 22 2018 at 15:28, on Zulip):

Is there a ticket for the latter?

mw (Nov 22 2018 at 15:28, on Zulip):

I haven't looked actually

nagisa (Nov 22 2018 at 15:29, on Zulip):

I’m not aware of one, but can’t say I’ve looked for it either

Esteban Küber (Nov 22 2018 at 15:29, on Zulip):

https://github.com/rust-lang/rust/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+elapsed+panic nothing other than rustc tickets here

nagisa (Nov 22 2018 at 15:29, on Zulip):

Ah, and for the record, it seems to me that this issue is indeed a duplicate of the linked issue

nagisa (Nov 22 2018 at 15:30, on Zulip):

(by @Wesley Wiser)

nagisa (Nov 22 2018 at 15:30, on Zulip):

regardless of what the reporter says about it being more intermittent.

Esteban Küber (Nov 22 2018 at 15:31, on Zulip):

How often it happens is probably just an interaction with newer code in rustc

nagisa (Nov 22 2018 at 15:32, on Zulip):

Okay, there seems to be some more activity going on, but lets just move on

nagisa (Nov 22 2018 at 15:32, on Zulip):

Add a forever unstable opt-out of const qualification checks #56123 That’s a PR. @eddyb care to elaborate what the question here is?

nagisa (Nov 22 2018 at 15:34, on Zulip):

The conversation in the PR is mostly about the colour of the shed

nagisa (Nov 22 2018 at 15:34, on Zulip):

i.e. what the flag/feature name should be named like.

oli (Nov 22 2018 at 15:34, on Zulip):

the question is whether we want a feature that is unsound to use by definition

oli (Nov 22 2018 at 15:34, on Zulip):

it is great for testing and demonstrating unsoundness

nagisa (Nov 22 2018 at 15:34, on Zulip):

well as long as it is a forever unstable debug feature, why not

oli (Nov 22 2018 at 15:34, on Zulip):

it is always wrong to be used in user-space

nagisa (Nov 22 2018 at 15:35, on Zulip):

demonstrating unsoundness

that doesn’t sound like a good reason to add a debug flag though

nagisa (Nov 22 2018 at 15:35, on Zulip):

I could see myself agreeing to it if it was instead marketed as

testing miri

oli (Nov 22 2018 at 15:35, on Zulip):

well... for RFCs around const stuff it's very hard to explain things

nagisa (Nov 22 2018 at 15:37, on Zulip):

we had weirder debug flags either way

oli (Nov 22 2018 at 15:37, on Zulip):

I'm not gonna market a feature in a way that I'm not planning on using it just to get it merged

nagisa (Nov 22 2018 at 15:37, on Zulip):

@Oli are the demonstrations for the purpose of writing RFCs?

oli (Nov 22 2018 at 15:38, on Zulip):

yes

oli (Nov 22 2018 at 15:38, on Zulip):

mostly giving examples why alternative X won't work

Esteban Küber (Nov 22 2018 at 15:38, on Zulip):

Would "internal unstable" (like the scope inside println) be appropriate for this?

oli (Nov 22 2018 at 15:39, on Zulip):

no, the libstd should never use this feature

oli (Nov 22 2018 at 15:39, on Zulip):

I can in fact add assertions that guarantee it's not used in the libstd

nagisa (Nov 22 2018 at 15:40, on Zulip):

making this feature a flag would make it impossible to misuse within libstd

nagisa (Nov 22 2018 at 15:40, on Zulip):

or most of the code, really

oli (Nov 22 2018 at 15:40, on Zulip):

indeed

oli (Nov 22 2018 at 15:40, on Zulip):

that is a good argument for making it a flag

oli (Nov 22 2018 at 15:40, on Zulip):

I'll bring this up in the PR

nagisa (Nov 22 2018 at 15:41, on Zulip):

#[feature(nll)] made a lot of sense for experiments

nagisa (Nov 22 2018 at 15:41, on Zulip):

but it also was indended to be used by crates

nagisa (Nov 22 2018 at 15:42, on Zulip):

Just saying, that we definitely had (and have) all sorts of weird -Z flags most of which are never used except in very specific situations by few people

nagisa (Nov 22 2018 at 15:43, on Zulip):

Okay, lets move on.

nagisa (Nov 22 2018 at 15:43, on Zulip):

regression-stable-to-stable

Internal Compiler Error in 1.30 and 1.31 #55846 seems to have been fixed in beta, a fix needs to be identified and perhaps backported. Also tested. Unless somebody volunteers, nothing to discuss here.

nagisa (Nov 22 2018 at 15:43, on Zulip):

/me looks around the room for volunteers

nagisa (Nov 22 2018 at 15:44, on Zulip):

Well it’s fixed anyway, :party_ball:!

nagisa (Nov 22 2018 at 15:45, on Zulip):

There also some more issues, but they all seem to be reviewed already.

Esteban Küber (Nov 22 2018 at 15:45, on Zulip):

Finding the PR that fixed it would be nice to verify that a test was added.

Esteban Küber (Nov 22 2018 at 15:45, on Zulip):

Otherwise we'll need to come up with a repro.

nagisa (Nov 22 2018 at 15:46, on Zulip):

We kinda have one, don’t we

nagisa (Nov 22 2018 at 15:47, on Zulip):

(I mean we verified it as fixed using the (quite large, admittedly) piece of code that’s linked to on the issue)

Esteban Küber (Nov 22 2018 at 15:47, on Zulip):

Right, it just needs minifying :)

nagisa (Nov 22 2018 at 15:47, on Zulip):

Well that’s not critical at all :slight_smile:

nagisa (Nov 22 2018 at 15:48, on Zulip):

/me marks as E-help-wanted will write up what needs to be done

varkor (Nov 22 2018 at 15:48, on Zulip):

we could slap E-easy E-mentor on it

eddyb (Nov 22 2018 at 15:48, on Zulip):

thanks, Zulip, notifications only on my phone, which I wasn't looking at...

varkor (Nov 22 2018 at 15:48, on Zulip):

all the E- flags!

nagisa (Nov 22 2018 at 15:49, on Zulip):

that concludes the agenda, mostly (I skipped looking at RFCs etc, which we don’t usually look at)

Esteban Küber (Nov 22 2018 at 15:49, on Zulip):

@varkor I usually slap E-easy until I have time to come back and write mentoring instructions and add E-mentor only then :)

nagisa (Nov 22 2018 at 15:49, on Zulip):

anybody wants to raise a topic for discussion?

Esteban Küber (Nov 22 2018 at 15:49, on Zulip):

https://github.com/rust-lang/rust/pull/55373

varkor (Nov 22 2018 at 15:49, on Zulip):

@Esteban Küber: in this case, the mentoring instructions should be: "reduce this test case as much as possible", so should be easy to write up :P

Esteban Küber (Nov 22 2018 at 15:50, on Zulip):

It's a very much WIP fix to an incorrectly allowed macro definition

nagisa (Nov 22 2018 at 15:50, on Zulip):

/me thinks

Esteban Küber (Nov 22 2018 at 15:50, on Zulip):

The fallout is large

nagisa (Nov 22 2018 at 15:51, on Zulip):

while yes is not really a part of the FOLLOWS list for $EXPR

Esteban Küber (Nov 22 2018 at 15:51, on Zulip):

I'll put in the work to turn it into a proper lint, which will be warn by default

Esteban Küber (Nov 22 2018 at 15:51, on Zulip):

But this will still have a large fallout throughout many macro defining crates

Esteban Küber (Nov 22 2018 at 15:51, on Zulip):

Even rustc was guilty of exploiting this a lot!

nagisa (Nov 22 2018 at 15:52, on Zulip):

Well, it is quite easy to $($x:expr)* :slight_smile:

Esteban Küber (Nov 22 2018 at 15:52, on Zulip):

Indeed

nagisa (Nov 22 2018 at 15:52, on Zulip):

how does it behave currently?

nagisa (Nov 22 2018 at 15:52, on Zulip):

does it greedily consume expressions separated by a space?

Esteban Küber (Nov 22 2018 at 15:53, on Zulip):

IIRC, it does

nagisa (Nov 22 2018 at 15:54, on Zulip):

Seems like a sensible behaviour given the code

nagisa (Nov 22 2018 at 15:54, on Zulip):

It seems sensible to lint this essentially forever

Esteban Küber (Nov 22 2018 at 15:55, on Zulip):

It doesn't https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=c27c674a45d16949c84452504e2e67e4

nagisa (Nov 22 2018 at 15:55, on Zulip):

It seems to work in a greedy manner

nagisa (Nov 22 2018 at 15:56, on Zulip):

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=7dadb31228b7363b3270d83f90ae6505

nagisa (Nov 22 2018 at 15:56, on Zulip):

while sure macro invocation appears to be super ambiguous.

nagisa (Nov 22 2018 at 15:56, on Zulip):

I guess the issue here is compatibility hazard

Esteban Küber (Nov 22 2018 at 15:57, on Zulip):

Yep, I feel that this will have to be a perma-warn lint

nagisa (Nov 22 2018 at 15:58, on Zulip):

oh, and where lints are impossible, it is fine to just warn

nagisa (Nov 22 2018 at 15:58, on Zulip):

especially in cases like these :slight_smile:

nagisa (Nov 22 2018 at 15:58, on Zulip):

does not make sense to give people ability to disable warning about buggy code that should be fixed

Esteban Küber (Nov 22 2018 at 15:58, on Zulip):

We're gonna get a lot of complaints...

nagisa (Nov 22 2018 at 15:58, on Zulip):

or deny(warnings) while at that.

nagisa (Nov 22 2018 at 15:59, on Zulip):

It is a bug.

Esteban Küber (Nov 22 2018 at 15:59, on Zulip):

If we just warn (I'd be down with that), it'd make sense to keep it nightly only for two releases

Esteban Küber (Nov 22 2018 at 15:59, on Zulip):

even though it is a bug fix, widely used crates exploit it

Esteban Küber (Nov 22 2018 at 15:59, on Zulip):

including the generated serde code

Esteban Küber (Nov 22 2018 at 16:00, on Zulip):

keeping it nightly only for two releases should give the community time to move away from using this, and then we can enable it for all channels from then on

Esteban Küber (Nov 22 2018 at 16:00, on Zulip):

what do y'all think?

nagisa (Nov 22 2018 at 16:01, on Zulip):

"it" here being the warning

Esteban Küber (Nov 22 2018 at 16:01, on Zulip):

Correct, a lint wouldn't need this much glove-handling

Esteban Küber (Nov 22 2018 at 16:02, on Zulip):

as each project could just adjust it and carry on as they were at their leisure

nagisa (Nov 22 2018 at 16:03, on Zulip):

Well, thanks all for participating.

Esteban Küber (Nov 22 2018 at 16:03, on Zulip):

Sorry for derailing the ending there ^_^

nagisa (Nov 22 2018 at 16:03, on Zulip):

Its fine, we had time :slight_smile:

nagisa (Nov 22 2018 at 16:03, on Zulip):

doesn’t happen too often

mw (Nov 22 2018 at 16:25, on Zulip):

Thanks for driving the meeting, @nagisa ! :heart:

Last update: Nov 16 2019 at 01:55UTC