Stream: t-compiler

Topic: weekly meeting 2018-09-20


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

hi @T-compiler just noting we'll be starting our weekly meeting here in 3 minutes or so

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

I'll go ahead and start writing stuff on the assumption that people can catch up. (Yay backlog system)

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

First up, P-high T-compiler: http://is.gd/0ohWmp

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

(for those unaware, I am following the agenda from an (old) etherpad)

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

first P-high issue: "Trait with HRTB blanket impl is implemented when it shouldn't be" #54302

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

its been bisected, yay

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

blame has been placed on a supposed refactoring

nikomatsakis (Sep 20 2018 at 14:03, on Zulip):

I was hoping to look into tthat this morning

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

PR: "Replace the global fulfillment cache with the evaluation cache" #42840

nikomatsakis (Sep 20 2018 at 14:03, on Zulip):

but got distracted

nikomatsakis (Sep 20 2018 at 14:03, on Zulip):

you can assign to me

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

okay, assigning to niko

pnkfelix (Sep 20 2018 at 14:04, on Zulip):

okay next up: "ICE compiling the objrs crate" #54059

pnkfelix (Sep 20 2018 at 14:04, on Zulip):

assign to mw but ariel's looked at it

pnkfelix (Sep 20 2018 at 14:04, on Zulip):

PR posted: "avoid leaking host details in proc macro metadata decoding" #54265

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

so it seems like its under control

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

