Stream: t-compiler

Topic: ICE with incorrect turbofish #60989


Pietro Albini (May 23 2019 at 14:18, on Zulip):

let's split a topic -- @oli @varkor @centril @pnkfelix

centril (May 23 2019 at 14:19, on Zulip):

@oli reduced some more: https://github.com/rust-lang/rust/issues/60989#issuecomment-495239018

pnkfelix (May 23 2019 at 14:21, on Zulip):

(@centril I moved your message over here)

Pietro Albini (May 23 2019 at 14:21, on Zulip):

so, to reiterate, if I'm the one running the release a patch has to be r+ed before 17 UTC, which is in ~1.5 hours

Pietro Albini (May 23 2019 at 14:22, on Zulip):

@oli if you make a PR please target it to the stable branch directly, we'll forward port it later

oli (May 23 2019 at 14:23, on Zulip):

will do

oli (May 23 2019 at 14:23, on Zulip):

my working theory is that prohibit_generics is now not called anymore because the early return was moved up

centril (May 23 2019 at 14:23, on Zulip):

@oli Reduced maximally:

fn main() {
    let c1 = ();
    c1::<()>;
}
oli (May 23 2019 at 14:26, on Zulip):

Ok, my theory is becoming certainty

oli (May 23 2019 at 14:26, on Zulip):

it's just nontrivial to fix :D I basically need to revert the PR that introduced the bug and fix self resolution again

Pietro Albini (May 23 2019 at 14:34, on Zulip):

I have to leave for around an hour -- once the PR is r+ed p=1000 it and @bors retry any other build currently running

pnkfelix (May 23 2019 at 14:54, on Zulip):

is the ICE from the PR that introduced the bug something that is less impactful than this ICE?

pnkfelix (May 23 2019 at 14:55, on Zulip):

in other words, would we be best off just reverting that PR for the short-term

pnkfelix (May 23 2019 at 14:55, on Zulip):

?

centril (May 23 2019 at 14:56, on Zulip):

reverting the PR would mean removing a run-pass test

centril (May 23 2019 at 14:56, on Zulip):

not a good idea

pnkfelix (May 23 2019 at 14:58, on Zulip):

it depends on the run-pass test

pnkfelix (May 23 2019 at 14:58, on Zulip):

(basically trying to weigh impact of one ICE vs another)

centril (May 23 2019 at 14:59, on Zulip):

We can't do a crater run to ascertain the breakage and it could break stable code

pnkfelix (May 23 2019 at 14:59, on Zulip):

look at this: https://github.com/rust-lang/rust/issues/57924#issuecomment-457892376

pnkfelix (May 23 2019 at 15:00, on Zulip):

the bug this was fixing was on-going for a number of releases

pnkfelix (May 23 2019 at 15:00, on Zulip):

(starting, IIUC, in 1.30)

oli (May 23 2019 at 15:00, on Zulip):

I think I have a fix, testing right now

pnkfelix (May 23 2019 at 15:01, on Zulip):

(in other words, I'm trying to say that I think Self::<stuff> occurs less often than random_ident::<stuff>)

pnkfelix (May 23 2019 at 15:01, on Zulip):

but hopefully @oli 's work will make the debate moot

pnkfelix (May 23 2019 at 15:02, on Zulip):

now my question is: How did we not encounter this in our own test suite?

centril (May 23 2019 at 15:02, on Zulip):

Probably, but you also have to factor in that an ICE for erroring code is still a form of non-compilation while an ICE for non-erroring code is non-compliance with "the spec" from rustc's side

centril (May 23 2019 at 15:02, on Zulip):

now my question is: How did we not encounter this in our own test suite?

Yes. That is the best question!

pnkfelix (May 23 2019 at 15:03, on Zulip):

I'm sort of shocked tthat the example that @centril came up with wasn't hit in our our source base and/or test suite. Maybe we don't use turbo-fish all that often ourselves?

centril (May 23 2019 at 15:03, on Zulip):

@pnkfelix I think we often are shoddy with our testing

pnkfelix (May 23 2019 at 15:03, on Zulip):

yep

centril (May 23 2019 at 15:03, on Zulip):

We are optimistic with our input space partitioning and don't do combinatorial tests

Esteban K├╝ber (May 23 2019 at 15:04, on Zulip):

We're very _reactive_, with notable exceptions for _some_ new features.

centril (May 23 2019 at 15:04, on Zulip):

with notable exceptions for _some_ new features.

I hope for it to stay this way -- I'm going to try to require high test coverage for stabilizations.

centril (May 23 2019 at 15:05, on Zulip):

and start with high test coverage from the very start

Pietro Albini (May 23 2019 at 15:31, on Zulip):

[I'm back]

Pietro Albini (May 23 2019 at 16:20, on Zulip):

@eddyb how's going?

eddyb (May 23 2019 at 16:20, on Zulip):

/me stares at Zulip

eddyb (May 23 2019 at 16:21, on Zulip):

@Pietro Albini I got it working, will push in minute

Pietro Albini (May 23 2019 at 16:21, on Zulip):

:heart:

centril (May 23 2019 at 16:25, on Zulip):

:tada:

eddyb (May 23 2019 at 16:26, on Zulip):

done, how do we handle the review?

Pietro Albini (May 23 2019 at 16:28, on Zulip):

@oli @varkor @pnkfelix any of you around for the review of https://github.com/rust-lang/rust/pull/61085?

varkor (May 23 2019 at 16:56, on Zulip):

yes, I'll do that now

eddyb (May 23 2019 at 16:58, on Zulip):

in it's current state, it's basically a partial revert of https://github.com/rust-lang/rust/pull/59025

eddyb (May 23 2019 at 16:58, on Zulip):

where rewrite_self_ctor still exists, but only does the Def -> Def rewrite

varkor (May 23 2019 at 17:04, on Zulip):

okay, it's being tested by bors now

Pietro Albini (May 23 2019 at 17:04, on Zulip):

:tada:

Pietro Albini (May 23 2019 at 17:04, on Zulip):

thanks everybody for the quick fix!

varkor (May 23 2019 at 17:06, on Zulip):

this will also need backporting to beta, right?

Pietro Albini (May 23 2019 at 17:06, on Zulip):

yeah, and to master

Last update: Nov 20 2019 at 01:25UTC