Stream: wg-traits

Topic: #57843 universe-related ICE


nikomatsakis (Jan 29 2019 at 14:23, on Zulip):

So, @pnkfelix ...

nikomatsakis (Jan 29 2019 at 14:23, on Zulip):

probably I should indeed make my own local build, but can you convey what the type is which is failing the assertion?

pnkfelix (Jan 29 2019 at 14:24, on Zulip):

I'll attempt to do so

pnkfelix (Jan 29 2019 at 14:24, on Zulip):

std::boxed::Box<[closure@../../issue-57843-ice-with-cloneablefn.rs:12:18: 12:24]>

pnkfelix (Jan 29 2019 at 14:24, on Zulip):

i.e. the closure in the fn main

pnkfelix (Jan 29 2019 at 14:25, on Zulip):

which is somewhat unsurprising given that the ICE goes away when one adds a type-annotation to the closure formal parameter

nikomatsakis (Jan 29 2019 at 14:25, on Zulip):

maybe with -Zverbose?

pnkfelix (Jan 29 2019 at 14:25, on Zulip):

that was with -Z verbose

nikomatsakis (Jan 29 2019 at 14:25, on Zulip):

hmm

nikomatsakis (Jan 29 2019 at 14:25, on Zulip):

is the problem that it has a placeholder in it?

pnkfelix (Jan 29 2019 at 14:25, on Zulip):

the type in question has needs_infer: false, has_ty_placholders: false, has_re_placholders: true

pnkfelix (Jan 29 2019 at 14:26, on Zulip):

yes

pnkfelix (Jan 29 2019 at 14:26, on Zulip):

i.e. the region placeholder

pnkfelix (Jan 29 2019 at 14:26, on Zulip):

oh let me just gist my log

nikomatsakis (Jan 29 2019 at 14:26, on Zulip):

yes ok

pnkfelix (Jan 29 2019 at 14:26, on Zulip):

https://gist.github.com/070ea7fe4b4a95c8129c06694cbeb457

nikomatsakis (Jan 29 2019 at 14:26, on Zulip):

I can sort of see what's happening

pnkfelix (Jan 29 2019 at 14:27, on Zulip):

(this is after I added a couple of debug! statements)

pnkfelix (Jan 29 2019 at 14:28, on Zulip):

and with RUST_LOG=rustc::traits,rustc::infer,rustc::ty::sty,rustc::infer::higher_ranked,rustc_typeck::check

nikomatsakis (Jan 29 2019 at 14:29, on Zulip):

(ok, doing a local build)

nikomatsakis (Jan 29 2019 at 14:29, on Zulip):

but also skimming some of the closure code

pnkfelix (Jan 29 2019 at 14:30, on Zulip):

so as part of the instrumentation I added debug! for the (few) places where RePlaceholder are created

nikomatsakis (Jan 29 2019 at 14:30, on Zulip):

just to leave a few random notes

pnkfelix (Jan 29 2019 at 14:30, on Zulip):

