Stream: t-compiler

Topic: #54818 weekly meeting 2018-12-20


pnkfelix (Dec 20 2018 at 13:24, on Zulip):

hi @T-compiler/meeting ; I'll be doing some pre-meeting triage on this topic area before the meeting itself starts in about 96 minutes

pnkfelix (Dec 20 2018 at 13:27, on Zulip):

first up, P-high issues

pnkfelix (Dec 20 2018 at 13:29, on Zulip):

first "Compiler panics unexpectedly in 1.31 when constructing struct with wrong syntax" #56611

pnkfelix (Dec 20 2018 at 13:31, on Zulip):

this is supposed to be fixed by PR #55994, which has not yet landed due to T-lang requesting revisions on how future compat warning is handled.

pnkfelix (Dec 20 2018 at 13:32, on Zulip):

last week the compiler team also briefly discussed that factoring out just the part that fixes the ICE as its own PR, so that we could backport it.

pnkfelix (Dec 20 2018 at 13:32, on Zulip):

but i don't think there has been movement on PR #55994 since last week.

pnkfelix (Dec 20 2018 at 13:32, on Zulip):

Hey @Alexander Regueiro do you think you'll be updating PR #55994 in the near future?

pnkfelix (Dec 20 2018 at 13:36, on Zulip):

next: "ICE: cannot relate region: LUB(ReErased, ReEmpty)" #56350. The beta backport landed, so closing as fixed.

pnkfelix (Dec 20 2018 at 13:36, on Zulip):

next: "prohibit "two-phase borrows" with existing borrows?" #56254. This is still on my plate. I haven't had a chance to attack it. We'll see how fast I finish this pre-meeting triage.

pnkfelix (Dec 20 2018 at 13:37, on Zulip):

next: "regression: stack overflow on macosx with xcode 6.4" #55471

pnkfelix (Dec 20 2018 at 13:38, on Zulip):

hmm. I made a comment in last week's triage that we should figure out what's going on with the supposed broken-ness, even with PR #56467, unless one uses RUST_MIN_STACK env var.

pnkfelix (Dec 20 2018 at 13:38, on Zulip):

I'm going to assign to self for follow up investigation.

pnkfelix (Dec 20 2018 at 13:40, on Zulip):

next: "1.30.0 fails to build for target powerpc-unknown-netbsd" #55465

pnkfelix (Dec 20 2018 at 13:41, on Zulip):

This should be fixed by Rust 1.31.1, which is supposed to be released today. I'll leave this open until the issue filer (he32) can confirm its fixed.

pnkfelix (Dec 20 2018 at 13:41, on Zulip):

next: "rustc 1.30.0's linker flavor inference is a non-backwards compat change to -Clinker" #55396

davidtwco (Dec 20 2018 at 13:42, on Zulip):

Last PR for that landed, should be resolved.

pnkfelix (Dec 20 2018 at 13:42, on Zulip):

all of the relevant PR's have landed, so I'm closing this as fixed.

pnkfelix (Dec 20 2018 at 13:43, on Zulip):

next: "[nll] _ patterns should not count as borrows" #53114

pnkfelix (Dec 20 2018 at 13:43, on Zulip):

I assigned this to myself last night. I tagged it as P-high solely for the task of adding tests encoding our current behavior (as represented in tables on comments on the issue)

pnkfelix (Dec 20 2018 at 13:43, on Zulip):

So it should be easy to fix that and then bring it back down to P-medium.

pnkfelix (Dec 20 2018 at 13:44, on Zulip):

