Stream: t-compiler/wg-nll

Topic: #55241 parity-common ICE


nikomatsakis (Oct 23 2018 at 20:17, on Zulip):

I am looking at this @pnkfelix fyi. Getting kind of worried about these ICEs.

pnkfelix (Oct 23 2018 at 20:18, on Zulip):

okay i'm glad you're looking at it

nikomatsakis (Oct 23 2018 at 20:18, on Zulip):

ah, wait, I ran my test in the wrong branch

nikomatsakis (Oct 23 2018 at 20:18, on Zulip):

I am hoping my PR fixes it :P

pnkfelix (Oct 23 2018 at 20:18, on Zulip):

I was going to try to look at the ICE's after I finished trying to fix my PR

nikomatsakis (Oct 23 2018 at 20:18, on Zulip):

I remember fixing some similar asserions

nikomatsakis (Oct 23 2018 at 20:19, on Zulip):

sadly, still fails

pnkfelix (Oct 23 2018 at 20:19, on Zulip):

are you also looking at the "ICE when accessing an associated constant of a generic impl" #55219 ?

nikomatsakis (Oct 23 2018 at 20:20, on Zulip):

not yet

pnkfelix (Oct 23 2018 at 20:20, on Zulip):

that one, #55219, I hope will have a straight forward fix

nikomatsakis (Oct 23 2018 at 20:20, on Zulip):

oh, this is one of those weird bugs... goes away with incremental rebuilds

nikomatsakis (Oct 23 2018 at 20:21, on Zulip):

that one was also reported on #55241

pnkfelix (Oct 23 2018 at 20:21, on Zulip):

yeah I was going to make a comment about that

pnkfelix (Oct 23 2018 at 20:21, on Zulip):

but I didn't get a chance to look at the tar.gz

pnkfelix (Oct 23 2018 at 20:21, on Zulip):

to verify that it really was a duplicate

pnkfelix (Oct 23 2018 at 20:21, on Zulip):

in terms of being a case of impl<T> Foo<T> { const C: Other = ...; ... }

nikomatsakis (Oct 23 2018 at 20:22, on Zulip):

that one at least has a minimzed test case I guess

nikomatsakis (Oct 23 2018 at 20:22, on Zulip):

it still reproduces with my PR too :)

pnkfelix (Oct 23 2018 at 20:22, on Zulip):

I can imagine why it happens

pnkfelix (Oct 23 2018 at 20:22, on Zulip):

impl<T> Foo<T> { const C: u32 = 10; ... }

pnkfelix (Oct 23 2018 at 20:23, on Zulip):

it not surprising that T is in some sense floating around as an infer type when we look at the const C

nikomatsakis (Oct 23 2018 at 20:23, on Zulip):

hmm

nikomatsakis (Oct 23 2018 at 20:23, on Zulip):

true

pnkfelix (Oct 23 2018 at 20:23, on Zulip):

I also mused about suggesting that we just ... kill the assertion ..

nikomatsakis (Oct 23 2018 at 20:23, on Zulip):

well

pnkfelix (Oct 23 2018 at 20:23, on Zulip):

but I suspect that would be bad

nikomatsakis (Oct 23 2018 at 20:23, on Zulip):

plausible

pnkfelix (Oct 23 2018 at 20:24, on Zulip):

better to find a more principled fix

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

we could maybe move it

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

I think there is another one

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

in particular, the goal is to ensure that we don't instantiate a type variable with a value that contains another type variable

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

well that would probably happen

pnkfelix (Oct 23 2018 at 20:24, on Zulip):

occurs check!

pnkfelix (Oct 23 2018 at 20:24, on Zulip):

(not exactly, I know)

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

right we could basically just implement the proper path

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

or at least more of it

nikomatsakis (Oct 23 2018 at 20:24, on Zulip):

and then this assert wouldn't be needed

nikomatsakis (Oct 23 2018 at 20:25, on Zulip):

that may be the right fix

nikomatsakis (Oct 23 2018 at 20:25, on Zulip):

e.g., maybe we allow a type variable, but only if it is unbound or something...?

nikomatsakis (Oct 23 2018 at 20:25, on Zulip):

the other ICE (#55241) definitely worries me more

nikomatsakis (Oct 23 2018 at 20:28, on Zulip):

ah, I think I see what it is

nikomatsakis (Oct 23 2018 at 20:41, on Zulip):

@pnkfelix I think I might be able to fix both of those ICEs at once

nikomatsakis (Oct 23 2018 at 21:09, on Zulip):

so the problem here is basically that the NLL type checker is a bit too rigid, I think

nikomatsakis (Oct 23 2018 at 21:09, on Zulip):

(also that we are encoding tons more user-type annotations than we really need, but I guess it's good to be able to handle them)

nikomatsakis (Oct 23 2018 at 21:09, on Zulip):

/me rubber ducks a bit

nikomatsakis (Oct 23 2018 at 21:09, on Zulip):

e.g., in one of the ICE cases, we have C::Out, which expands to <C as Hasher<_>>::Out

nikomatsakis (Oct 23 2018 at 21:09, on Zulip):

this fails to normalize because we don't know what the _ is

nikomatsakis (Oct 23 2018 at 21:10, on Zulip):

the only reason it works ever is that we have C: Hasher<H> in the environment

nikomatsakis (Oct 23 2018 at 21:10, on Zulip):

and hence we can infer that _ must be H because there is no other way to prove C: Hasher

nikomatsakis (Oct 23 2018 at 21:10, on Zulip):

problem is, in the NLL type checker, we normalize first and then solve predicates

nikomatsakis (Oct 23 2018 at 21:11, on Zulip):

but if we swap the order, I think other things will fail, because the predicates might depend on stuff that proving type equality needed

nikomatsakis (Oct 23 2018 at 21:11, on Zulip):

what we can do though is to add a query that creates a fresh infcx and adds all these constraints so that they can get solved in any order

nikomatsakis (Oct 23 2018 at 21:12, on Zulip):

this would also let us go through the more general infcx code paths and avoid the limitations around type variables that triggered that other assertion

pnkfelix (Oct 24 2018 at 10:27, on Zulip):

hey @nikomatsakis if you've already extracted a standalone test case for #55421, would you mind posting it to the issue for posterity's sake (and so that I or others don't waste time trying to do so in parallel) ?

nikomatsakis (Oct 24 2018 at 10:28, on Zulip):

@pnkfelix I haven't yet but I prob can later this morning

nikomatsakis (Oct 24 2018 at 10:28, on Zulip):

or .. did I ..

nikomatsakis (Oct 24 2018 at 10:28, on Zulip):

I forget :)

pnkfelix (Oct 24 2018 at 10:28, on Zulip):

no problem, just wanted to make sure I mentioned it somewhere

pnkfelix (Oct 24 2018 at 10:28, on Zulip):

(i'm going through all my issues and trying to close off the easy tasks before I embark on round 3 of .stderr review for NLL)

Last update: Nov 21 2019 at 13:50UTC