there are only two lines that correspond to RePlaceholder(Placeholder { universe: U6, name: BrNamed(crate0:DefIndex(1:14), 'a) })

nikomatsakis (Jan 29 2019 at 14:31, on Zulip):
pnkfelix (Jan 29 2019 at 14:32, on Zulip):

https://gist.github.com/pnkfelix/070ea7fe4b4a95c8129c06694cbeb457#file-rlog-L3715

pnkfelix (Jan 29 2019 at 14:32, on Zulip):

https://gist.github.com/pnkfelix/070ea7fe4b4a95c8129c06694cbeb457#file-rlog-L3779

nikomatsakis (Jan 29 2019 at 14:32, on Zulip):

...which creates the liberated signature...

nikomatsakis (Jan 29 2019 at 14:33, on Zulip):

hmm, I don't see anything obviously wrong with this path

nikomatsakis (Jan 29 2019 at 14:33, on Zulip):

@pnkfelix the type which winds up being invalid, what is it the type of?

nikomatsakis (Jan 29 2019 at 14:33, on Zulip):

oh, my build is done

nikomatsakis (Jan 29 2019 at 14:33, on Zulip):

er, no it's not

pnkfelix (Jan 29 2019 at 14:33, on Zulip):

the Boxed closure?

nikomatsakis (Jan 29 2019 at 14:34, on Zulip):

I guess I can browser your gist

pnkfelix (Jan 29 2019 at 14:34, on Zulip):

here: https://gist.github.com/pnkfelix/070ea7fe4b4a95c8129c06694cbeb457#file-rlog-L5539

pnkfelix (Jan 29 2019 at 14:35, on Zulip):

but I think I must misunderstand your Q, since "boxed closure" is what I pasted up above

pnkfelix (Jan 29 2019 at 14:37, on Zulip):

hmm

pnkfelix (Jan 29 2019 at 14:37, on Zulip):

my debug log doesn't show a call to sig_of_closure_with_expectation

pnkfelix (Jan 29 2019 at 14:40, on Zulip):

but even sig_of_closure_no_expectation will also go down into closure_sigs which is what is calling liberate_late_bound_regions ..

nikomatsakis (Jan 29 2019 at 14:41, on Zulip):

@pnkfelix I'm poking at your log, sorry, still trying to understand just where the placeholder is/comes from. I really dislike how types print out, even with -Zverbose :(

pnkfelix (Jan 29 2019 at 14:41, on Zulip):

no need to be sorry; hearing you being unsure about where they arise makes me feel less dumb about my own inability to determine it

pnkfelix (Jan 29 2019 at 14:43, on Zulip):

I believe they are coming from rustc::traits::select::match_impl

pnkfelix (Jan 29 2019 at 14:44, on Zulip):

(but maybe you want to identify the culprit further from the hot end of the call stack)

pnkfelix (Jan 29 2019 at 14:47, on Zulip):

maybe you can supply me with higher level understanding of the design here

pnkfelix (Jan 29 2019 at 14:48, on Zulip):

I infer that that placeholders (of any kind, region or type) are not meant to show up in the tables that we write into in writeback.rs, right?

pnkfelix (Jan 29 2019 at 14:49, on Zulip):

so the code that does replace_bound_vars_with_placeholders ... is that only supposed to be creating types that are just temporary objects that we use for e.g. comparisons with other types, and then discard?

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

right

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

what should be happening is that we create placeholders

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

but if they wind up related to some inference variable

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

that would appear in the writeback table

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

then the varibale is in an earlier universe that cannot contain the placeholder

nikomatsakis (Jan 29 2019 at 14:51, on Zulip):

and hence gets changed during inference

nikomatsakis (Jan 29 2019 at 15:38, on Zulip):

@pnkfelix i think maybe I found the problem

pnkfelix (Jan 29 2019 at 15:39, on Zulip):

I'm all ears

nikomatsakis (Jan 29 2019 at 15:39, on Zulip):

let me double check one thing

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

ok I haven't quite proven it yet but

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

I'm pretty sure the problem is this 'opportunistic region resolve' thing

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

right now, if we have a make_eqregion call, we unify region variables

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

but if they are in different universes

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

I think this can cause the placeholder to leak out

nikomatsakis (Jan 29 2019 at 15:44, on Zulip):

we're not using a universe-aware unification, in other words

pnkfelix (Jan 29 2019 at 15:46, on Zulip):

hmm

pnkfelix (Jan 29 2019 at 15:46, on Zulip):

that sounds more subtle than what I was hoping for

pnkfelix (Jan 29 2019 at 15:46, on Zulip):

but an understandable oversight, I guess

nikomatsakis (Jan 29 2019 at 16:05, on Zulip):

I'm still trrying to "prove it"

nikomatsakis (Jan 29 2019 at 16:06, on Zulip):

not sure the best fix

nikomatsakis (Jan 29 2019 at 16:07, on Zulip):

I might be wrong

nikomatsakis (Jan 29 2019 at 16:08, on Zulip):

although if I am, it sort of suggests another possible bug lurking :)

nikomatsakis (Jan 29 2019 at 16:09, on Zulip):

@pnkfelix ok I thnk i'm wrong

nikomatsakis (Jan 29 2019 at 16:09, on Zulip):

but I think know what the bug is now

nikomatsakis (Jan 29 2019 at 17:18, on Zulip):

well, it seems like I am wrong again. I thought the problem (which seems like maybe another bug lurking) had to do with type generalization using the wrong universes.

nikomatsakis (Jan 29 2019 at 17:19, on Zulip):

I think @pnkfelix maybe it'd be interesting tomorrow morning to try and walk through this a bit together, though my schedule tomorrow likely doesn't mesh with yours (I think I am available only starting at 11:00 UTC-05:00)

pnkfelix (Jan 30 2019 at 10:21, on Zulip):

yeah that's around when I have to leave to pick up Logan

pnkfelix (Jan 30 2019 at 10:21, on Zulip):

though actually: You know, I could just leave earlier and work from home before then

pnkfelix (Jan 30 2019 at 10:21, on Zulip):

that would allow us to work together for maybe 25 minutes or so, starting at 11:00 UTC-05:00

nikomatsakis (Jan 30 2019 at 12:30, on Zulip):

ok let's do that

nikomatsakis (Jan 30 2019 at 16:02, on Zulip):

@pnkfelix ping

pnkfelix (Jan 30 2019 at 16:03, on Zulip):

argh

pnkfelix (Jan 30 2019 at 16:03, on Zulip):

I didn't actually leave in time

pnkfelix (Jan 30 2019 at 16:04, on Zulip):

because I got nerd-sniped doing #57735

pnkfelix (Jan 30 2019 at 16:04, on Zulip):

so I just cannot do a pair up right now; I have to leave too soon. :sad:

nikomatsakis (Jan 30 2019 at 16:05, on Zulip):

@pnkfelix no worries, maybe I'll dig a bit and leave some more notes?

pnkfelix (Jan 30 2019 at 16:06, on Zulip):

sure. sorry for failing to live up to the proposal

nikomatsakis (Jan 30 2019 at 16:06, on Zulip):

We could try to sync up a bit tomorrow morning before compiler meeting (though I know you often do pre-triage) or Friday, too

nikomatsakis (Jan 30 2019 at 16:06, on Zulip):

well, it was a bit tight

nikomatsakis (Jan 31 2019 at 15:56, on Zulip):

@pnkfelix so I wasn't able to follow up on this

nikomatsakis (Jan 31 2019 at 15:56, on Zulip):

but I still want to =)

pnkfelix (Jan 31 2019 at 15:56, on Zulip):

Do you have time tomorrow?

nikomatsakis (Jan 31 2019 at 15:56, on Zulip):

Do you think you'd like to try pairing tomorrow morning?

nikomatsakis (Jan 31 2019 at 15:56, on Zulip):

Yeah, Friday is relatively free for me right now

pnkfelix (Jan 31 2019 at 15:56, on Zulip):

yeah sure

pnkfelix (Jan 31 2019 at 15:57, on Zulip):

the only thing I want to do is go over all the homework for Berlin. :)