(and I'm a bit of a zombie today so reviewing+writing tests may either be ideal or terrible task for today...)

pnkfelix (Dec 20 2018 at 13:45, on Zulip):

next: "two-phase-borrows need a specification" #46901. This is lumped in with #56254. I'll be taking action on it soon, i promise.

pnkfelix (Dec 20 2018 at 13:45, on Zulip):

finally: "figure out how to integrate constants and the MIR type checker" #46702

pnkfelix (Dec 20 2018 at 13:46, on Zulip):

this is yet another NLL issue (I made a bunch of them P-high last night, that's why we're seeing them now). This one is assigned to @nikomatsakis to determine whether there are any actual problems associated with the issue outlined here.

pnkfelix (Dec 20 2018 at 13:46, on Zulip):

so that's all the P-high issues

pnkfelix (Dec 20 2018 at 13:47, on Zulip):

next, beta-nominations

pnkfelix (Dec 20 2018 at 13:48, on Zulip):

first up: "Fixed issue with using Self ctor in typedefs" #56850

pnkfelix (Dec 20 2018 at 13:48, on Zulip):

actually I won't attempt to pre-triage these.

pnkfelix (Dec 20 2018 at 13:49, on Zulip):

Or at least, I'm not sure it makes as much sense to do so. The whole point of the beta-nom process is to give everyone chance to review them and weigh in on risk/reward for backporting

pnkfelix (Dec 20 2018 at 13:50, on Zulip):

the description for PR #56850 does not actually list the issues it fixes. I 'll at least fix that now

pnkfelix (Dec 20 2018 at 13:54, on Zulip):

Hey @Alexander Regueiro do you think you'll be updating PR #55994 in the near future?

and is PR #56850 somehow related? (Is it the hypothetical factored out fixes for beta backport...?)

pnkfelix (Dec 20 2018 at 13:54, on Zulip):

I added links to #56850 to the two issues that it adds to the test suite

pnkfelix (Dec 20 2018 at 13:55, on Zulip):

the regression that PR #56850 explicitly claims to fix is #56835 (which is only tagged as a stable-to-stable regression, not T-compiler)

pnkfelix (Dec 20 2018 at 13:57, on Zulip):

(I guess this series of notes shows that there is some value in pre-triage of beta-nominated PRs, in at least trying to add meta-data to PR's as appropriate...)

pnkfelix (Dec 20 2018 at 13:58, on Zulip):

based on PR #56919, adding T-compiler label to #56800

Alexander Regueiro (Dec 20 2018 at 13:58, on Zulip):

@pnkfelix Hmm, I thought that was a stable-to-beta regression.

pnkfelix (Dec 20 2018 at 13:58, on Zulip):

@Alexander Regueiro "that" being which? There have been a couple different issues mentioned since I pinged you

Alexander Regueiro (Dec 20 2018 at 14:00, on Zulip):

@pnkfelix And no, those two PRs are not related. I factored out #56850 from my implementation of type alias enum variants PR.

pnkfelix (Dec 20 2018 at 14:00, on Zulip):

okay, just checking.

pnkfelix (Dec 20 2018 at 14:00, on Zulip):

based on PR #56850, adding T-compiler label to #56199, #56835

Alexander Regueiro (Dec 20 2018 at 14:00, on Zulip):

#56835 I meant

pnkfelix (Dec 20 2018 at 14:01, on Zulip):

okay interesting

pnkfelix (Dec 20 2018 at 14:01, on Zulip):

nagisa added the stable-to-stable label 5 days ago. let me just look

Alexander Regueiro (Dec 20 2018 at 14:02, on Zulip):

Yeah, might be mistaken. Not sure though.

pnkfelix (Dec 20 2018 at 14:02, on Zulip):

beta ICE"s on the test case

pnkfelix (Dec 20 2018 at 14:02, on Zulip):

stable complains that Self struct constructors are unstable, and then ICE's.

pnkfelix (Dec 20 2018 at 14:03, on Zulip):

I suspect the fact that it subsequently ICE's on stable is why @nagisa tagged it as a stable-to-stable regression.

Alexander Regueiro (Dec 20 2018 at 14:04, on Zulip):

And the trait alias PR... I intend to finish that today. We are waiting on a fix for the traitobject crate though. I submitted a PR to it and emailed the author but he’s been AWOL for almost 2 years it seems. His crate is the only real offender on the crater run, i.e. the only obstacle to fixing the bad duplicate traits behaviour and going to a hard error tight away.

Alexander Regueiro (Dec 20 2018 at 14:05, on Zulip):

Yeah. Makes sense.

pnkfelix (Dec 20 2018 at 14:05, on Zulip):

And the trait alias PR... I intend to finish that today. [snip]

Okay, thanks for the update.

pnkfelix (Dec 20 2018 at 14:07, on Zulip):

based on PR #56790, marked issue #56797 as T-compiler

pnkfelix (Dec 20 2018 at 14:07, on Zulip):

(its not a regression; its just a big-old soundness problem)

pnkfelix (Dec 20 2018 at 14:08, on Zulip):

(that is NLL-fixed-by-NLL)

pnkfelix (Dec 20 2018 at 14:09, on Zulip):

okay, that's all the beta nominations

pnkfelix (Dec 20 2018 at 14:09, on Zulip):

there are no stable nominations

pnkfelix (Dec 20 2018 at 14:10, on Zulip):

next up, (all) stable-to-beta regressions

pnkfelix (Dec 20 2018 at 14:10, on Zulip):

only two of these are relevant to us, I think

pnkfelix (Dec 20 2018 at 14:11, on Zulip):

first: " cannot mutate statics in initalizer of another static" #56903

pnkfelix (Dec 20 2018 at 14:12, on Zulip):

we're preventing mutable borrows even just to (unsafely) initialize a *mut. @Oli has a fix in PR #56916. Marking as P-high.

Alexander Regueiro (Dec 20 2018 at 14:13, on Zulip):

@pnkfelix you may want to delete your comment https://github.com/rust-lang/rust/issues/56611#issuecomment-449000949 now. since it's not related to the trait alias PR, and actually fixed in nightly anyway, in a PR I factored out from my enum variants one. :-)

pnkfelix (Dec 20 2018 at 14:13, on Zulip):

okay thanks @Alexander Regueiro i'll go look

oli (Dec 20 2018 at 14:13, on Zulip):

do we want to bump the bors priority on the fix PR #56916, too?

pnkfelix (Dec 20 2018 at 14:14, on Zulip):

@Alexander Regueiro do you recall the PR number for the one you factored out?

pnkfelix (Dec 20 2018 at 14:14, on Zulip):

next (and last) stable-to-beta regression: "Size no longer known at compiletime" #56899

Alexander Regueiro (Dec 20 2018 at 14:15, on Zulip):

@pnkfelix yeah, it was factored out from https://github.com/rust-lang/rust/pull/56225.

pnkfelix (Dec 20 2018 at 14:15, on Zulip):

@Oli its in a rollup

lqd (Dec 20 2018 at 14:15, on Zulip):

@Oli already in a rollup with p=19 (but WIP apparently it's failing) ah felix has answered...

pnkfelix (Dec 20 2018 at 14:15, on Zulip):

@pnkfelix yeah, it was factored out from https://github.com/rust-lang/rust/pull/56225.

sorry, I wasn't clear; I meant I wanted the PR that had just the factored out change

pnkfelix (Dec 20 2018 at 14:16, on Zulip):

/me skims

Alexander Regueiro (Dec 20 2018 at 14:17, on Zulip):

@pnkfelix oh, that's just https://github.com/rust-lang/rust/pull/56850, for which I think you just updated the message. :-)

pnkfelix (Dec 20 2018 at 14:17, on Zulip):

ah okay thanks

pnkfelix (Dec 20 2018 at 14:17, on Zulip):

so ... PR #56850 does fix #56611 (apart from, maybe, missing a unit test) ?

pnkfelix (Dec 20 2018 at 14:18, on Zulip):

@Alexander Regueiro ^

pnkfelix (Dec 20 2018 at 14:18, on Zulip):

I thought you said earlier that it was not related

Alexander Regueiro (Dec 20 2018 at 14:19, on Zulip):

@pnkfelix sorry, I meant the trait alias one was unrelated. some confusion. and yes, it fixes that one too (three issues, I guess). I probably didn't add a separate regression test for it because it was extremely similar to another one.

pnkfelix (Dec 20 2018 at 14:19, on Zulip):

(wait I should be able to readily test this, since PR #56850 landed 4 days ago)

Alexander Regueiro (Dec 20 2018 at 14:19, on Zulip):

perhaps even identical to one of the other two... I forget.

Alexander Regueiro (Dec 20 2018 at 14:19, on Zulip):

yeah heh. it works. :-)

pnkfelix (Dec 20 2018 at 14:21, on Zulip):

okay, where was i

pnkfelix (Dec 20 2018 at 14:21, on Zulip):

I was looking at the stable-to-beta regressions

pnkfelix (Dec 20 2018 at 14:21, on Zulip):

and was on: "Size no longer known at compiletime" #56899

pnkfelix (Dec 20 2018 at 14:21, on Zulip):

marking as P-high

pnkfelix (Dec 20 2018 at 14:23, on Zulip):

okay, next, stable-to-nightly regressions

pnkfelix (Dec 20 2018 at 14:24, on Zulip):

first: "Nightly: thread '<unnamed>' panicked at 'couldn't compile the test', src/librustdoc/test.rs:326:13" #56867

pnkfelix (Dec 20 2018 at 14:24, on Zulip):

nasty

pnkfelix (Dec 20 2018 at 14:24, on Zulip):

(@nagisa encounters a number of difficulties with reproduction)

nagisa (Dec 20 2018 at 14:25, on Zulip):

Icould reproduce the crossbeam issue :slight_smile:

pnkfelix (Dec 20 2018 at 14:25, on Zulip):

(but did find at least one scenario that yields the linker problem)

pnkfelix (Dec 20 2018 at 14:25, on Zulip):

right

pnkfelix (Dec 20 2018 at 14:25, on Zulip):

anyway

pnkfelix (Dec 20 2018 at 14:25, on Zulip):

I'll mark as P-high for now

pnkfelix (Dec 20 2018 at 14:26, on Zulip):

next/last: "nightly rustc --version hangs forever" #56736

pnkfelix (Dec 20 2018 at 14:26, on Zulip):

we talked about this last week I think

pnkfelix (Dec 20 2018 at 14:27, on Zulip):

with this particular juicy quote from @nagisa https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/subject/.2354818.20weekly.20meeting.202018-12-13/near/151616981

pnkfelix (Dec 20 2018 at 14:29, on Zulip):

I don't know what we can do here at this point

pnkfelix (Dec 20 2018 at 14:30, on Zulip):

and its not really a T-compiler question to resolve, from what I'm reading

pnkfelix (Dec 20 2018 at 14:32, on Zulip):

since it is a consequence (if I understand correctly) of using jemalloc in our CI-builds (which are what we distribute) and our CI builds using .... an old linux kernel? At least that's the basis of a claim for why we end up using fcntl instead of O_CLOEXEC ... boy-oh-boy

pnkfelix (Dec 20 2018 at 14:32, on Zulip):

should we foist this off on T-infra? I'll leave a comment saying as much.

pnkfelix (Dec 20 2018 at 14:34, on Zulip):

nothing is waiting for our team

pnkfelix (Dec 20 2018 at 14:34, on Zulip):

there are three I-nominated T-compiler issues

pnkfelix (Dec 20 2018 at 14:35, on Zulip):

I'll let us talk about those at the meeting itself.

pnkfelix (Dec 20 2018 at 14:35, on Zulip):

(So, in case its not clear: my hope is that we can spend the meeting focusing solely on the beta-nominations and the I-nominated issues)

pnkfelix (Dec 20 2018 at 14:37, on Zulip):

(interestingly, none of the I-nominated issues have P-tags)

pnkfelix (Dec 20 2018 at 14:37, on Zulip):

I'll let us figure those out in the meeting itself

pnkfelix (Dec 20 2018 at 15:01, on Zulip):

ping @T-compiler/meeting

pnkfelix (Dec 20 2018 at 15:01, on Zulip):

so you can all read the backlog at your leisure

pnkfelix (Dec 20 2018 at 15:02, on Zulip):

(I am now realizing that perhaps I should be doing the pre-meeting triage in a slightly different order, so that the process of tagging issues with P-high precedes going over the P-high issues themselves...)

pnkfelix (Dec 20 2018 at 15:03, on Zulip):

In particular, because of the act of adding new P-high labels

pnkfelix (Dec 20 2018 at 15:03, on Zulip):

there are now a couple issues tagged as P-high that need to be assigned to someone

pnkfelix (Dec 20 2018 at 15:03, on Zulip):

lets go through at least hose

pnkfelix (Dec 20 2018 at 15:03, on Zulip):

first up: "Size no longer known at compiletime" #56899

pnkfelix (Dec 20 2018 at 15:04, on Zulip):

any takers? at the very least we should bisect

nikomatsakis (Dec 20 2018 at 15:04, on Zulip):

I'll take it

nikomatsakis (Dec 20 2018 at 15:04, on Zulip):

I'll start with bisection

nikomatsakis (Dec 20 2018 at 15:04, on Zulip):

I don't have a P-high issue right now :)

oli (Dec 20 2018 at 15:04, on Zulip):

looks a lot like the recent pin errors (which were fixed)

pnkfelix (Dec 20 2018 at 15:05, on Zulip):

okay, next is "Nightly: thread '<unnamed>' panicked at 'couldn't compile the test', src/librustdoc/test.rs:326:13" #56867

pnkfelix (Dec 20 2018 at 15:05, on Zulip):

@nagisa you were looking at this

pnkfelix (Dec 20 2018 at 15:05, on Zulip):

@nagisa

pnkfelix (Dec 20 2018 at 15:05, on Zulip):

shall I assign to you?

nagisa (Dec 20 2018 at 15:05, on Zulip):

sure but I’m not expecting to have a look at this any time soon

pnkfelix (Dec 20 2018 at 15:06, on Zulip):

as in, you think it might be a couple weeks before you look, due to e.g. holidays?

nagisa (Dec 20 2018 at 15:06, on Zulip):

ah, no, I’m gonna probably have more time to contribute during the holidays. Rather, I do not expect this to be my priority

nagisa (Dec 20 2018 at 15:06, on Zulip):

especially given my analysis

nagisa (Dec 20 2018 at 15:06, on Zulip):

and difficulties in reproducing

pnkfelix (Dec 20 2018 at 15:06, on Zulip):

well lets give it to @nagisa for now and see where we are in a week. Maybe the issue filer will produce a minimized test case in response to your comment.

pnkfelix (Dec 20 2018 at 15:08, on Zulip):

okay all other P-highs have assignees

pnkfelix (Dec 20 2018 at 15:08, on Zulip):

I didn't look at all of them during pre-triage of P-high list, but the remaining ones are e.g. tagged as regressions, so I think they've been looked at in some fashion

pnkfelix (Dec 20 2018 at 15:09, on Zulip):

so lets move along to the two main agenda items for all of us to look at: beta-nominations and I-nominated issues

pnkfelix (Dec 20 2018 at 15:09, on Zulip):

beta-nominations

pnkfelix (Dec 20 2018 at 15:09, on Zulip):

first: "Remove a wrong multiplier on relocation offset computation" #56919

pnkfelix (Dec 20 2018 at 15:10, on Zulip):

seems uncontroversial. Any objections to beta-accepting PR #56919 ?

pnkfelix (Dec 20 2018 at 15:10, on Zulip):

ooh cool choice of emoji @nagisa

nikomatsakis (Dec 20 2018 at 15:11, on Zulip):

I presume that :back: represents "backport"

nikomatsakis (Dec 20 2018 at 15:11, on Zulip):

I'm not entirely sure what the customs officer is meant to be though :)

pnkfelix (Dec 20 2018 at 15:11, on Zulip):

stamp on passport perhaps?

nagisa (Dec 20 2018 at 15:11, on Zulip):

@nikomatsakis passport.

Esteban Küber (Dec 20 2018 at 15:11, on Zulip):

I interpreted it as rubber-stamping

pnkfelix (Dec 20 2018 at 15:11, on Zulip):

@nikomatsakis passport.

oh my

Esteban Küber (Dec 20 2018 at 15:12, on Zulip):

Oohhhh

pnkfelix (Dec 20 2018 at 15:12, on Zulip):

next up: "Fix a recently introduced regression" #56916

pnkfelix (Dec 20 2018 at 15:12, on Zulip):

I gotta say @Oli , that PR title is not your best work.

nikomatsakis (Dec 20 2018 at 15:12, on Zulip):

lol

oli (Dec 20 2018 at 15:12, on Zulip):

heh

pnkfelix (Dec 20 2018 at 15:12, on Zulip):

this fixes "cannot mutate statics in initalizer of another static" #56903

pnkfelix (Dec 20 2018 at 15:13, on Zulip):

(where the actual issue is a &mut borrow, not an actual mutation from the other initializer)

pnkfelix (Dec 20 2018 at 15:14, on Zulip):

anyway, this seems fine to me?

nikomatsakis (Dec 20 2018 at 15:14, on Zulip):

:back:

pnkfelix (Dec 20 2018 at 15:14, on Zulip):

okay tagging as beta-accepted.

pnkfelix (Dec 20 2018 at 15:15, on Zulip):

(i'll try to start things off with the emoji vote on my own comment in the future to give us a single point-of-click)

pnkfelix (Dec 20 2018 at 15:15, on Zulip):

next: "fix trait objects with a Self-containing projection values" #56863

pnkfelix (Dec 20 2018 at 15:17, on Zulip):

I don't have an opinion here

pnkfelix (Dec 20 2018 at 15:17, on Zulip):

but I see @nikomatsakis has already weighed in

pnkfelix (Dec 20 2018 at 15:17, on Zulip):

okay beta-accepting

pnkfelix (Dec 20 2018 at 15:18, on Zulip):

next: "Fixed issue with using Self ctor in typedefs" #56850

pnkfelix (Dec 20 2018 at 15:18, on Zulip):

which I recently learned fixes yet another bug, let me add that to the description

pnkfelix (Dec 20 2018 at 15:19, on Zulip):

so if I understand correctly

pnkfelix (Dec 20 2018 at 15:19, on Zulip):

the a feature that this is fixing a bug in

pnkfelix (Dec 20 2018 at 15:19, on Zulip):

is allowed on the beta channel

pnkfelix (Dec 20 2018 at 15:20, on Zulip):

(but not the stable channel)

nagisa (Dec 20 2018 at 15:20, on Zulip):

This seems to fix quite many different things

nagisa (Dec 20 2018 at 15:20, on Zulip):

some stable-to-stable regressions too

nagisa (Dec 20 2018 at 15:20, on Zulip):

but I’m not inclined to accept this unless this is fixing something in beta

pnkfelix (Dec 20 2018 at 15:20, on Zulip):

right, but I think the stable-to-stable is an ICE

pnkfelix (Dec 20 2018 at 15:20, on Zulip):

that comes after you get a warning about

pnkfelix (Dec 20 2018 at 15:21, on Zulip):

the feature being unsupported

Esteban Küber (Dec 20 2018 at 15:21, on Zulip):

I believe that is the case

pnkfelix (Dec 20 2018 at 15:21, on Zulip):

Thats why I was mentinoing that the feature in question is stabilized in beta

pnkfelix (Dec 20 2018 at 15:21, on Zulip):

(and thus this does seem worth considering for beta at least. but probably not worth stable-nominating, if I understand correctly. Not that anyone was suggesting that.)

pnkfelix (Dec 20 2018 at 15:21, on Zulip):

but

pnkfelix (Dec 20 2018 at 15:22, on Zulip):

its "only" fixing ICE's

pnkfelix (Dec 20 2018 at 15:22, on Zulip):

if we produce reasonable errors before the ICE in each csae

pnkfelix (Dec 20 2018 at 15:22, on Zulip):

then I might be disinclined to backport

nikomatsakis (Dec 20 2018 at 15:22, on Zulip):

it seems pretty harmless

pnkfelix (Dec 20 2018 at 15:22, on Zulip):

I dont think the diagnostic here is reasonable #56199

pnkfelix (Dec 20 2018 at 15:22, on Zulip):

So there goes my argument against backporting. :smile:

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

though there are a lot of stray edits, I think the core change, if I understood,

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

is to issue an error in one particular case

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

here

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

(@Alexander Regueiro can likely confirm)

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

I would imagine that this fixes an ice due to delay_span_bug later?

nikomatsakis (Dec 20 2018 at 15:23, on Zulip):

(yep)

pnkfelix (Dec 20 2018 at 15:24, on Zulip):

should we consider requesting a refinement of the PR that has smaller scope?

pnkfelix (Dec 20 2018 at 15:24, on Zulip):

to be fair, constructing such a refinement has its own risks

nikomatsakis (Dec 20 2018 at 15:24, on Zulip):

I am in favor of backporting. I am neutral on whether to do a refined PR

nikomatsakis (Dec 20 2018 at 15:24, on Zulip):

exactly

pnkfelix (Dec 20 2018 at 15:25, on Zulip):

Okay. I know @Alexander Regueiro said they have little experience with backports

pnkfelix (Dec 20 2018 at 15:25, on Zulip):

maybe a reasonable answer is to approve this one

Alexander Regueiro (Dec 20 2018 at 15:25, on Zulip):

yeah, a little.

pnkfelix (Dec 20 2018 at 15:25, on Zulip):

but note that in the future, more targetted fixes would be appreciated?

Alexander Regueiro (Dec 20 2018 at 15:26, on Zulip):

yeah, I should have labelled which issues it fixed. I meant to in fact. must have slipped my mind.

pnkfelix (Dec 20 2018 at 15:27, on Zulip):

@Alexander Regueiro to be honest one biggest issue I think I have is the reordering and addition of the AdtFlags values. I don't know the PR well enough to know whether that had to be part of the fix to the ICE's here.

nikomatsakis (Dec 20 2018 at 15:27, on Zulip):

it looks like there was a new flag added,

pnkfelix (Dec 20 2018 at 15:27, on Zulip):

anyway, approved for backport.

nikomatsakis (Dec 20 2018 at 15:27, on Zulip):

IS_STRUCT or something

nikomatsakis (Dec 20 2018 at 15:27, on Zulip):

so just removing those diffs wholesale would be bad

Alexander Regueiro (Dec 20 2018 at 15:27, on Zulip):

technically not, but petrochenkov approved it as part of my other PR.

Alexander Regueiro (Dec 20 2018 at 15:27, on Zulip):

he thought it made sense, as I did

Alexander Regueiro (Dec 20 2018 at 15:28, on Zulip):

it shouldn't hurt, though I know what you mean.

pnkfelix (Dec 20 2018 at 15:28, on Zulip):

I totally agree that its an improvement on the state of the code w.r.t. nightly

pnkfelix (Dec 20 2018 at 15:28, on Zulip):

just wondering whether it could have been factored out into a follow-on PR. anyway, not worth spending more time on now.

Alexander Regueiro (Dec 20 2018 at 15:29, on Zulip):

mhm

pnkfelix (Dec 20 2018 at 15:29, on Zulip):

last one: "Make RValue::Discriminant a normal Shallow read" #56790

pnkfelix (Dec 20 2018 at 15:29, on Zulip):

so this is interesting

pnkfelix (Dec 20 2018 at 15:30, on Zulip):

this is a soundness fix

pnkfelix (Dec 20 2018 at 15:30, on Zulip):

but its only an NLL-fixed-by-NLL fix

pnkfelix (Dec 20 2018 at 15:30, on Zulip):

AST-borrowck will still accept the unsound code

nikomatsakis (Dec 20 2018 at 15:30, on Zulip):

incidentally, for my trait alias PR, I'm converting the future compat warning into a plain old lint... does anyone have suggestions on how to do that? is there a guide somewhere? (maybe we can take this conversation elsewhere.)

I'll respond in a separte topic

pnkfelix (Dec 20 2018 at 15:30, on Zulip):

My first inclination is to say "let it ride the trains"

pnkfelix (Dec 20 2018 at 15:31, on Zulip):

but since its coupled with NLL-migration mode

pnkfelix (Dec 20 2018 at 15:31, on Zulip):

(since its NLL-fixed-by-NLL)

pnkfelix (Dec 20 2018 at 15:31, on Zulip):

that leads me to at least be willing to consider backport.

nikomatsakis (Dec 20 2018 at 15:31, on Zulip):

I'm a bit confused

pnkfelix (Dec 20 2018 at 15:31, on Zulip):

(does anyone need me to explain the NLL-fixed-by-NLL label?)

nikomatsakis (Dec 20 2018 at 15:31, on Zulip):

I think what you are saying is:

nikomatsakis (Dec 20 2018 at 15:31, on Zulip):
nikomatsakis (Dec 20 2018 at 15:32, on Zulip):
nikomatsakis (Dec 20 2018 at 15:32, on Zulip):
pnkfelix (Dec 20 2018 at 15:32, on Zulip):

(does anyone need me to explain the NLL-fixed-by-NLL label?)

NLL-fixed-by-NLL denotes bugs (which can be soundness issues, or expresiveness ones, or even diagnostic ones) where the bug fix is only available if you opt into NLL

pnkfelix (Dec 20 2018 at 15:32, on Zulip):

in most cases NLL-fixed-by-NLL fixes are available on the 2018 edition (via the NLL migration mode that we deployed there)

pnkfelix (Dec 20 2018 at 15:33, on Zulip):

this was beta-nominated by @Matthew Jasper , a member of the NLL team

pnkfelix (Dec 20 2018 at 15:33, on Zulip):

does anyone here want to advocate for a backport?

nikomatsakis (Dec 20 2018 at 15:34, on Zulip):

it seems like a backport is not necessary

pnkfelix (Dec 20 2018 at 15:34, on Zulip):

but on the other hand

pnkfelix (Dec 20 2018 at 15:34, on Zulip):

the risk associated with backport is minor

pnkfelix (Dec 20 2018 at 15:34, on Zulip):

and it will get us more feedback more quickly as to the impact of this change

pnkfelix (Dec 20 2018 at 15:35, on Zulip):

(I think that last comment from me is the main reason I can think of to backport)

nikomatsakis (Dec 20 2018 at 15:36, on Zulip):

and it will get us more feedback more quickly as to the impact of this change

I mean, we're making the change regardless, right? it's a clear soundness violation

nikomatsakis (Dec 20 2018 at 15:36, on Zulip):

anyway I'm feeling neutral about it

pnkfelix (Dec 20 2018 at 15:36, on Zulip):

well

pnkfelix (Dec 20 2018 at 15:36, on Zulip):

one could imagine having ways to opt out of inline discriminants

pnkfelix (Dec 20 2018 at 15:36, on Zulip):

on a e.g. per-enum basis

pnkfelix (Dec 20 2018 at 15:37, on Zulip):

and then allow the borrows that are being rejected by this PR in that case

pnkfelix (Dec 20 2018 at 15:37, on Zulip):

but that would be an RFC

nikomatsakis (Dec 20 2018 at 15:37, on Zulip):

correct

pnkfelix (Dec 20 2018 at 15:37, on Zulip):

at the very least

pnkfelix (Dec 20 2018 at 15:37, on Zulip):

@nikomatsakis says they are neutral. I'm personally back to "let it ride the trains."

nikomatsakis (Dec 20 2018 at 15:38, on Zulip):

ps, I like :back: and :train: as our two emojis :)

pnkfelix (Dec 20 2018 at 15:38, on Zulip):

amazojis!

nikomatsakis (Dec 20 2018 at 15:39, on Zulip):

I cry because that "word" hurts me so, in case it was unclear

pnkfelix (Dec 20 2018 at 15:41, on Zulip):

okay, next up

pnkfelix (Dec 20 2018 at 15:41, on Zulip):

nominated issues

pnkfelix (Dec 20 2018 at 15:41, on Zulip):

first: "rustdoc crashing on linux-sparc64 due to unaligned access" #56927

pnkfelix (Dec 20 2018 at 15:42, on Zulip):

sadly i haven't had access to a sparc workstation nearly a decade

nagisa (Dec 20 2018 at 15:42, on Zulip):

I have requested an account on the GCC’s build farm to look into this issue in addition to other stuff

nagisa (Dec 20 2018 at 15:42, on Zulip):

we do have a bisected PR that is fairly small

nikomatsakis (Dec 20 2018 at 15:43, on Zulip):

amazing that it has been bisected to such a small PR

nagisa (Dec 20 2018 at 15:43, on Zulip):

it could be a question of just a "revert-for-now"

nikomatsakis (Dec 20 2018 at 15:43, on Zulip):

(is @eddyb around?)

pnkfelix (Dec 20 2018 at 15:43, on Zulip):

@nagisa did you nominate mostly because you wanted broader discussion of what P-label to apply? or ... oh, I see, you think we may want to try a blind fix?

nagisa (Dec 20 2018 at 15:43, on Zulip):

I nominated this because this is technically a regression

mw (Dec 20 2018 at 15:43, on Zulip):

sadly i haven't had access to a sparc workstation nearly a decade

See https://cfarm.tetaneutral.net/ if you want access.

pnkfelix (Dec 20 2018 at 15:44, on Zulip):

Okay. (And admittedly, I don't always get to the stable-to-stable regressions. Especially if they aren't tagged with T-compiler)

nagisa (Dec 20 2018 at 15:44, on Zulip):

I think the ideal path forward would involve getting together with @eddyb and thinking about how that PR could’ve broken things

nikomatsakis (Dec 20 2018 at 15:44, on Zulip):

I'm in favor of fixing it, seems a shame to have tracked it to a specific PR and then not fix it

pnkfelix (Dec 20 2018 at 15:44, on Zulip):

still, what priority?

nikomatsakis (Dec 20 2018 at 15:44, on Zulip):

tl;dr I feel like P-high is good so we don't lose track

nikomatsakis (Dec 20 2018 at 15:44, on Zulip):

but I'd prefer to have @eddyb weigh in on their preferred fix

pnkfelix (Dec 20 2018 at 15:45, on Zulip):

Okay. I was thinking P-high, not becasue sparc is high tier platform, but rather because this may be symptom of deeper problem.

pnkfelix (Dec 20 2018 at 15:45, on Zulip):

who wants to own it?

nagisa (Dec 20 2018 at 15:45, on Zulip):

@pnkfelix oh also another reason I nominated this: sparc is not the only architecture with very strict alignment requirements

nagisa (Dec 20 2018 at 15:45, on Zulip):

I fear that it could be potential UB elsewhere as well

nagisa (Dec 20 2018 at 15:46, on Zulip):

can assign me, was planning to work on sparc for a little while anyway

pnkfelix (Dec 20 2018 at 15:46, on Zulip):

okay great

nagisa (Dec 20 2018 at 15:46, on Zulip):

(as soon and if I get access to sparc machines in the gcc buildfarm)

pnkfelix (Dec 20 2018 at 15:46, on Zulip):

next: "PhantomData fields in repr(C) structs change ABI on aarch64" #56877

pnkfelix (Dec 20 2018 at 15:47, on Zulip):

I'm assuming this is P-high based on @nikomatsakis 's input

nikomatsakis (Dec 20 2018 at 15:47, on Zulip):

I nominated this because it was highlighted to me by various gecko folks as something likely to become high priority for them

pnkfelix (Dec 20 2018 at 15:47, on Zulip):

is this a regression?

nikomatsakis (Dec 20 2018 at 15:47, on Zulip):

seems like maybe we're close to a fix in any case

nikomatsakis (Dec 20 2018 at 15:48, on Zulip):

I don't know that it's a regression

nikomatsakis (Dec 20 2018 at 15:48, on Zulip):

I would guess more of a "never worked"

pnkfelix (Dec 20 2018 at 15:48, on Zulip):

okay just curious

nikomatsakis (Dec 20 2018 at 15:48, on Zulip):

I think it's causing crashes on some attempts to port code over to that architecture

pnkfelix (Dec 20 2018 at 15:48, on Zulip):

I'm willing to take point on it, in any case

nikomatsakis (Dec 20 2018 at 15:49, on Zulip):

aarch64 is fairly common these days as a target, seems good to have solid support — I guess it's also true that the behavior is wrong, though that's not entirely clear. but it'd be pretty unfortunate for phantomdata to affect ABIs.

pnkfelix (Dec 20 2018 at 15:49, on Zulip):

I'll tag as P-high and assign to self.

pnkfelix (Dec 20 2018 at 15:50, on Zulip):

last nominated issue: "Why is jemalloc linked to rustc by default only via CI?" #56812

pnkfelix (Dec 20 2018 at 15:50, on Zulip):

I filed this last week

pnkfelix (Dec 20 2018 at 15:51, on Zulip):

Basically the default when you build rustc locally on Linux/MacOSX is to use your OS's allocator. If you want jemalloc, you need to opt-into it.

pnkfelix (Dec 20 2018 at 15:52, on Zulip):

Which definitely has various benefits (i.e. someone who uses tools that rely on their OS's allocator being utilized will immediatley benefit, when they are debugging their local build)

pnkfelix (Dec 20 2018 at 15:52, on Zulip):

but the CI-generated build products (which include IIUC all the nightly/beta/stable channel stuff) link in jemalloc on Linux+MacOSX

pnkfelix (Dec 20 2018 at 15:53, on Zulip):

which means you get into situations where some bugs only arise when someone uses the CI product and a local build won't immediately repro

nikomatsakis (Dec 20 2018 at 15:53, on Zulip):

Are there other ways that our default configuration differ from CI?

nagisa (Dec 20 2018 at 15:53, on Zulip):

Yes.

nikomatsakis (Dec 20 2018 at 15:53, on Zulip):

I sort of expect the "defaults" to be CI, but I also think the main thing is that it should be very easy to get a CI build

nagisa (Dec 20 2018 at 15:54, on Zulip):

I have found it very non-trivial to make a ./x.py dist work by default

pnkfelix (Dec 20 2018 at 15:54, on Zulip):

The basic argument in favor of the status quo is that the CI wants the produce the most performant rustc (which means linking in jemalloc) and, as @nikomatsakis and @nagisa note, if you want to replicate a "true CI" build, you need to use a docker image

nagisa (Dec 20 2018 at 15:54, on Zulip):

I had to change quite a few things in configuration to make dist work at all

nikomatsakis (Dec 20 2018 at 15:54, on Zulip):

personally my "standard" config.toml always turned off jemalloc

nikomatsakis (Dec 20 2018 at 15:54, on Zulip):

I wonder if we should provide various config.toml "templates"

varkor (Dec 20 2018 at 15:54, on Zulip):

it sounds like this would be a good topic to expand upon in the rustc guide

pnkfelix (Dec 20 2018 at 15:54, on Zulip):

My personal inclination is that I don't really want to fight with T-infra nor @Alex Crichton about this.

varkor (Dec 20 2018 at 15:55, on Zulip):

and make sure that it's very clear exactly what the differences are

nikomatsakis (Dec 20 2018 at 15:55, on Zulip):

there are already some notes in rustc-guide

nagisa (Dec 20 2018 at 15:55, on Zulip):

I’d like things to at least work at all by default

nikomatsakis (Dec 20 2018 at 15:55, on Zulip):

but not about CI comparison

pnkfelix (Dec 20 2018 at 15:55, on Zulip):

and that I'd probably be satisified with doc improvements

pnkfelix (Dec 20 2018 at 15:55, on Zulip):

in both rustc-guide and the config.toml.example

nagisa (Dec 20 2018 at 15:55, on Zulip):

I don’t mind so much about differing defaults, but I’ll likely very much mind when it makes me waste hours trying to reproduce something and fail due to mismatch there.

pnkfelix (Dec 20 2018 at 15:55, on Zulip):

(I think its important the CI differences are highlighted in both locations)

pnkfelix (Dec 20 2018 at 15:57, on Zulip):

Would anyone like to take point here on trying to 1. reproducing the process of exactly replicating a CI build (via docker), 2. surveying the differences between CI build env and the defaults of config.toml.example, and 3. documenting the things from 2?

pnkfelix (Dec 20 2018 at 15:57, on Zulip):

if not, maybe I'll just tag this as an E-mentor bug

nikomatsakis (Dec 20 2018 at 15:58, on Zulip):

seems like something that a T-infra person may be better equipped to do

pnkfelix (Dec 20 2018 at 15:58, on Zulip):

true

pnkfelix (Dec 20 2018 at 15:59, on Zulip):

okay maybe I'll tag this with T-infra, renominate for discussion amongst their team

pnkfelix (Dec 20 2018 at 15:59, on Zulip):

and transcribe the three steps I put above as a request for how to resolve this to my satisfaction?

pnkfelix (Dec 20 2018 at 16:00, on Zulip):

Well I'll just do that

pnkfelix (Dec 20 2018 at 16:00, on Zulip):

that's all the beta-noms and I-noms! and that's an hour

pnkfelix (Dec 20 2018 at 16:00, on Zulip):

we're done (on time!)

nikomatsakis (Dec 20 2018 at 16:00, on Zulip):

(Seems like https://github.com/rust-lang/rust/issues/56899 is already fixed; at least I cannot reproduce with the latest beta, and I can with older betas)

nikomatsakis (Dec 20 2018 at 16:00, on Zulip):

Shall I just close?

pnkfelix (Dec 20 2018 at 16:00, on Zulip):

Oh, and we should cancel next weeks' meeting, right?

nikomatsakis (Dec 20 2018 at 16:00, on Zulip):

Yes, definitely :)

nagisa (Dec 20 2018 at 16:01, on Zulip):

Next Thuesday is 27th and the one after that is 3rd

nikomatsakis (Dec 20 2018 at 16:01, on Zulip):

I'll be around on the 3rd but prob not the 27th

nagisa (Dec 20 2018 at 16:01, on Zulip):

So technically neither falls onto a holiday, but probably many of us will be vacationing anyway, given how there’re only ~3 work days between all the holidays and weekends.

nikomatsakis (Dec 20 2018 at 16:02, on Zulip):

(I just started a topic called "holiday hours" if people want to note their availability)

pnkfelix (Dec 20 2018 at 16:02, on Zulip):

I think we can keep the meeting that is scheduled for the 3rd

pnkfelix (Dec 20 2018 at 16:03, on Zulip):

or rather

nikomatsakis (Dec 20 2018 at 16:03, on Zulip):

shall I delete the event on the 27th tho?

pnkfelix (Dec 20 2018 at 16:03, on Zulip):

I'll just thumbs up nikos' note

nikomatsakis (Dec 20 2018 at 16:03, on Zulip):

k

pnkfelix (Dec 20 2018 at 16:03, on Zulip):

but yeah I think deleting 27th event is good

pnkfelix (Dec 20 2018 at 16:04, on Zulip):

and then maybe I'll just try to make sure we don't make any mission critical decisions on the 3rd.

pnkfelix (Dec 20 2018 at 16:04, on Zulip):

(given that I expect various people to be absent that day)

nagisa (Dec 20 2018 at 16:06, on Zulip):

Just got my compile farm account request approved

nagisa (Dec 20 2018 at 16:06, on Zulip):

so that is sort-of a confirmation that I’ll be looking at least to some extent into that sparc issue.

Last update: Nov 16 2019 at 02:10UTC