(We'll be coming back around to that PR when we do the beta-nominations, but for now lets move along)

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

Next up: "Implement some way to run UI tests ignoring run-pass tests" #54047

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

boy I keep trying and it keeps failing

nikomatsakis (Sep 20 2018 at 14:07, on Zulip):

anything in particular?

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

okay well anyway this is assigned ot me

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

I think I need to figure out how to test e.g. the asmjs target locally

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

rather than just responding to bors and then hoping

nikomatsakis (Sep 20 2018 at 14:08, on Zulip):

oh dear

davidtwco (Sep 20 2018 at 14:08, on Zulip):

I had the same thing with the compile-fail migration, it is painful.

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

Or ... do try builds build for those targets and run the test suite?

varkor (Sep 20 2018 at 14:08, on Zulip):

try builds don't run the test suite afaiu

nikomatsakis (Sep 20 2018 at 14:08, on Zulip):

I'm not sure if try builds run on all platforms? @Pietro Albini would prob know

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

Okay if try builds don't run the tests then that's pointless

nikomatsakis (Sep 20 2018 at 14:09, on Zulip):

I think they do, but it doesn't cause them to "fail"? not sure

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

okay well in any case I'm looiking into it

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

there's certainly another option

nagisa (Sep 20 2018 at 14:09, on Zulip):

there was another PR that involves running back to bors and checking if stuff works

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

of eagerly moving the src/test/ui/run-pass/ back to src/test/run-pass right now

davidtwco (Sep 20 2018 at 14:09, on Zulip):

I was recommended changing the .travis.yml to enable other platforms temporarily for the PR testing instead of queuing for bors each time for the other platforms.

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

hmm

nagisa (Sep 20 2018 at 14:10, on Zulip):

perhaps we could summarize the steps necessary to minimize the interference with everything else while this is being done somewhere (forge?)

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

@davidtwco maybe I will follow up with you after the meeting for more info on that avenue ?

davidtwco (Sep 20 2018 at 14:10, on Zulip):

I never did it, but I'll link you to the comment where I told that.

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

Okay, thanks that would be helpful

nikomatsakis (Sep 20 2018 at 14:10, on Zulip):

perhaps we could summarize the steps necessary to minimize the interference with everything else while this is being done somewhere (forge?)

rustc-guide plz

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

And I think I agree with @nagisa that it seems like there is general knowledge here that should be documented.

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

so I'll open an issue for that

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

the only Q I have is

nagisa (Sep 20 2018 at 14:11, on Zulip):

@pnkfelix current PR that is doing so is https://github.com/rust-lang/rust/pull/54004

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

do we think that enough people are sufficiently inconvienced right now

nagisa (Sep 20 2018 at 14:11, on Zulip):

(I recommended them to change travis.yml as well, but did not know any detains on what they should do)

davidtwco (Sep 20 2018 at 14:11, on Zulip):

(@pnkfelix that comment about travis.yml)

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

that I shoud indeed undo the original src/test/run-pass ==> src/test/ui/run-pass work

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

That was my only Question

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

anyone have any thoughts? I know @Vadim Petrochenkov probably would be happiest if I did that

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

so I'll treat that as one vote in favor

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

Does anyone else care one way or another in the short term?

nagisa (Sep 20 2018 at 14:12, on Zulip):

I don’t mind either way, but I want to see our tests migrated to a tag system we spoke about last week ASAP.

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

That's ... a broader design question

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

I think we don't want to block e.g. testing of NLL against run-pass suite on that ("that" being the broader reworking of our test suite + compiletest to be property based)

nikomatsakis (Sep 20 2018 at 14:14, on Zulip):

so the current status is that run-pass + UI have been largely merged

nikomatsakis (Sep 20 2018 at 14:14, on Zulip):

and you are currently working on moving everything back to run-pass directory

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

no

nikomatsakis (Sep 20 2018 at 14:14, on Zulip):

but making run-pass use ui mode

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

oh

nikomatsakis (Sep 20 2018 at 14:14, on Zulip):

/me confused

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

the current status

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

is that some portion of run-pass was moved to ui

nikomatsakis (Sep 20 2018 at 14:14, on Zulip):

right, ok, I spoke imprecisely, but that's what I meant

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

the remaining task was to get the remainder of run-pass testing against NLL

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

so it seems to me that what you are proposing is to split your PR into two halves:

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

the PR I have up, it only turns on ui/ mode on the existing run-pass/ directory

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

and it doesn't shuffle anything around

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

we didn't really find any NLL bugs in the move...

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

I think that is true (that the move has not uncovered NLL bugs)

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

oh, ok

nikomatsakis (Sep 20 2018 at 14:16, on Zulip):

I don't care, I think it's fine if you want to move the files back. Obviously we lose some test coverage but it's prob ok

nikomatsakis (Sep 20 2018 at 14:16, on Zulip):

(until we switch run-pass to ui)

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

The main motivation I have to not move the files back

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

is that right now people who are maintaining those files

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

are doing it in the ui directory

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

which means my change to the run-pass/ suite does have a prayer of landing

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

if I move the files back

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

it will increase the effort required to change the semantics of run-pass/ to a ui-mode

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

in terms of rebasing

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

Here's an idea

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

maybe I set a time limit

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

i.e. if I can land PR #54223

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

by, what, next T-compiler meeting

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

in one week

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

then that's gravy

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

(and then we can readily move src/test/ui/run-pass back, trivially)

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

If the PR hasn't landed by that time, for whatever reason

nikomatsakis (Sep 20 2018 at 14:18, on Zulip):

ok

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

then we acquiesce to the needs to people who are inconvenienced

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

I'm happy with that plan, anyone object?

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

Okay well if someone objects they can comment on the ticket. (I'll make a note there of this plan)

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

next up: "Regression in #[allow(deprecated)] for impl Trait return type" #54045

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

hmm, are either @eddyb or @Oli here right now?

eddyb (Sep 20 2018 at 14:20, on Zulip):

hi

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

Hi: Is #54045 on your current radar?

eddyb (Sep 20 2018 at 14:21, on Zulip):

@Oli has been away, and will return in October

eddyb (Sep 20 2018 at 14:21, on Zulip):

and, no, I was stuck on edition things :(

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

understandable

nikomatsakis (Sep 20 2018 at 14:21, on Zulip):

ah right

nikomatsakis (Sep 20 2018 at 14:21, on Zulip):

I had forgotten about this refactoring

eddyb (Sep 20 2018 at 14:21, on Zulip):

sorry for not saying something earlier - I forgot it was assigned to me

nikomatsakis (Sep 20 2018 at 14:21, on Zulip):

@eddyb do you think you'll have time to do that + edition stuff?

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

(well maybe its my fault for assigning both of you)

eddyb (Sep 20 2018 at 14:22, on Zulip):

does it need a beta backport or something? I can try but I'm worried I don't know what @Oli did exactly

eddyb (Sep 20 2018 at 14:22, on Zulip):

(that is, I can do it while waiting for a crater run, probably)

pnkfelix (Sep 20 2018 at 14:22, on Zulip):

I guess its now a stable-to-stable regression

eddyb (Sep 20 2018 at 14:22, on Zulip):

:(

pnkfelix (Sep 20 2018 at 14:22, on Zulip):

which means... would we bother with a beta backport and not attempt a stable one?

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

My experience has been that we either backport all the way in such cases, or not at all

eddyb (Sep 20 2018 at 14:23, on Zulip):

it is it high-impact? I'm confused as to why this even got high priority

eddyb (Sep 20 2018 at 14:23, on Zulip):

surely you can put the #[allow(deprecated)] higher up or something?

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

I don't know, the bug isn't clear about that

nikomatsakis (Sep 20 2018 at 14:23, on Zulip):

it does seem a bit like "just a bug"

nikomatsakis (Sep 20 2018 at 14:24, on Zulip):

cc @Taylor Cramer, you around?

eddyb (Sep 20 2018 at 14:24, on Zulip):

oh we already were too late when anyone was assigned https://github.com/rust-lang/rust/issues/54045#issuecomment-419836436

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

this one works: https://play.rust-lang.org/?gist=f16fb745294da41128a2b3db93811d40&version=stable&mode=debug&edition=2015

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

which means to me that @eddyb is right: You can work around this

eddyb (Sep 20 2018 at 14:25, on Zulip):

yeah, it's not unrooted, it's just rooted to the parent module

eddyb (Sep 20 2018 at 14:25, on Zulip):

which is something I tried to steer @Oli from, but apparently some things were easier for them this way

eddyb (Sep 20 2018 at 14:25, on Zulip):

our HIR handling is a mess :(

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

Okay, so here's what I suggest

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

We note that the workaround exists in the bug

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

and we don't plan to backport

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

we'll fix it when we fix it

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

it can stay P-high

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

but maybe even that's not so important

eddyb (Sep 20 2018 at 14:26, on Zulip):

I'd really prefer to wait for @Oli to get back and not step on their toes

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

(we can revisit that choice later)

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

I'm trying to say we can justify waiting

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

rather than putting @eddyb on it immediately

eddyb (Sep 20 2018 at 14:27, on Zulip):

yeah, I hope @Taylor Cramer agrees

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

because the workaround exists

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

shall I continue to leave it assigned to both of you, or just assign it to @Oli ?

eddyb (Sep 20 2018 at 14:27, on Zulip):

both is fine, since we'll want to discuss it

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

okay

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

I'll write a note in the issue after the meeting

eddyb (Sep 20 2018 at 14:27, on Zulip):

and come up with some design that interacts well with other things as well

pnkfelix (Sep 20 2018 at 14:28, on Zulip):

(should I remove the stable-nominated tag that @simulacrum added as well?)

eddyb (Sep 20 2018 at 14:28, on Zulip):

I guess

pnkfelix (Sep 20 2018 at 14:28, on Zulip):

((I guess we can keep it on there until @Oli gets back))

pnkfelix (Sep 20 2018 at 14:28, on Zulip):

or at least until next week. :smile:

pnkfelix (Sep 20 2018 at 14:28, on Zulip):

lets move on

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

next P-high: "ICE in libserialize with incremental build across changed rustc version" #53792

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

seems like there is ongoing discussion of the problem and potential solutions there

nagisa (Sep 20 2018 at 14:30, on Zulip):

Does not appear to be P-high to me.

eddyb (Sep 20 2018 at 14:30, on Zulip):

yeah it's a typically hard problem

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

I cannot tell whether it should continue to be tagged with T-compiler

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

or if we should delegate it entirely to T-infra

mw (Sep 20 2018 at 14:31, on Zulip):

the changes in the code of the compiler itself are trivial

mw (Sep 20 2018 at 14:31, on Zulip):

so could be delegated

eddyb (Sep 20 2018 at 14:31, on Zulip):

I wonder if it's just like those distros builds

pnkfelix (Sep 20 2018 at 14:31, on Zulip):

@nagisa you figure that since @Taylor Cramer has an applicable workaround, we can downgrade to P-medium ?

eddyb (Sep 20 2018 at 14:31, on Zulip):

that are missing git commit info

pnkfelix (Sep 20 2018 at 14:31, on Zulip):

what do people think about that, a downgrade to P-medium here?

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

I figure if we delegate entirely to T-infra, then we could let them decide to downgrade...

nikomatsakis (Sep 20 2018 at 14:32, on Zulip):

seems ok; it does seem like incorporating the git commit into the hash or something would suffice

mw (Sep 20 2018 at 14:32, on Zulip):

if @Taylor Cramer doesn't object

eddyb (Sep 20 2018 at 14:32, on Zulip):

@nikomatsakis AFAIK we already do but some ways of building Rust loses that info

nikomatsakis (Sep 20 2018 at 14:32, on Zulip):

ok

eddyb (Sep 20 2018 at 14:32, on Zulip):

e.g. our tarballs have historically been very deficient here :(

eddyb (Sep 20 2018 at 14:32, on Zulip):

(which an entirely infra/release issue)

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

okay next: "ICE when running cargo doc on typenum at librustc/traits/structural_impls.rs:178" #52873

nikomatsakis (Sep 20 2018 at 14:35, on Zulip):

I fixed that...

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

with PR #54199 ?

nikomatsakis (Sep 20 2018 at 14:35, on Zulip):

oh

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

that is marked [WIP]

nikomatsakis (Sep 20 2018 at 14:35, on Zulip):

I forgot that it hadn't mergd :P

nikomatsakis (Sep 20 2018 at 14:35, on Zulip):

yeah, my bad

nikomatsakis (Sep 20 2018 at 14:36, on Zulip):

I was trying to find an independent test case and failing

nikomatsakis (Sep 20 2018 at 14:36, on Zulip):

and I totally forgot about it

pnkfelix (Sep 20 2018 at 14:36, on Zulip):

okay well the issue sounds like its in good, if very-very overworked, hands

nikomatsakis (Sep 20 2018 at 14:36, on Zulip):

I'll try a bit harder

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

next up: "infinite recursion ICE" #52701

nikomatsakis (Sep 20 2018 at 14:37, on Zulip):

uh, I forgot about this too

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

want me to take it?

pnkfelix (Sep 20 2018 at 14:38, on Zulip):

I noted that I don't think I have too much stuff from T-compiler on my plate at the moment

nikomatsakis (Sep 20 2018 at 14:38, on Zulip):

hmm let me add it to my to-do list (I think that was my fail), but I may ping you

nikomatsakis (Sep 20 2018 at 14:38, on Zulip):

since it's late in your day

pnkfelix (Sep 20 2018 at 14:38, on Zulip):

okay. left a note accordingly

pnkfelix (Sep 20 2018 at 14:38, on Zulip):

that's all the P-highs

pnkfelix (Sep 20 2018 at 14:39, on Zulip):

next up, stable-to-beta regressions

pnkfelix (Sep 20 2018 at 14:39, on Zulip):

first is "Regression in crate ipfs-api on 1.30 beta" #54386

pnkfelix (Sep 20 2018 at 14:39, on Zulip):

seems P-high to me

eddyb (Sep 20 2018 at 14:40, on Zulip):

do we assign @Vadim Petrochenkov ?

pnkfelix (Sep 20 2018 at 14:40, on Zulip):

I was just wondering that. I thought I might let @Vadim Petrochenkov respond to the ping they received on it, rather than pre-emptively assigning them

pnkfelix (Sep 20 2018 at 14:41, on Zulip):

But on the other hand, if it were me I'd probably want someone to assign it to me

nikomatsakis (Sep 20 2018 at 14:41, on Zulip):

related: leo is working now at a company using Rust, hope it's not blocking them :)

pnkfelix (Sep 20 2018 at 14:41, on Zulip):

I guess they are the obvious person; and they can unassign themselves if they want.

nikomatsakis (Sep 20 2018 at 14:42, on Zulip):

maybe we can bisect

nikomatsakis (Sep 20 2018 at 14:42, on Zulip):

also I think assign

pnkfelix (Sep 20 2018 at 14:43, on Zulip):

just did, okay

pnkfelix (Sep 20 2018 at 14:43, on Zulip):

and we already looked at #54059, which was the only other stable-to-beta regression

pnkfelix (Sep 20 2018 at 14:43, on Zulip):

next up, stable-to-nightly regressions

pnkfelix (Sep 20 2018 at 14:44, on Zulip):

! zarroh bugs !

eddyb (Sep 20 2018 at 14:44, on Zulip):

ipfs-api looks legitimate btw :(

eddyb (Sep 20 2018 at 14:44, on Zulip):

they have (many) cyclic glob imports (and things named serde are imported from glob-importing modules)

pnkfelix (Sep 20 2018 at 14:45, on Zulip):

Okay, then, next are the beta-nominations

pnkfelix (Sep 20 2018 at 14:45, on Zulip):

beta nominations

pnkfelix (Sep 20 2018 at 14:46, on Zulip):

the first #54323 does not look like its T-compiler's issue to approve. (Its not tagged.)

pnkfelix (Sep 20 2018 at 14:46, on Zulip):

T-infra maintains rustbuild, right?

pnkfelix (Sep 20 2018 at 14:46, on Zulip):

or is it us? Or is it @Alex Crichton on their own?

nikomatsakis (Sep 20 2018 at 14:46, on Zulip):

it's sort of unclear

nikomatsakis (Sep 20 2018 at 14:46, on Zulip):

but it's more infra I think

pnkfelix (Sep 20 2018 at 14:46, on Zulip):

gotta be T-infra, @eddyb cc'ed them. :)

nikomatsakis (Sep 20 2018 at 14:47, on Zulip):

and/or @simulacrum

pnkfelix (Sep 20 2018 at 14:47, on Zulip):

okay. I tagged T-infra. Hopefully it'll get addressed.

pnkfelix (Sep 20 2018 at 14:48, on Zulip):

Next one is us I think: "avoid leaking host details in proc macro metadata decoding" #54265

nikomatsakis (Sep 20 2018 at 14:48, on Zulip):

looks ok to me

nikomatsakis (Sep 20 2018 at 14:49, on Zulip):

based on a casual perusal

nikomatsakis (Sep 20 2018 at 14:49, on Zulip):

I'm not super familiar with the code, but it's a short diff, and @Alex Crichton seemed satisfied :)

eddyb (Sep 20 2018 at 14:49, on Zulip):

hopefully eventually we might be able to even hide the host completely (when we generate the proc macro metadata)

pnkfelix (Sep 20 2018 at 14:49, on Zulip):

okay, any objections to beta-accepting this?

pnkfelix (Sep 20 2018 at 14:50, on Zulip):

done

pnkfelix (Sep 20 2018 at 14:50, on Zulip):

next: waiting for our team

pnkfelix (Sep 20 2018 at 14:50, on Zulip):

zaaroh bugs

pnkfelix (Sep 20 2018 at 14:51, on Zulip):

next: I-nominated T-compiler (alone)

pnkfelix (Sep 20 2018 at 14:51, on Zulip):

ooh boy five bugs and 9 minutes to go

pnkfelix (Sep 20 2018 at 14:51, on Zulip):

first up: "What should we guarantee regarding "sort-of unused" extern statics" #54388

pnkfelix (Sep 20 2018 at 14:51, on Zulip):

this can be quick, maybe

pnkfelix (Sep 20 2018 at 14:51, on Zulip):

I just filed this an hour ago

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

if you do extern "C" { static C: u8; } and reference it from dead code

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

e.g. fn main() { let _x = C; }

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

(with an unsafe of course)

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

we currently will either succeed or fail to compile

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

depending on debug symbols

pnkfelix (Sep 20 2018 at 14:52, on Zulip):

and a pending PR causes it to sometimes fail even without debug symbols turned on

pnkfelix (Sep 20 2018 at 14:53, on Zulip):

(based we think on codegen-units issues)

nagisa (Sep 20 2018 at 14:53, on Zulip):

Seems fine to leave it implementation defined to me

nikomatsakis (Sep 20 2018 at 14:53, on Zulip):

well, we need to be able to test reliably

nikomatsakis (Sep 20 2018 at 14:53, on Zulip):

whatever we promise externally

pnkfelix (Sep 20 2018 at 14:54, on Zulip):

I just wanted to see if anyone would object to me saying "this code is bogus as written; you need to at leave provide the extra object code with the definition at link time, or you'll get UB (or compile-time error, maybe)"

nikomatsakis (Sep 20 2018 at 14:54, on Zulip):

I guess that the failure, @pnkfelix, is because LLVM optimizes away that dead code, and then we don't link?

nagisa (Sep 20 2018 at 14:54, on Zulip):

I assume it succeeds to compile when LLVM notices the static is not used and removes all references to it, which then passes through the linker

pnkfelix (Sep 20 2018 at 14:54, on Zulip):

yes, I believe these hypotheses are correct

pnkfelix (Sep 20 2018 at 14:54, on Zulip):

I haven't managed to validate them to completion

nagisa (Sep 20 2018 at 14:54, on Zulip):

It is trivial to make it not fail to link in either case.

eddyb (Sep 20 2018 at 14:54, on Zulip):

isn't this the same as extern { ... fn foo(); ... }?

nikomatsakis (Sep 20 2018 at 14:54, on Zulip):

presumably the same applies to functions

pnkfelix (Sep 20 2018 at 14:54, on Zulip):

Right, avoiding failure (by providing the definition) is trivial

nagisa (Sep 20 2018 at 14:54, on Zulip):

but in that case you’d get a ton of weird interactions with certain linker features which we do not want to have those weird interactions.

pnkfelix (Sep 20 2018 at 14:55, on Zulip):

@eddyb (maybe same as extern { fn foo(); }, not sure)

nagisa (Sep 20 2018 at 14:55, on Zulip):

@pnkfelix I meant making this kind of failure impossible from the standpoint of the compiler

nagisa (Sep 20 2018 at 14:55, on Zulip):

by e.g. giving a weak definition. But then that has interactions with weak linkage and linking order and all that stuff

pnkfelix (Sep 20 2018 at 14:56, on Zulip):

@nagisa you mean to actually consistently error in all cases? or consistently succeed in all cases?

nikomatsakis (Sep 20 2018 at 14:56, on Zulip):

so clearly failing to provide the symbol you declared can sometimes produce an error

nikomatsakis (Sep 20 2018 at 14:56, on Zulip):

is the question "is it ok to leave it ill-defined when we might choose not to"

pnkfelix (Sep 20 2018 at 14:56, on Zulip):

I don't know how to implement a guaranteed consistent semantics for this program

nagisa (Sep 20 2018 at 14:56, on Zulip):

making it consistently error in all cases is harder, at least I can’t think of a way to do so

pnkfelix (Sep 20 2018 at 14:56, on Zulip):

maybe we shouldn't try to decide this in five minutes

nikomatsakis (Sep 20 2018 at 14:56, on Zulip):

presumably we could geneerate some sort of "dummy references" (effectively what debuginfo is doing...) but I'm ok with current behavior I guess

nikomatsakis (Sep 20 2018 at 14:57, on Zulip):

I can imagine why CGU would be involved

pnkfelix (Sep 20 2018 at 14:57, on Zulip):

I'll leave the issue up for people to discuss there?

nagisa (Sep 20 2018 at 14:57, on Zulip):

okay..

pnkfelix (Sep 20 2018 at 14:57, on Zulip):

and maybe remove the I-nominated tag

pnkfelix (Sep 20 2018 at 14:57, on Zulip):

if we come to some conclusion on-ticket

pnkfelix (Sep 20 2018 at 14:57, on Zulip):

oaky well that took up more time than I wanted

pnkfelix (Sep 20 2018 at 14:57, on Zulip):

next "[WIP] rustc: check that type alias where clauses are well-formed." #54352

pnkfelix (Sep 20 2018 at 14:58, on Zulip):

@eddyb can you summarize?

eddyb (Sep 20 2018 at 14:58, on Zulip):

right, so, we plan to improve type alias checking in Rust 2018

eddyb (Sep 20 2018 at 14:58, on Zulip):

and figure out what we need to integrate type aliases better into the typesystem

eddyb (Sep 20 2018 at 14:59, on Zulip):

so far the data shows that requiring "enough bounds" is hopeless (400 regressions, migration can't be automated atm) while denying "too many bounds" has a handful of regressions

eddyb (Sep 20 2018 at 15:00, on Zulip):

however, while @nikomatsakis pointed out we can treat type X = Y; as getting implied bounds from WF(Y), we have to be careful around bounds that are written

mw (Sep 20 2018 at 15:00, on Zulip):

/me is off

eddyb (Sep 20 2018 at 15:00, on Zulip):

e.g. do we want type Foo<S: Into<str>> = Vec<S>; to compile without a hard error?

pnkfelix (Sep 20 2018 at 15:00, on Zulip):

In 2015 alone? Or in 2015 and 2018 ?

eddyb (Sep 20 2018 at 15:00, on Zulip):

the crater run found only two regressions

eddyb (Sep 20 2018 at 15:01, on Zulip):

yeah, edition-dependent behavior is the question here. personally I think it fits in our story of erroring in 2018 for some obscure things

nikomatsakis (Sep 20 2018 at 15:01, on Zulip):

e.g., enable said checks in Rust 2018?

eddyb (Sep 20 2018 at 15:01, on Zulip):

but do we do them in Rust 2015 too?

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

I'm okay with it being edition dependent.

nikomatsakis (Sep 20 2018 at 15:01, on Zulip):

I think we were saying:

nikomatsakis (Sep 20 2018 at 15:01, on Zulip):

I feel like that got said at some point

nikomatsakis (Sep 20 2018 at 15:02, on Zulip):

it seems to make sense to me

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

is the motivation for doing the error in edition 2015 ... to ease future porting to 2018?

eddyb (Sep 20 2018 at 15:02, on Zulip):

future-compat should be fine as there are only 2 regressions in obscure crates AFAICT

nikomatsakis (Sep 20 2018 at 15:02, on Zulip):

is the motivation for doing the error in edition 2015 ... to ease future porting to 2018?

yes, but also because it's misleading

eddyb (Sep 20 2018 at 15:02, on Zulip):

the motivation is type-system related, and soundness if we try to do anything clever

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

(okay yes of course there are other motivations)

eddyb (Sep 20 2018 at 15:03, on Zulip):

that is, even if there are things we can keep a warning forever/until 2021, there are subtle details that have worse implications and also have almost no regressions when fixing them

eddyb (Sep 20 2018 at 15:03, on Zulip):

that is, the bounds are just invalid as-written, instead of the usual missing bounds

nikomatsakis (Sep 20 2018 at 15:03, on Zulip):

I think we should make it a future-compat warning, personally, since I would like to stake the claim that this is a bug :P

eddyb (Sep 20 2018 at 15:03, on Zulip):

I agree

nikomatsakis (Sep 20 2018 at 15:03, on Zulip):

(but one we may or may not ever fix in Rust 2015)

varkor (Sep 20 2018 at 15:04, on Zulip):

was requiring enough bounds going to be enabled for 2018 as well?
or just denying too many?

eddyb (Sep 20 2018 at 15:04, on Zulip):

if we fix it, it allows us the "implied bounds" interpretation

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

So what i'm hearing is: We can do future-compat warning now, and potentially change edition-2015 behavior in a future release. And we make it hard error for edtion-2018, as planned

eddyb (Sep 20 2018 at 15:04, on Zulip):

which is, IMO, enough to cover switching to check-type-aliases-at-use-sites in the future on all editions

eddyb (Sep 20 2018 at 15:05, on Zulip):

@varkor on 2018, denying too-many only (plus cases like the one discussed right now) - not-enough has 400 regressions with no hope of automatically migrating them. and many of those crates have dependents

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

okay and I think @eddyb is also on board for plan as I summarized it?

eddyb (Sep 20 2018 at 15:05, on Zulip):

yupp, it seems we're in agreement

eddyb (Sep 20 2018 at 15:06, on Zulip):

(nominated it to double-check that we can carve off this special case from the main warning/ignore case with 400 regressions)

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

we're 6 minutes over time but I'd like to try to power through the remaining nominatins

nikomatsakis (Sep 20 2018 at 15:06, on Zulip):

I have to run now, but I feel a bit unsure about the situation re: "too few bounds" for Rust 2018

eddyb (Sep 20 2018 at 15:06, on Zulip):

sadly I nominated another thing

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

next one is: "Compiler panic using 'static_assertions'" #54327

nikomatsakis (Sep 20 2018 at 15:06, on Zulip):

/me will stay a few more

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

I don't think this needed to be nominated

pnkfelix (Sep 20 2018 at 15:07, on Zulip):

but .. why iddn't I see it during ... oh we hadn't looked at stable-to-stable regressions yet

nagisa (Sep 20 2018 at 15:07, on Zulip):

well, it needs a P- tag.

pnkfelix (Sep 20 2018 at 15:07, on Zulip):

and the people doing tagging didn't use P-high, okay yes

pnkfelix (Sep 20 2018 at 15:07, on Zulip):

seems P-high to me

nikomatsakis (Sep 20 2018 at 15:07, on Zulip):

yeah I suppose so

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

lets move on

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

i nominated "NLL: disallow creation of immediately unusable variables" #54188

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

but I'm going to skip it

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

the intention was to discuss the broad issue of how PR authors should delegate investigation of errors that solely arise due to e.g. code-gen units linker issues

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

but in this case the PR was actually relevant to why the test started failing

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

(the aforementioned bug #54388 )

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

I would like to have a discussion about this, but I don't want to take time for it now

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

we can do it asynchronously

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

last nominated issue: "Report const eval error inside the query" #53821

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

@eddyb you want to take over now?

eddyb (Sep 20 2018 at 15:12, on Zulip):

okay, so, is @RalfJ around?

eddyb (Sep 20 2018 at 15:13, on Zulip):

@Oli and @RalfJ have been working on this refactor of miri error reporting so it happens during const-eval instead of by whatever invoked const-eval

eddyb (Sep 20 2018 at 15:13, on Zulip):

which is fine, but there are behavioral changes and potentially backwards-incompat regressions coming out of it

nikomatsakis (Sep 20 2018 at 15:13, on Zulip):

so when @Oli wrote:

Functional changes: We no longer warn about bad constants embedded in unused types. This relied on being able to report just a warning, not a hard error on that case, which we cannot do any more now that error reporting is consistently centralized.

they mean that we now error

nagisa (Sep 20 2018 at 15:13, on Zulip):

@eddyb what’s the distinction?

nikomatsakis (Sep 20 2018 at 15:13, on Zulip):

as opposed to doing nothing

eddyb (Sep 20 2018 at 15:14, on Zulip):

@nagisa some places chose to warn, some places to error, but everything errors now

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

(right?)

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

hmm

nagisa (Sep 20 2018 at 15:14, on Zulip):

I mean what’s the distinction between > so it happens during const-eval instead of by whatever invoked const-eval

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

how hard would it be to buffer the errors and downgrade them when appropriate?

eddyb (Sep 20 2018 at 15:14, on Zulip):

@nagisa some places reported the error as a warning, instead. now it's always an error

nagisa (Sep 20 2018 at 15:14, on Zulip):

ain’t const eval gonna be invoked by many things at all points in the compiler anyway?

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

(as we did in NLL)

eddyb (Sep 20 2018 at 15:14, on Zulip):

@pnkfelix that's what is being refactored away

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

that ... doesn't sound like "refactoring" to me...

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

but killing off functionality is fact-of-life at times

eddyb (Sep 20 2018 at 15:15, on Zulip):

one thing to keep in mind is that we should try to fit this into the type alias story

varkor (Sep 20 2018 at 15:15, on Zulip):

was the previous functionality actually useful, or just a side effect of the implementation?

varkor (Sep 20 2018 at 15:16, on Zulip):

because always erroring seems like a good idea

eddyb (Sep 20 2018 at 15:16, on Zulip):

one way I thought about conceptualizing it is:
type Foo = [u8; 0 - 1] should always error, because the constant doesn't depend on how Foo is used - Foo just can't be used

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

but if its an artifact of e.g. macro-generated code

eddyb (Sep 20 2018 at 15:17, on Zulip):

and we should extend this to other things, not just constants, e.g. type Foo = Vec<str>; (working on a PR atm, to crater this hypothesis)

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

then its expected that, sure, it can't be used...

eddyb (Sep 20 2018 at 15:17, on Zulip):

interesting, good point!

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

(Don't get me wrong, I'm in principle in favor of an error. But it seems a shame if it injects regressions...)

varkor (Sep 20 2018 at 15:18, on Zulip):

is it a silly/impossible idea to give more leeway to code generated by macros than user-written code?

nikomatsakis (Sep 20 2018 at 15:18, on Zulip):

have we cratered this PR, at least?

nagisa (Sep 20 2018 at 15:18, on Zulip):

is it a silly/impossible idea to give more leeway to code generated by macros than user-written code?

I would rather not.

varkor (Sep 20 2018 at 15:18, on Zulip):

because there are other places bad practices are permitted because of macros

nikomatsakis (Sep 20 2018 at 15:18, on Zulip):

is it a silly/impossible idea to give more leeway to code generated by macros than user-written code?

I feel like the way we do that is to issue warnings

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

I'm not sufficiently familiar with the architecture to understand: Can we not feed in more context into the MIRI interpreter saying it should warn instead of error?

nikomatsakis (Sep 20 2018 at 15:19, on Zulip):

(which macros can silence)

nikomatsakis (Sep 20 2018 at 15:19, on Zulip):

I gotta go though

eddyb (Sep 20 2018 at 15:19, on Zulip):

the PR is not cratered :(

eddyb (Sep 20 2018 at 15:19, on Zulip):

but @RalfJ wants to push it through because he wants to make changes that would otherwise conflict, after it lands...

varkor (Sep 20 2018 at 15:20, on Zulip):

it would only take a day to check crater run

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

okay, what scenarios does this change affect?

RalfJ (Sep 20 2018 at 15:20, on Zulip):

hi there, what's the question?

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

I think we may want to resolve this asynchronously at this point, as I have to leave as well

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

maybe lets change to a dedicated topic for this

eddyb (Sep 20 2018 at 15:21, on Zulip):

yeah

RalfJ (Sep 20 2018 at 15:22, on Zulip):

okay then I better make that phone call first

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

(shifted to topic: "#53821 Report const eval error inside the query" )

Last update: Nov 22 2019 at 04:40UTC