pnkfelix (Jan 31 2019 at 15:57, on Zulip):

but we should carve out time for this

pnkfelix (Jan 31 2019 at 15:58, on Zulip):

Your schedule is more constrained than mine. Do you have a time in mind?

pnkfelix (Jan 31 2019 at 15:58, on Zulip):

If not then you can just ping me.

pnkfelix (Jan 31 2019 at 15:59, on Zulip):

oh wait, let me check something

nikomatsakis (Jan 31 2019 at 16:17, on Zulip):

@pnkfelix I was imagining 9am UTC-5

pnkfelix (Feb 01 2019 at 14:07, on Zulip):

@nikomatsakis is that, like, now?

nikomatsakis (Feb 01 2019 at 14:23, on Zulip):

@pnkfelix would've been, but I got held up

nikomatsakis (Feb 01 2019 at 14:24, on Zulip):

Also, I had in mind doing a video chat, but for .. reasons .. I'm holed up in a cafe now. But I would be happy to chat back and forth a bit on Zulip

nikomatsakis (Feb 01 2019 at 14:24, on Zulip):

regardless I plan to spend some time investigating

nikomatsakis (Feb 01 2019 at 14:34, on Zulip):

So, some notes:

nikomatsakis (Feb 01 2019 at 14:34, on Zulip):
nikomatsakis (Feb 01 2019 at 14:35, on Zulip):

