Stream: t-compiler/wg-nll

Topic: #54593-impl-Trait-ICE


eddyb (Sep 26 2018 at 10:43, on Zulip):

what landed in the last nightly? it broke at least one thing https://travis-ci.com/lykenware/gll/builds/85984866

could it be this? https://github.com/rust-lang/rust/pull/53438

eddyb (Sep 26 2018 at 10:48, on Zulip):

cc @nikomatsakis @Matthew Jasper

eddyb (Sep 26 2018 at 10:48, on Zulip):

(I also asked in #compiler on Discord)

Matthew Jasper (Sep 26 2018 at 11:14, on Zulip):

https://github.com/rust-lang/rust/pull/53542 looks more likely, if it's in this nightly. Although I wrote some of the code in that PR, so I guess it's on me either way.

eddyb (Sep 26 2018 at 11:26, on Zulip):

yeah I was came here to paste https://github.com/rust-lang/rust/pull/53542 heh

nikomatsakis (Sep 26 2018 at 16:17, on Zulip):

@eddyb any change you can file an issue (even better if you can reproduce some other way...)

eddyb (Sep 26 2018 at 16:17, on Zulip):

I can uhh try to reduce it?

nikomatsakis (Sep 26 2018 at 16:24, on Zulip):

well that always helps but I'd like to have an issue just to help me stay organized

eddyb (Sep 26 2018 at 16:25, on Zulip):

yes, but, all I have to point to is that travis build failure, I wanted something more substantive

eddyb (Sep 26 2018 at 16:50, on Zulip):

reduced to one-liner! #![feature(nll)] fn foo() -> impl FnOnce() { || (|| foo())()() }

eddyb (Sep 26 2018 at 16:52, on Zulip):

oh wooow

eddyb (Sep 26 2018 at 17:01, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/issues/54593

Tyler Laing (Sep 26 2018 at 17:05, on Zulip):

0.0 whyyyyyyyy

eddyb (Sep 26 2018 at 17:08, on Zulip):

I just edited it a bit - you don't need impl FnOnce(), any impl Trait will do

nikomatsakis (Sep 26 2018 at 17:09, on Zulip):

@eddyb thanks!

nikomatsakis (Sep 26 2018 at 17:09, on Zulip):

that's awesome

eddyb (Sep 26 2018 at 17:11, on Zulip):

breaking GLL is not awesome :P

eddyb (Sep 26 2018 at 17:11, on Zulip):

(IMO the PR was a bit rushed)

eddyb (Sep 26 2018 at 17:12, on Zulip):

at least we have older nightlies, but we won't be able to land PRs without messing with .travis.yml ... somehow

nikomatsakis (Sep 26 2018 at 17:12, on Zulip):

let me see if I can see what the problem is...

nikomatsakis (Sep 26 2018 at 17:12, on Zulip):

having a test case is mucho helpful

eddyb (Sep 26 2018 at 17:13, on Zulip):

yeah I'm surprised it reduced!

eddyb (Sep 26 2018 at 17:13, on Zulip):

this is... right smack in the middle of a system for composing "thunks" acting on continuations

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

but it's more about closures somehow inheriting some ability regarding -> impl Trait of their parents

nikomatsakis (Sep 26 2018 at 17:15, on Zulip):

yes

nikomatsakis (Sep 26 2018 at 17:17, on Zulip):

hmm. I just tested your example with my build from https://github.com/rust-lang/rust/pull/54453 and it passes.

nikomatsakis (Sep 26 2018 at 17:17, on Zulip):

this build is fully rebased...

nikomatsakis (Sep 26 2018 at 17:17, on Zulip):

though maybe I need to rebuild since rebasing

nikomatsakis (Sep 26 2018 at 17:17, on Zulip):

I think I only ran x.py check

nikomatsakis (Sep 26 2018 at 17:17, on Zulip):

so maybe the executable itself is from before that PR landed

nikomatsakis (Sep 26 2018 at 17:18, on Zulip):

/me rebuilds

nikomatsakis (Sep 26 2018 at 17:29, on Zulip):

(yep, I get the ICE now)

eddyb (Sep 26 2018 at 17:46, on Zulip):

@nikomatsakis you might be able to just revert the NLL changes in the PR. it would break some tests but I suspect it will fix my testcase

nikomatsakis (Sep 26 2018 at 17:50, on Zulip):

well

nikomatsakis (Sep 26 2018 at 17:50, on Zulip):

I see why it's failing

nikomatsakis (Sep 26 2018 at 17:50, on Zulip):

but I don't quite know how the code went wrong yet

nikomatsakis (Sep 26 2018 at 17:52, on Zulip):

woah, I just read this code

nikomatsakis (Sep 26 2018 at 17:53, on Zulip):
        if let Err(terr) = self.sub_types(sub, sup, locations, category) {
            if let TyKind::Opaque(..) = sup.sty {
                return self.eq_opaque_type_and_type(sub, sup, locations, category);
            } else {
                return Err(terr);
            }
        }
        Ok(())
nikomatsakis (Sep 26 2018 at 17:53, on Zulip):

that doesn't seem quite right

nikomatsakis (Sep 26 2018 at 17:53, on Zulip):

(as in, I don't like the idea of "try to do subtyping and if it fails do something else")

nikomatsakis (Sep 26 2018 at 17:53, on Zulip):

hmm

nikomatsakis (Sep 26 2018 at 17:54, on Zulip):

it probably also doesn't make sense to do type equality in the fallback case

nikomatsakis (Sep 26 2018 at 17:54, on Zulip):

well, let me see if I can figure out the problem at hand, then we can revisit these points...

nikomatsakis (Sep 26 2018 at 18:18, on Zulip):

ok, I see the bug I think. We are instantiating the opaque types defined on the "parent def-id" (so, the enclosing function). We used to instantiate the opaque types defined on the closure itself (none)

nikomatsakis (Sep 26 2018 at 18:19, on Zulip):

I'm not really clear on why we are doing that though

nikomatsakis (Sep 26 2018 at 18:21, on Zulip):

@alercah you around? I'm trying to fix the problem that @eddyb hit in https://github.com/rust-lang/rust/pull/53542/ ... I am wondering why in eq_opaque_type_and_type you are using the parent_def_id when invoking instantiate_opaque_types (instead of self.mir_def_id)...

nikomatsakis (Sep 26 2018 at 18:21, on Zulip):

well, I guess i'll try changing it and see what happens :)

eddyb (Sep 26 2018 at 18:22, on Zulip):

@nikomatsakis did you meant to ping @Alexander Regueiro ?

nikomatsakis (Sep 26 2018 at 18:22, on Zulip):

maybe?

nikomatsakis (Sep 26 2018 at 18:22, on Zulip):

did I misread the author of the PR? :)

eddyb (Sep 26 2018 at 18:22, on Zulip):

(who is easier to find on Discord)

nikomatsakis (Sep 26 2018 at 18:22, on Zulip):

it appears I did

nikomatsakis (Sep 26 2018 at 18:22, on Zulip):

sorry @alercah

nikomatsakis (Sep 26 2018 at 18:23, on Zulip):

ok well I can confirm it fixes this example

nikomatsakis (Sep 26 2018 at 18:23, on Zulip):

let me see if anything..else breaks...

nikomatsakis (Sep 26 2018 at 18:37, on Zulip):

hmm, still seems to pass all tests...

Tyler Laing (Sep 26 2018 at 18:39, on Zulip):

Add a new test(s)?

nikomatsakis (Sep 26 2018 at 18:40, on Zulip):

no, ok, I found some tests that break, which is good (tests added by the PR in question)

nikomatsakis (Sep 26 2018 at 18:40, on Zulip):

they were just under run-pass

nikomatsakis (Sep 26 2018 at 18:40, on Zulip):

can't wait to get rid of those directories

nikomatsakis (Sep 26 2018 at 18:41, on Zulip):

I wonder how this is being desugared

nikomatsakis (Sep 26 2018 at 18:41, on Zulip):

I guess I have to read a bit more closely

Last update: Nov 21 2019 at 23:25UTC