but I'm trying to remember why I thought that

nikomatsakis (Feb 01 2019 at 14:35, on Zulip):

the output of closure types is super unhelpful here

pnkfelix (Feb 01 2019 at 14:37, on Zulip):

Maybe we should revise their output under -Z verbose

pnkfelix (Feb 01 2019 at 14:38, on Zulip):

anyway sure lets chat here for maybe 20minutes; I am going to attempt to take Logan to his first trip to a barber after that.

nikomatsakis (Feb 01 2019 at 14:39, on Zulip):

Maybe we should revise their output under -Z verbose

I'm doing that now to double check a few things

nikomatsakis (Feb 01 2019 at 14:39, on Zulip):

but what I believe is that _#9t is the type variable we use to represent the closure signature

nikomatsakis (Feb 01 2019 at 14:39, on Zulip):

in any case, '_#9t gets created when we are doing some trait solving

nikomatsakis (Feb 01 2019 at 14:39, on Zulip):

at first I was confused because

nikomatsakis (Feb 01 2019 at 14:39, on Zulip):

I presumed that the placeholder from U6 was "leaking" into a variable where it didn't belong

nikomatsakis (Feb 01 2019 at 14:40, on Zulip):

the basic idea being that any type which writeback is trying to resolve

nikomatsakis (Feb 01 2019 at 14:41, on Zulip):

should be in universe 0

nikomatsakis (Feb 01 2019 at 14:41, on Zulip):

and hence not able to contain placeholders

nikomatsakis (Feb 01 2019 at 14:42, on Zulip):

but for some reason '_#9t is defined in universe 6 I believe

pnkfelix (Feb 01 2019 at 14:46, on Zulip):

the hypothesized "leaking" was occurring due to this line: https://gist.github.com/pnkfelix/070ea7fe4b4a95c8129c06694cbeb457#file-rlog-L3735 ?

nikomatsakis (Feb 01 2019 at 14:46, on Zulip):

right

nikomatsakis (Feb 01 2019 at 14:46, on Zulip):

so

nikomatsakis (Feb 01 2019 at 14:46, on Zulip):

I found what I think are two bugs so far :)

nikomatsakis (Feb 01 2019 at 14:46, on Zulip):

but neither of them apply here

nikomatsakis (Feb 01 2019 at 14:47, on Zulip):

because in this case '_#9t is in U6, so it's not really a leak

pnkfelix (Feb 01 2019 at 14:47, on Zulip):

as my thesis advisor used to say

pnkfelix (Feb 01 2019 at 14:47, on Zulip):

"that's great news!"

pnkfelix (Feb 01 2019 at 14:47, on Zulip):

(or maybe it was more like "congratulations! You found a bug!")

nikomatsakis (Feb 01 2019 at 14:47, on Zulip):

so i'm trying now to dig a bit more into the context around this '_#9t

nikomatsakis (Feb 01 2019 at 14:47, on Zulip):

I have the fixes for the bugs in my branch, so afer this maybe I'll try to create test cases

pnkfelix (Feb 01 2019 at 14:48, on Zulip):

its ('_#9t) for the type parameter T, right? https://gist.github.com/pnkfelix/070ea7fe4b4a95c8129c06694cbeb457#file-rlog-L3718

pnkfelix (Feb 01 2019 at 14:49, on Zulip):

(in the context of the instantiation of that trait within fn main, I assume)

nikomatsakis (Feb 01 2019 at 14:50, on Zulip):

yeah, presumably. So I see this:

DEBUG 2019-02-01T14:48:44Z: rustc::infer::at: eq(
<[closure@/home/nmatsakis/tmp/57843.rs:15:18: 15:24 closure_kind_ty=_#4t closure_sig_ty=extern "rust-call" fn((_#3t,))] as ClonableFn<&RePlaceholder(Placeholder { universe: U6, name: BrNamed(crate0:DefIndex(1:14), 'a) }) bool>>
==
<_#10t as ClonableFn<_#9t>>)
nikomatsakis (Feb 01 2019 at 14:52, on Zulip):

which I think is coming from trait solving

nikomatsakis (Feb 01 2019 at 14:53, on Zulip):

the debug output is wacky here but basically it's unifying the impl trait ref (_#10t: CloneableFn<_#9t>) with the thing the closure offers

nikomatsakis (Feb 01 2019 at 14:53, on Zulip):

and you can see that the closure has a placeholder

pnkfelix (Feb 01 2019 at 14:57, on Zulip):

I have a higher level question

pnkfelix (Feb 01 2019 at 14:58, on Zulip):

which is: What .. do you expect here?

pnkfelix (Feb 01 2019 at 14:58, on Zulip):

for no type at all to be fed into writeback ?

pnkfelix (Feb 01 2019 at 14:58, on Zulip):

(for the higher-ranked closure, that is)

pnkfelix (Feb 01 2019 at 14:58, on Zulip):

or for a type with a different structure to be fed in?

nikomatsakis (Feb 01 2019 at 14:59, on Zulip):

which is: What .. do you expect here?

I expect -- I think -- a region inference error, but I guess I need to go back and study the example program

nikomatsakis (Feb 01 2019 at 15:00, on Zulip):
DEBUG 2019-02-01T14:56:51Z: rustc::traits::select: assemble_candidates_from_impls(obligation=Obligation(predicate=Binder(TraitPredicate(<[closure@/home/nmatsakis/tmp/57843.rs:15:18: 15:24 closure_kind_ty=_#4t closure_sig_ty=extern "rust-call" fn((_#3t,))] as ClonableFn<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(1:14), 'a)) bool>>)),cause=ObligationCause { span: /home/nmatsakis/tmp/57843.rs:15:9: 15:25, body_id: NodeId(115), code: ObjectCastObligation((dyn for<'a> ClonableFn<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(1:14), 'a)) bool> + '_#1r)) },param_env=ParamEnv { caller_bounds: [], reveal: UserFacing, def_id: None },depth=2))
nikomatsakis (Feb 01 2019 at 15:00, on Zulip):

that is the context of what is happening

nikomatsakis (Feb 01 2019 at 15:00, on Zulip):

that I finally found

nikomatsakis (Feb 01 2019 at 15:00, on Zulip):

I realized I wasn't dumping the rustc::traits debug log

nikomatsakis (Feb 01 2019 at 15:00, on Zulip):

let me try to translate that

nikomatsakis (Feb 01 2019 at 15:01, on Zulip):

we are asked to solve:

for<'a> {
   Closure<fn(_#3t)>: ClonableFn(&'a bool)
}
nikomatsakis (Feb 01 2019 at 15:03, on Zulip):

where the fn(_#3t) is the "pseudo-type parameter" closures use to encode their expected signature

nikomatsakis (Feb 01 2019 at 15:03, on Zulip):

but now I want to circle back to the example program again

nikomatsakis (Feb 01 2019 at 15:03, on Zulip):

we have this

 Foo(Box::new(|_| ()));

where Foo is struct Foo(Box<dyn for<'a> ClonableFn<&'a bool>>)

nikomatsakis (Feb 01 2019 at 15:04, on Zulip):

and indeed when type-checking the closure we do have the "expected" type of

ExpectRvalueLikeUnsized((dyn for<'a> ClonableFn<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(1:14), 'a)) bool> + '_#0r)))
nikomatsakis (Feb 01 2019 at 15:05, on Zulip):

and yet we wind up with no expected signature:

DEBUG 2019-02-01T14:56:51Z: rustc_typeck::check::closure: check_closure(opt_kind=None, expected_sig=None)
nikomatsakis (Feb 01 2019 at 15:05, on Zulip):

I'm not sure why that is, but it fits my expectation that we should be seeing an error here

nikomatsakis (Feb 01 2019 at 15:06, on Zulip):

in particular, we've effectively created a type variable _#3t in the root universe and said the closure type has signature fn(_#3t) -- note the binder is not on the fn

nikomatsakis (Feb 01 2019 at 15:06, on Zulip):

but then we are tying to say (effectively)

exists<T3> { // the variable `_#3t`
    forall<'a> {
        T3 = &'a bool // because of the required fn signature
    }
}

and of course that should not work

nikomatsakis (Feb 01 2019 at 15:14, on Zulip):

@pnkfelix ok i'm digging a bit more. I suspect one of the bugs I found may be the culprit, but my fix was incompleted. In particular, I'm looking at the "generalize" code. I see these logs (from my branch, which has a few extra logs):

DEBUG 2019-02-01T14:56:51Z: rustc::infer::combine: generalize(ty=&'_#3r bool, for_vid=_#3t, dir=EqTo
DEBUG 2019-02-01T14:56:51Z: rustc::infer::combine: generalize: ambient_variance = o
DEBUG 2019-02-01T14:56:51Z: rustc::infer::combine: generalize: for_universe = U0
DEBUG 2019-02-01T14:56:51Z: rustc::infer::combine: generalize: success { &'_#3r bool, false }
DEBUG 2019-02-01T14:56:51Z: rustc::infer::combine: instantiate(a_ty=&'_#3r bool, dir=EqTo, b_vid=_#3t, generalized b_ty=&'_#3r bool)

what this is saying is that we have to unify _#3t (which is in the universe U0) with &'_#3r bool -- but '_#3r was created in the universe U6:

  12327:DEBUG 2019-02-01T14:56:51Z: rustc::infer::region_constraints: created new region variable '_#3r in U6 with origin MiscVariable(/home/nmatsakis/tmp/57843.rs:15:9: 15:25)

the plan was to "instantiate" '_#3t to &'X bool where 'X is a fresh variable in U0 (i.e., the universe of '_#3t) and then relate &'X bool and &'3 bool, which should eventually result in a region error. but something's not working there.

nikomatsakis (Feb 01 2019 at 15:15, on Zulip):

Ah, I see

nikomatsakis (Feb 01 2019 at 15:19, on Zulip):

I think the good news is that it is one of the two bugs I found; the bad news is that my fix was wrong (but I can fix it, so that's not really bad). But let's see.

nikomatsakis (Feb 01 2019 at 15:27, on Zulip):

Huzzah:

> rustc +rust-2-stage1 ~/tmp/57843.rs
error[E0308]: mismatched types
  --> /home/nmatsakis/tmp/57843.rs:15:9
   |
15 |     Foo(Box::new(|_| ()));
   |         ^^^^^^^^^^^^^^^^ one type is more general than the other
   |
   = note: expected type `std::ops::FnOnce<(&'a bool,)>`
              found type `std::ops::FnOnce<(&bool,)>`
nikomatsakis (Feb 01 2019 at 15:27, on Zulip):

now I just need to rebase over @lqd's PR so I can get decent diagnostics :)

lqd (Feb 01 2019 at 15:30, on Zulip):

(unfortunately I think this latter diagnostic might be #57875 with PolyTraitRefs ?)

nikomatsakis (Feb 01 2019 at 15:30, on Zulip):

heh, probably true

nikomatsakis (Feb 01 2019 at 15:38, on Zulip):

Incidentally, @pnkfelix, this points to something of an efficiency shortcoming in the way I integrated universe support. We adopted this simplification where the inference table always has a single "current universe" counter that is incremented each time we enter a placeholder -- but never decremented again. This avoided the need to carry universes around in our obligations and other things, but it means that we sometimes wind up with inference variables that logically could be in the root universe being assigned to some higher universe (which is an extension of the root universe, so it all works out).

I think that as the chalk transition continues, we could do better here.

All of this is making me think that we really need to do some "let's talk about the design of universes in detail" sort of thing. (Next rustc lecture series, maybe?)

Last update: Nov 12 2019 at 16:20UTC