Stream: t-compiler/wg-rfc-2229

Topic: test failures


blitzerr (Mar 07 2019 at 16:12, on Zulip):

@nikomatsakis based on our discussion we want to inspect the self type here.

blitzerr (Mar 07 2019 at 16:13, on Zulip):

But when I print the debug logs
debug!("vtable_auto_impl: nested={:?}", nested);
doesn't get printed. So, are we sure that's the code path that is executed in the test ?

nikomatsakis (Mar 07 2019 at 17:13, on Zulip):

@blitzerr good question =) remind me what the test is, again?

nikomatsakis (Mar 07 2019 at 17:13, on Zulip):

I could be wrong of course

blitzerr (Mar 07 2019 at 17:15, on Zulip):

@nikomatsakis
https://github.com/rust-lang/rust/blob/master/src/test/ui/not-clone-closure.rs

blitzerr (Mar 07 2019 at 17:16, on Zulip):

I was tracking the call stack here

blitzerr (Mar 07 2019 at 17:17, on Zulip):

That is in progress ..

nikomatsakis (Mar 07 2019 at 17:58, on Zulip):

Ah, @blitzerr, yeah, that's a slightly different path

nikomatsakis (Mar 07 2019 at 17:58, on Zulip):

same root problem though

nikomatsakis (Mar 07 2019 at 17:59, on Zulip):

But the trait here is Clone, not an auto trait

nikomatsakis (Mar 07 2019 at 17:59, on Zulip):

the "constituent types" callback in that case is here

nikomatsakis (Mar 07 2019 at 18:00, on Zulip):

and we wind up assembling the candidates here

nikomatsakis (Mar 07 2019 at 18:00, on Zulip):

so we will wind up doing the "confirmation step" here

nikomatsakis (Mar 07 2019 at 18:01, on Zulip):

and hence the cause that we want to alter is on this line

blitzerr (Mar 08 2019 at 00:38, on Zulip):

@nikomatsakis Got it, thanks a lot. I will get on this today.

blitzerr (Mar 09 2019 at 18:54, on Zulip):

@nikomatsakis is there a reason we should not unwrap the tuple here rather than return a (vec![], bool,) tuple and then based on that call the obligation.cause.clone() as mentioned here under the bullet point with green background which was the cleanest as we discussed ?

blitzerr (Mar 10 2019 at 03:57, on Zulip):

In terms of code I replaced

if is_copy_trait || is_clone_trait {
    Where(ty::Binder::bind(vec![substs.upvar_tuple_ty(def_id, self.tcx())])))

with

if is_copy_trait || is_clone_trait {
    let upvars = match substs.upvar_tuple_ty(def_id, self.tcx()) {
    ty::Tuple(tys) => tys,
    ref t => bug!("A tuple was expected but found : {:?}", t),
};
Where(ty::Binder::bind(upvars.to_vec()))

in fn copy_clone_conditions() under the ty::Closure(def_id, substs) => { match condition

blitzerr (Mar 10 2019 at 04:04, on Zulip):

and I get this error

error[E0308]: mismatched types============================>        ] 107/125
    --> src/librustc/traits/select.rs:2443:25
     |
2443 |                         ty::Tuple(tys) => tys,
     |                         ^^^^^^^^^^^^^^ expected struct `ty::TyS`, found enum `ty::sty::TyKind`
     |
     = note: expected type `ty::TyS<'_>`
                found type `ty::sty::TyKind<'_>`

Want to make sure this is okay before I proceed further. File from the branch without the recent changes here

blitzerr (Mar 10 2019 at 04:12, on Zulip):

Otherwise as we discussed, we will have to change

enum BuiltinImplConditions<'tcx> {
 314     /// The impl is conditional on T1,T2,.. : Trait
 315     Where(ty::Binder<Vec<Ty<'tcx>>>),
 316     /// There is no built-in impl. There may be some other
 317     /// candidate (a where-clause or user-defined impl).
 318     None,
 319     /// It is unknown whether there is an impl.
 320     Ambiguous,
 321 }

to

Where(ty::Binder<(Vec<Ty<'tcx>>, bool)>),

Which one do you agree with more ?

ange (Mar 10 2019 at 22:20, on Zulip):

and I get this error

@blitzerr not sure if this helps, but the error seems to be because match substs.upvar_tuple_ty(def_id, self.tcx()) should be match substs.upvar_tuple_ty(def_id, self.tcx()).sty instead?

blitzerr (Mar 11 2019 at 01:08, on Zulip):

Thanks @ange that definitely solves the error.

blitzerr (Mar 11 2019 at 01:30, on Zulip):

Was I reading it wrong - the error message

note: expected type `ty::TyS<'_>`
                found type `ty::sty::TyKind<'_>`

should be

note: expected type `ty::sty::TyKind<'_>`
                found type `ty::TyS<'_>`

?

blitzerr (Mar 11 2019 at 03:49, on Zulip):

@nikomatsakis Apparently this does not solve the problem.So I will get back to returning the (vec, bool) and give that a shot

ange (Mar 11 2019 at 14:29, on Zulip):

@blitzerr can one link to a specific line of the Roadmap document somehow?

blitzerr (Mar 11 2019 at 17:04, on Zulip):

@ange Unfortunately, links can only be generated for headings (H1/H2) not for a line of text

ange (Mar 11 2019 at 17:11, on Zulip):

ok. I was going to ask re: the two options for fixing the ui change in not-clone-closure.rs. Would adding a Synthetic flag to the Ty be viable alternative? Returning an extra bool from constituent_types_for_ty seems a bit ugly. Special-casing the closure-tuple case in confirm_auto_impl_candidate feels a bit awkward. That said, I doubt there'd be any more synthetic types in the future, so there's that :-)

ange (Mar 11 2019 at 17:11, on Zulip):

@blitzerr just a question for the next time the subject comes up

blitzerr (Mar 11 2019 at 17:17, on Zulip):

Ty is a central datastructure and putting things in it, that's not required for the entire duration is a waste. In our case, it needs to span across a couple function calls.

ange (Mar 11 2019 at 17:21, on Zulip):

I agree that if there's no other applicable use, it makes sense to keep the change localized. space-wise there shouldn't be an issue (only half of the flag bits are currently used), but there's the mental load to think of, true

blitzerr (Mar 11 2019 at 17:25, on Zulip):

@ange ya, that's the idea.

blitzerr (Mar 12 2019 at 01:31, on Zulip):

@nikomatsakis After implementing what we discussed for both confirm_auto_impl_candidate and confirm_builtin_candidate, it solved the two tests I was eyeing (ui/not-clone-closure.rs and ui/closures/closure-move-sync.rs) but overall it has led to an increase in failures. For all the failures please look here. The test failures are handled by the last two commits. The compilation for changes in select.rs is extremely slow (takes an hour on my system), so the progress is slow.

nikomatsakis (Mar 13 2019 at 18:22, on Zulip):

@blitzerr sorry I got a bit overwhelmed as usual, trying to catch up here

nikomatsakis (Mar 13 2019 at 18:23, on Zulip):

I requested access to that paper doc btw

blitzerr (Mar 14 2019 at 00:22, on Zulip):

@nikomatsakis
You should have access now. Sorry about that.

nikomatsakis (Mar 20 2019 at 19:21, on Zulip):

@blitzerr where can I find your code?

nikomatsakis (Mar 20 2019 at 19:28, on Zulip):

Great, I'm doing a local build now

nikomatsakis (Mar 20 2019 at 19:40, on Zulip):

@blitzerr so I started doing some investigation. The first test case I looked at, src/test/ui/closure-move-sync.rs -- your branch produces the same output as nightly

nikomatsakis (Mar 20 2019 at 19:41, on Zulip):

oh, maybe I misunderstood the doc

nikomatsakis (Mar 20 2019 at 19:42, on Zulip):

OK, so, maybe I'll leave some comments in the doc

nikomatsakis (Mar 20 2019 at 19:52, on Zulip):

ok, it's not duplicate prevention

nikomatsakis (Mar 20 2019 at 19:52, on Zulip):

if I edit the not-send-sync.rs test case like so:

#![feature(generators)]

use std::cell::Cell;

fn main() {
    fn assert_sync<T: Sync>(_: T) {}
    fn assert_send<T: Send>(_: T) {}

    assert_sync(|| {
        //~^ ERROR: E0277
        let a = Cell::new(2);
        yield;
    });

//    let a = Cell::new(2);
//    assert_send(|| {
//        //~^ ERROR: E0277
//        drop(&a);
//        yield;
//    });
}

then we no longer get errors :(

nikomatsakis (Mar 20 2019 at 19:53, on Zulip):

curious

nikomatsakis (Mar 20 2019 at 19:58, on Zulip):

also curious, running with RUST_LOG=rustc::traits winds up with an ICE :(

blitzerr (Mar 20 2019 at 20:48, on Zulip):

I ran it with RUST_LOG=rustc::traits::select and it was fine. I will try that and see.

nikomatsakis (Mar 22 2019 at 16:05, on Zulip):

On this topic, @blitzerr --

nikomatsakis (Mar 22 2019 at 16:05, on Zulip):

what I wanted to do but didn't find time to do

nikomatsakis (Mar 22 2019 at 16:05, on Zulip):

well, first of all, I presume you are able to reproduce that the above test case (with the second case commented out) passes, right?

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

so what I had started to do was reverting some of your commits one by one

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

basically trying to bisect where the failure came from

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

e.g., you changed things to return a tuple

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

that is, the (vec, bool) tuple

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

I changed all the cases to have the bool be "false"

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

and the test still was (incorrectly) passing

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

so yeah I would suggest you do some of this "quasi-bisection"

nikomatsakis (Mar 22 2019 at 16:06, on Zulip):

since I'm a bit confused as to what is going on

nikomatsakis (Mar 22 2019 at 16:07, on Zulip):

this btw is a case where using the -i really helps

nikomatsakis (Mar 22 2019 at 16:07, on Zulip):

because rebuilding the compiler can be (relatively) swift

nikomatsakis (Mar 22 2019 at 16:07, on Zulip):

if you're having trouble with that, we can discuss :)

blitzerr (Mar 22 2019 at 16:09, on Zulip):

Sure @nikomatsakis I will do that dissection. Haven't found time since my last comment on the paper doc.
I do use -i always. :grinning:

nikomatsakis (Mar 22 2019 at 16:09, on Zulip):

well, specifically I mean in conjunction with --keep-stage, as well

blitzerr (Mar 22 2019 at 16:09, on Zulip):

This branch is old and so didn't need to sync from mainline

blitzerr (Mar 22 2019 at 16:10, on Zulip):

Keep stage is a test specific flag right ?

blitzerr (Mar 22 2019 at 16:10, on Zulip):

By that I mean you use it as ./x.py test ...

nikomatsakis (Mar 22 2019 at 16:16, on Zulip):

no, keep-stage is a super hack

nikomatsakis (Mar 22 2019 at 16:16, on Zulip):

that avoids rebuilding libraries

nikomatsakis (Mar 22 2019 at 16:16, on Zulip):

which can have a big impact on total turnaround time

nikomatsakis (Mar 22 2019 at 16:16, on Zulip):

there are two workflows

nikomatsakis (Mar 22 2019 at 16:18, on Zulip):

one workflow is documented here, but there's another option that may be better

nikomatsakis (Mar 22 2019 at 16:18, on Zulip):

I should open a PR for that in rustc-guide

nikomatsakis (Mar 22 2019 at 16:18, on Zulip):

the basic idea would be to do

./x.py build -i src/libstd # first build
./x.py build -i --keep-stage 1 src/libstd # later builds
nikomatsakis (Mar 22 2019 at 16:18, on Zulip):

then you can just run the test by running rustc by hand on the test file

nikomatsakis (Mar 22 2019 at 16:18, on Zulip):

that's what I do anyway

nikomatsakis (Mar 22 2019 at 16:19, on Zulip):

I don't really use the test runner when I'm debugging an individual test

nikomatsakis (Mar 22 2019 at 16:19, on Zulip):

I should open a PR for that in rustc-guide

ps, if you wanted to open a PR amending the section that talks about using --stage 1 to talk about that workflow, i'd be appreciative :)

nikomatsakis (Mar 22 2019 at 16:19, on Zulip):

not sure if it requires detailed edits to surrounding text though

nikomatsakis (Mar 22 2019 at 16:19, on Zulip):

the main thing is that the --stage 1 flow can be a bit faster sometimes, particularly on the initial build, but it also sometimes fails in weird ways that the one above would not

nikomatsakis (Mar 22 2019 at 16:19, on Zulip):

so probably better to just recommend the second one

blitzerr (Mar 22 2019 at 16:23, on Zulip):

@nikomatsakis I will open a PR

blitzerr (Mar 26 2019 at 04:51, on Zulip):

@nikomatsakis In my branch, there two commits that were exclusively for fixing the test errors. I changed them both to return false in all the cases and the not-send-sync.rs still fails. I will go further down the rabbit hole tomorrow and figure out the provenance of the error.

ange (Apr 03 2019 at 11:06, on Zulip):

@blitzerr fwiw, I haven't forgotten about this. did a loop to get the test results for every commit so I can read up and get some idea of what started failing when, hopefully on friday

blitzerr (Apr 04 2019 at 23:21, on Zulip):

@ange thanks a lot. Let me know if anything is not clear in the doc or commits

ange (Apr 06 2019 at 20:36, on Zulip):

So, re: the ui/generator/not-send-sync.rs testcase, this is the summary of the differences in the compiler output for the commits in this branch:

for sha in 75a369c5b1 e9faf40169 7fa36f4f63 8b32d92153 61a1d2c75d 59a6a0c400 8568565465 136a711012 4a02c5b487 64d0145d19 662e0a8396 7567a314b6; do echo "--- $sha ---"; grep  -e 'required because.*`(' -e '\[E0277' not-send-sync-results/${sha}.out; done
--- 75a369c5b1 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
--- e9faf40169 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
--- 7fa36f4f63 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 8b32d92153 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 61a1d2c75d ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 59a6a0c400 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 8568565465 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 136a711012 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 4a02c5b487 ---
--- 64d0145d19 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 662e0a8396 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
   = note: required because it appears within the type `(&std::cell::Cell<i32>,)`
--- 7567a314b6 ---
error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely

(4a02c5b487 is blank cause it failed to build)

ange (Apr 06 2019 at 20:40, on Zulip):

unfortunately I'm visiting friends in berlin and traveling back home tomorrow, so couldn't really get into the actual error. I did capture the debug output when compiling the testcase though, so normalizing and diffing the debug logs might give some good clues as to where unexpected divergences in the output arise :fingers_crossed:

ange (Apr 09 2019 at 17:49, on Zulip):

I should add that I had accidentally built in a rust source tree without the correct config.toml, so got only minimal debugging output. tried to rebuild again but kept running into seemingly unrelated segfaults. threw out llvm (only other variable and also where the segfault was manifesting) and now waiting for it to hopefully go through...

ange (Apr 11 2019 at 21:51, on Zulip):

FWIW, the first differences that seem significant in the debug output, when compiling ui/generator/not-send-sync.rs with e9faf40169 vs 7567a314b6 are

rustc::traits::select: builtin_bound: nested=Binder(([], false))
rustc::traits::select: builtin_bound: nested=Binder(([{integer}], false))
ena::unify: TyVidEqKey: created new key: TyVidEqKey { vid: _#13t, phantom: PhantomData }
ena::unify: TyVid: created new key: _#13t
rustc::infer::type_variable: new_var(index=_#13t, diverging=false, origin=ClosureSynthetic(src/test/ui/generator/not-send-sync.rs:9:17: 13:6)

The first two lines are simply the addition of the boolean, the last three lines are completely absent in the earlier commit (for which the testcase passes).

This results in GeneratorSubsts { substs: [(), (), _, _] } that have one extra , _, this in turn results in an extra obligation predicate. This of course spills over the rest of the diff, until we get to the unification, where (surprisingly?) there's one less call into ena in the later commit (the one that has the extra _). Ah, this seems to be balanced by an extra call to ena that appears much later. Then at ~85% of the diff I'm losing track of the differences, until we eventually get the shared error message for the second closure. Recall, with the later commit, we're losing the error message for the first closure. Then the earlier (correct) commit enters process_obligations 3 more times, does some unification and spits out the error message. This could be a diff artifact though.

ange (Apr 11 2019 at 21:52, on Zulip):

This all seems in order to me, AFAICT with the addition of the tuple there should be one extra type variable. Dunno if @nikomatsakis sees any smoking guns in the above cc: @blitzerr

nikomatsakis (Apr 15 2019 at 19:42, on Zulip):

hmm

nikomatsakis (Apr 15 2019 at 19:42, on Zulip):

I guess I gotta re-test locally

ange (Apr 15 2019 at 20:12, on Zulip):

To be clear, the issue is there, it's just that the differences I could track in the debug output seem to be accounted for by the change introduced in the patch

nikomatsakis (Apr 30 2019 at 18:23, on Zulip):

Ok, I'm trying to remember the status now :)

nikomatsakis (Apr 30 2019 at 18:23, on Zulip):

Ah, right, I vaguely recall -- we added some logic to suppress some duplicate lines

nikomatsakis (Apr 30 2019 at 18:23, on Zulip):

and we wound up masking errors intead

nikomatsakis (Apr 30 2019 at 18:25, on Zulip):

although from what I can tell

nikomatsakis (Apr 30 2019 at 18:25, on Zulip):

the test maybe just started failing due to th change itself

nikomatsakis (Apr 30 2019 at 18:25, on Zulip):

Ah, right, I vaguely recall -- we added some logic to suppress some duplicate lines

and not so much as a result of this lgoic

blitzerr (Apr 30 2019 at 18:26, on Zulip):

I remember I had a doc. Let me find that. It had the failures

blitzerr (Apr 30 2019 at 18:26, on Zulip):

https://paper.dropbox.com/doc/UI-tests-failing-X96kJW6agXuvx36GPxmcg

blitzerr (Apr 30 2019 at 18:27, on Zulip):

That is possible.

blitzerr (Apr 30 2019 at 18:27, on Zulip):

We never ran the tests with the incremental changes

nikomatsakis (Apr 30 2019 at 18:28, on Zulip):

I'm digging in

nikomatsakis (Apr 30 2019 at 20:15, on Zulip):

@blitzerr I may have found the problem, testing a fix now

nikomatsakis (Apr 30 2019 at 20:18, on Zulip):

the tl;dr is that, in your branch, you have lost the "witness" when enumerating the constituent types for generators:

https://github.com/rust-lang/rust/blob/5b7baa53c91d7c33b925fc8aec553e3521548a07/src/librustc/traits/select.rs#L2601

nikomatsakis (Apr 30 2019 at 20:18, on Zulip):

(that's the master code, not your branch)

nikomatsakis (Apr 30 2019 at 20:19, on Zulip):

this witness, in particular, carries the types of local variables in the generator that are live across yields

nikomatsakis (Apr 30 2019 at 20:19, on Zulip):

which is exactly what this test is testing

blitzerr (Apr 30 2019 at 20:19, on Zulip):

:tada: :octopus: :crab:

nikomatsakis (Apr 30 2019 at 20:19, on Zulip):

building a branch locally now to see if this fixes the problem, but I strongly suspect it will

blitzerr (Apr 30 2019 at 20:23, on Zulip):

Ohh man !

blitzerr (Apr 30 2019 at 20:24, on Zulip):

You mean in one of my commits, I just removed it ?

nikomatsakis (Apr 30 2019 at 20:24, on Zulip):

yeah

nikomatsakis (Apr 30 2019 at 20:24, on Zulip):

copy-n-paste bug, basically :)

nikomatsakis (Apr 30 2019 at 20:24, on Zulip):

ok, it seems to work

blitzerr (Apr 30 2019 at 20:24, on Zulip):

lol

blitzerr (Apr 30 2019 at 20:24, on Zulip):

ok, it seems to work

That is great

nikomatsakis (Apr 30 2019 at 20:25, on Zulip):

I pushed a commit to your branch

nikomatsakis (Apr 30 2019 at 20:25, on Zulip):

I'm starting a full test run now, but it'll probably be a while :)

blitzerr (Apr 30 2019 at 20:30, on Zulip):

Based on missing the witness part, it should only fail generator tests right ?

blitzerr (Apr 30 2019 at 20:30, on Zulip):

But what about the closure tests ?

blitzerr (Apr 30 2019 at 20:31, on Zulip):

It might be a good idea to try some of the tests mentioned in here before running the full ui suite ? What do you think ?

blitzerr (Apr 30 2019 at 20:33, on Zulip):

@nikomatsakis :point_up:

nikomatsakis (Apr 30 2019 at 20:40, on Zulip):

there are still plenty of errors in the ui test suite

nikomatsakis (Apr 30 2019 at 20:41, on Zulip):

I'm running with --bless and will look in a bit more detail

nikomatsakis (Apr 30 2019 at 20:41, on Zulip):

the good news is that, with --bless, the tests
pass

nikomatsakis (Apr 30 2019 at 20:41, on Zulip):

that is, we are still generating the same set of errors we did before

nikomatsakis (Apr 30 2019 at 20:42, on Zulip):

but it does seem like we're not quite getting the 'backtrace" data right

blitzerr (Apr 30 2019 at 20:42, on Zulip):

so doesn't bless changes the output accordingly ?

blitzerr (Apr 30 2019 at 20:43, on Zulip):

Then the tests will pass unless we introduced a crash ? Is that understanding right ?

nikomatsakis (Apr 30 2019 at 21:11, on Zulip):

so doesn't bless changes the output accordingly ?

bless updates the reference files, but not the //~ ERROR anotations

nikomatsakis (Apr 30 2019 at 21:11, on Zulip):

so you have to still get the same number of errors, but the text can change

nikomatsakis (Apr 30 2019 at 21:11, on Zulip):

I (personaly) always run with --bless, but then I just use git diff to compare output

blitzerr (Apr 30 2019 at 21:16, on Zulip):

I see. Makes sense.

blitzerr (Apr 30 2019 at 21:16, on Zulip):

It's always helpful to know people's workflow

nikomatsakis (Apr 30 2019 at 21:16, on Zulip):

this is partly because I the emacs integration w/ git is really great

nikomatsakis (Apr 30 2019 at 21:17, on Zulip):

so it's easy to browse the list of files w/ diffs etc

blitzerr (Apr 30 2019 at 21:19, on Zulip):

Waaatt ? You don't use vs-code with rust analyzer ? :smiley:

nikomatsakis (Apr 30 2019 at 21:23, on Zulip):

I have interated r-a into emacs, at least on my local machine ... still need to do that on my linux desktop that I use for rustc...

blitzerr (Apr 30 2019 at 21:25, on Zulip):

They didn't have integration steps for vim, so I just use bare-bones vim for rustc

blitzerr (May 01 2019 at 14:28, on Zulip):

@nikomatsakis
Curious if the entire suite of test passed with your changes ?

nikomatsakis (May 01 2019 at 16:13, on Zulip):

no, there are plenty of failures, I've not had time to dig into 'em

blitzerr (May 12 2019 at 03:09, on Zulip):

I pushed a change as you suggested that fixed a few test cases. We still have 68 failed tests. They have this symptoms.

blitzerr (May 12 2019 at 03:09, on Zulip):
---- [ui] ui/feature-gates/feature-gate-trivial_bounds.rs stdout ----
diff of stderr:

107 LL | | }
108    | |_^ doesn't have a size known at compile-time
109    |
-      = help: within `Dst<(dyn A + 'static)>`, the trait `std::marker::Sized` is not implemented for `(dyn A + 'static)`
+      = help: the trait `std::marker::Sized` is not implemented for `(dyn A + 'static)`
111    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
-      = note: required because it appears within the type `Dst<(dyn A + 'static)>`
113    = help: see issue #48214
114    = help: add #![feature(trivial_bounds)] to the crate attributes to enable
115
blitzerr (May 12 2019 at 03:10, on Zulip):
---- [ui] ui/unsized-locals/issue-50940-with-feature.rs stdout ----
diff of stderr:

4   LL |     A as fn(str) -> A<str>;
5      |     ^ doesn't have a size known at compile-time
6      |
-      = help: within `main::A<str>`, the trait `std::marker::Sized` is not implemented for `str`
+      = help: the trait `std::marker::Sized` is not implemented for `str`
8      = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
-      = note: required because it appears within the type `main::A<str>`
10     = note: the return type of a function must have a statically known size
11
12  error: aborting due to previous error
blitzerr (May 12 2019 at 03:11, on Zulip):

Those two are similar. Some of the other ones are

blitzerr (May 12 2019 at 03:11, on Zulip):
---- [ui] ui/suggestions/path-by-value.rs stdout ----
diff of stderr:

2     --> $DIR/path-by-value.rs:3:6
3      |
4   LL | fn f(p: Path) { }
-      |      ^ borrow the `Path` instead
+      |      ^ doesn't have a size known at compile-time
6      |
-      = help: within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
+      = help: the trait `std::marker::Sized` is not implemented for `[u8]`
8      = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
-      = note: required because it appears within the type `std::path::Path`
10     = note: all local variables must have a statically known size
11     = help: unsized locals are gated as an unstable feature
12
blitzerr (May 12 2019 at 03:12, on Zulip):
---- [ui] ui/traits/trait-suggest-where-clause.rs stdout ----
diff of stderr:

15  LL |     mem::size_of::<Misc<U>>();
16     |     ^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
17     |
-      = help: within `Misc<U>`, the trait `std::marker::Sized` is not implemented for `U`
+      = help: the trait `std::marker::Sized` is not implemented for `U`
19     = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
20     = help: consider adding a `where U: std::marker::Sized` bound
-      = note: required because it appears within the type `Misc<U>`
22     = note: required by `std::mem::size_of`
23
24  error[E0277]: the trait bound `u64: std::convert::From<T>` is not satisfied
blitzerr (May 12 2019 at 03:12, on Zulip):

@nikomatsakis :point_of_information:

blitzerr (May 12 2019 at 18:31, on Zulip):

Tracking details here

blitzerr (May 13 2019 at 01:48, on Zulip):

I created another branch with all chanegs squashed into 1. Maybe that will be better to locate changes that caused errors.

blitzerr (May 13 2019 at 01:54, on Zulip):

Also I don't see ppaux.rs anymore in the rust-mainline. So merging will be interesting :slight_smile:

nikomatsakis (May 13 2019 at 17:41, on Zulip):

lol uh yeah I think @eddyb did some rewrite

nikomatsakis (May 13 2019 at 17:42, on Zulip):

sounds like progress, @blitzerr

nikomatsakis (May 13 2019 at 17:42, on Zulip):

these all seem to have to do with the "within" text... hmmm...

eddyb (May 13 2019 at 17:43, on Zulip):

@blitzerr most ppaux changes should be made against src/librustc/ty/print/pretty.rs now

blitzerr (May 13 2019 at 17:44, on Zulip):

Ya a lot of them have to do with within context

blitzerr (May 13 2019 at 17:45, on Zulip):

Thanks a lot @eddyb . Will keep that in mind about ppaux. pretty sounds like a better name for it

blitzerr (May 13 2019 at 17:46, on Zulip):

@nikomatsakis Do you know where the within text comes from ?

blitzerr (May 13 2019 at 17:46, on Zulip):

Also, you can look at the squashed branch, it should give us a better coverage of actual change as we did, un-did and re-did a few changes

blitzerr (May 17 2019 at 16:12, on Zulip):

Hi @nikomatsakis
If you have some pointers on the next set of failures, then let me know. I plan on making some progress this weekend.

nikomatsakis (May 17 2019 at 17:22, on Zulip):

@blitzerr let me take a look

blitzerr (May 17 2019 at 17:28, on Zulip):

Thank you sir. :grinning:

nikomatsakis (May 17 2019 at 18:09, on Zulip):

@blitzerr wait, where did you push those commits?

nikomatsakis (May 17 2019 at 18:09, on Zulip):

I pulled from your branch but I still see 1000+ failures

nikomatsakis (May 17 2019 at 18:09, on Zulip):

oh, I see, d'oh

nikomatsakis (May 17 2019 at 18:11, on Zulip):

Ya a lot of them have to do with within context

you asked where that text comes from -- the answer is this line in the trait error-reporting code, I believe.

nikomatsakis (May 17 2019 at 18:22, on Zulip):

@blitzerr I...might have found your problem. Running a quick test now.

blitzerr (May 17 2019 at 19:42, on Zulip):

Awesome !

blitzerr (May 17 2019 at 19:42, on Zulip):

Thank you, thank you

nikomatsakis (May 17 2019 at 20:00, on Zulip):

it looks like I was correct

nikomatsakis (May 17 2019 at 20:00, on Zulip):

but the bug was in 2 places

nikomatsakis (May 17 2019 at 20:01, on Zulip):

re-running the test at the moment

nikomatsakis (May 17 2019 at 20:01, on Zulip):
modified   src/librustc/traits/select.rs
@@ -2770,7 +2770,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 ),
             };

-            let cause = if is_closure_tuple {
+            let cause = if !is_closure_tuple {
                 obligation.derived_cause(BuiltinDerivedObligation)
             } else {
                 obligation.cause.clone()
@@ -2829,7 +2829,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     ) -> VtableAutoImplData<PredicateObligation<'tcx>> {
         debug!("vtable_auto_impl: nested={:?}", nested);

-        let cause = if is_upvar_tuple_ty {
+        let cause = if !is_upvar_tuple_ty {
             obligation.derived_cause(BuiltinDerivedObligation)
         } else {
             obligation.cause.clone()
nikomatsakis (May 17 2019 at 20:20, on Zulip):

@blitzerr can you give me perm to write to that branch?

nikomatsakis (May 17 2019 at 20:20, on Zulip):

or else open a PR or something?

nikomatsakis (May 17 2019 at 20:21, on Zulip):

we appear to be down to 9 diffs

nikomatsakis (May 17 2019 at 20:21, on Zulip):

those 9 diffs I don't quite understand yet

nikomatsakis (May 17 2019 at 20:21, on Zulip):

oh I...think maybe I do

nikomatsakis (May 17 2019 at 20:22, on Zulip):

but I don't know how to fix it

blitzerr (May 17 2019 at 20:22, on Zulip):

You have pushed chnages to my branch before. Is it not working this time ?

blitzerr (May 17 2019 at 20:22, on Zulip):

*changes

blitzerr (May 17 2019 at 20:24, on Zulip):

@nikomatsakis Which branch are you trying to write to upvar-tuple or upvar-tys-squashed ?

nikomatsakis (May 17 2019 at 20:25, on Zulip):

upvar-tys-squashed

nikomatsakis (May 17 2019 at 20:25, on Zulip):

which would no longer be squashed :)

nikomatsakis (May 17 2019 at 20:25, on Zulip):

I can force push to upvar-tuple if you want

nikomatsakis (May 17 2019 at 20:25, on Zulip):

I think maybe you have an open PR for that?

blitzerr (May 17 2019 at 20:25, on Zulip):

Ya, maybe I will just create a new PR :slight_smile:

blitzerr (May 17 2019 at 20:27, on Zulip):

Do you prefer to have one squashed PR or incremental commits ? What would be easier for you to review ?
@nikomatsakis

nikomatsakis (May 17 2019 at 20:28, on Zulip):

mmm I mean commits are good unless they are revising one another

nikomatsakis (May 17 2019 at 20:28, on Zulip):

in cases like this what I usually do is to squash to 1 commit

nikomatsakis (May 17 2019 at 20:28, on Zulip):

and then maybe, just before opening the PR, try to break that 1 commit into steps if I can

nikomatsakis (May 17 2019 at 20:29, on Zulip):

really it doesn't matter that much in this case

blitzerr (May 17 2019 at 20:30, on Zulip):

The changes here were/are revising each other a lot :slight_smile:

blitzerr (May 17 2019 at 20:31, on Zulip):

Sounds good.

blitzerr (May 17 2019 at 20:32, on Zulip):

@nikomatsakis Let me know which branch you end up pushing and I will take them all and create one PR. If you have some idea for the remaining 9 that you would like me to explore, let me know.

nikomatsakis (May 17 2019 at 20:33, on Zulip):

@blitzerr for now I just pushed to upvar-tys-squashed on nikomatsakis/rust

nikomatsakis (May 17 2019 at 20:33, on Zulip):

those 9 failures are here:

nikomatsakis (May 17 2019 at 20:34, on Zulip):
modified   src/test/ui/closures/closure-move-sync.stderr
@@ -6,7 +6,7 @@ LL |     let t = thread::spawn(|| {
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6 recv:&std::sync::mpsc::Receiver<()>]`
+   = note: required because it appears within the type `(&std::sync::mpsc::Receiver<()>,)`
    = note: required by `std::thread::spawn`

 error[E0277]: `std::sync::mpsc::Sender<()>` cannot be shared between threads safely
@@ -17,7 +17,7 @@ LL |     thread::spawn(|| tx.send(()).unwrap());
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<()>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Sender<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42 tx:&std::sync::mpsc::Sender<()>]`
+   = note: required because it appears within the type `(&std::sync::mpsc::Sender<()>,)`
    = note: required by `std::thread::spawn`

 error: aborting due to 2 previous errors
nikomatsakis (May 17 2019 at 20:34, on Zulip):

that's an example

nikomatsakis (May 17 2019 at 20:34, on Zulip):

I think what is happening is that the "causal chain" is getting messed up

nikomatsakis (May 17 2019 at 20:34, on Zulip):

in particular, before you inserted those booleans, we used to have a chain like:

nikomatsakis (May 17 2019 at 20:35, on Zulip):
nikomatsakis (May 17 2019 at 20:35, on Zulip):

I think that, at least sometimes, we are now getting

and losing the closure type

nikomatsakis (May 17 2019 at 20:35, on Zulip):

I don't quite understand why

nikomatsakis (May 17 2019 at 20:36, on Zulip):

I can maybe poke at a bit more, but if I had to guess, I would guess that some code in select.rs is taking the "self type", idk

nikomatsakis (May 17 2019 at 20:36, on Zulip):

er, sorry, building a chain from the self type (which is a tuple)

blitzerr (May 17 2019 at 20:36, on Zulip):

I see.

nikomatsakis (May 17 2019 at 20:38, on Zulip):

there is one failure that doesn't fit that template

nikomatsakis (May 17 2019 at 20:38, on Zulip):
modified   src/test/ui/mismatched_types/issue-36053-2.stderr
@@ -1,3 +1,11 @@
+error[E0631]: type mismatch in closure arguments
+  --> $DIR/issue-36053-2.rs:7:32
+   |
+LL |     once::<&str>("str").fuse().filter(|a: &str| true).count();
+   |                                ^^^^^^ -------------- found signature of `for<'r> fn(&'r str) -> _`
+   |                                |
+   |                                expected signature of `for<'r> fn(&'r &str) -> _`
+
 error[E0599]: no method named `count` found for type `std::iter::Filter<std::iter::Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:53]>` in the current scope
   --> $DIR/issue-36053-2.rs:7:55
    |
@@ -8,14 +16,6 @@ LL |     once::<&str>("str").fuse().filter(|a: &str| true).count();
            `&mut std::iter::Filter<std::iter::Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:53]> : std::iter::Iterator`
            `std::iter::Filter<std::iter::Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:53]> : std::iter::Iterator`

-error[E0631]: type mismatch in closure arguments
-  --> $DIR/issue-36053-2.rs:7:32
-   |
-LL |     once::<&str>("str").fuse().filter(|a: &str| true).count();
-   |                                ^^^^^^ -------------- found signature of `for<'r> fn(&'r str) -> _`
-   |                                |
-   |                                expected signature of `for<'r> fn(&'r &str) -> _`
-
 error: aborting due to 2 previous errors

 Some errors occurred: E0599, E0631.
nikomatsakis (May 17 2019 at 20:38, on Zulip):

no idewhat's going on there

nikomatsakis (May 17 2019 at 20:38, on Zulip):

oh, I guess just the ordering of the errors changed

nikomatsakis (May 17 2019 at 20:38, on Zulip):

that's probably a non-issue

blitzerr (May 17 2019 at 20:40, on Zulip):

The read and the green looks exactly the same.

blitzerr (May 17 2019 at 20:40, on Zulip):

*red

blitzerr (May 17 2019 at 20:41, on Zulip):

So, looks like we re-ordered the error messages

nikomatsakis (May 17 2019 at 20:51, on Zulip):

yeah I think that test is probably fine

nikomatsakis (May 17 2019 at 20:51, on Zulip):

so that leaves the other 8

blitzerr (May 17 2019 at 20:53, on Zulip):

Okay, that a great reduction in numbers. Thank a lot

nikomatsakis (Jun 04 2019 at 18:11, on Zulip):

so we're down to 8

nikomatsakis (Jun 04 2019 at 18:11, on Zulip):

the question is

nikomatsakis (Jun 04 2019 at 18:11, on Zulip):

should we try to fix those first

nikomatsakis (Jun 04 2019 at 18:11, on Zulip):

or rebase first :)

nikomatsakis (Jun 04 2019 at 18:11, on Zulip):

were you able to investigate at all?

nikomatsakis (Jun 04 2019 at 18:12, on Zulip):

the errors I am seeing all seem to be that the closure type

nikomatsakis (Jun 04 2019 at 18:12, on Zulip):

is replaced with a tuple

nikomatsakis (Jun 04 2019 at 18:12, on Zulip):
modified   src/test/ui/closures/closure-move-sync.stderr
@@ -6,7 +6,7 @@ LL |     let t = thread::spawn(|| {
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6 recv:&std::sync::mpsc::Receiver<()>]`
+   = note: required because it appears within the type `(&std::sync::mpsc::Receiver<()>,)`
    = note: required by `std::thread::spawn`

 error[E0277]: `std::sync::mpsc::Sender<()>` cannot be shared between threads safely
@@ -17,7 +17,7 @@ LL |     thread::spawn(|| tx.send(()).unwrap());
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<()>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Sender<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42 tx:&std::sync::mpsc::Sender<()>]`
+   = note: required because it appears within the type `(&std::sync::mpsc::Sender<()>,)`
    = note: required by `std::thread::spawn`

 error: aborting due to 2 previous errors
nikomatsakis (Jun 04 2019 at 18:12, on Zulip):

stuff like that

nikomatsakis (Jun 04 2019 at 18:12, on Zulip):

seems pretty specific

blitzerr (Jun 04 2019 at 18:12, on Zulip):

Maybe fixing them first might be good idea then we know that the code is perfect based of off one commit.

nikomatsakis (Jun 04 2019 at 18:12, on Zulip):

yeah, can't hurt

blitzerr (Jun 04 2019 at 18:12, on Zulip):

Any failures after that are merge related issues.

blitzerr (Jun 04 2019 at 18:13, on Zulip):

My two cents

nikomatsakis (Jun 04 2019 at 18:16, on Zulip):

ok well I inserted some debug!

nikomatsakis (Jun 04 2019 at 18:16, on Zulip):

I'm going to try and take a look at this in the background as I do other things

blitzerr (Jun 04 2019 at 18:17, on Zulip):

So assuming that merge is all that is left, which I plan to finish this week if we get all the tests fixed, what should be the next steps ?

nikomatsakis (Jun 04 2019 at 18:24, on Zulip):

not sure, let's re-evaluate then

nikomatsakis (Jun 04 2019 at 18:25, on Zulip):

I think the freevars refactorings are still needed

nikomatsakis (Jun 04 2019 at 18:25, on Zulip):

I haven't quite figured out the impact of @eddyb's changes though

blitzerr (Jun 04 2019 at 18:25, on Zulip):

@csmoe was working on that.

nikomatsakis (Jun 04 2019 at 18:28, on Zulip):

well I think I see the bug, basically the change I suggested is kind of flawed

nikomatsakis (Jun 04 2019 at 18:29, on Zulip):

specifically this idea:

            let cause = if !is_closure_tuple {
                obligation.derived_cause(BuiltinDerivedObligation)
            } else {
                obligation.cause.clone()
            };
nikomatsakis (Jun 04 2019 at 18:30, on Zulip):

doesn't work out because of what derived_cause does

nikomatsakis (Jun 04 2019 at 18:30, on Zulip):

the idea was that we have a complete chain that looks like

nikomatsakis (Jun 04 2019 at 18:30, on Zulip):

and we want to remove the "upvar tuple" from the chain

nikomatsakis (Jun 04 2019 at 18:31, on Zulip):

ah hmm

nikomatsakis (Jun 04 2019 at 18:32, on Zulip):

so basically if you walk through the fix I suggested it winds up creating the wrong chain

nikomatsakis (Jun 04 2019 at 18:32, on Zulip):

I think a better fix might be something like this:

nikomatsakis (Jun 04 2019 at 18:49, on Zulip):

(sorry, had to do a quick meeting)

nikomatsakis (Jun 04 2019 at 18:49, on Zulip):

introduce a new variant ObligationCauseCode::HiddenDerivedObligation

nikomatsakis (Jun 04 2019 at 18:49, on Zulip):

and then have the code above be like

 let cause = if !is_closure_tuple {
                obligation.derived_cause(BuiltinDerivedObligation)
            } else {
                obligation.derived_cause(HiddenDerivedObligation)
            };
nikomatsakis (Jun 04 2019 at 18:51, on Zulip):

and then modify this code here to make a HiddenDerivedObligation just recurse to its parents and print nothing

nikomatsakis (Jun 04 2019 at 18:51, on Zulip):

@blitzerr does that :point_up: look like something you'd be game to try?

blitzerr (Jun 04 2019 at 19:37, on Zulip):

Sounds good @nikomatsakis . Will give that a shot. Thanks for looking into it and suggesting the fix.

blitzerr (Jun 08 2019 at 20:55, on Zulip):

@nikomatsakis
I made the change you suggested. But the problem is not solved.

blitzerr (Jun 08 2019 at 21:00, on Zulip):

Changes are here. What you said makes complete sense but recursing there is not giving us what we want. It still generates the note as stated by this line. Would if make sense to check for type here and then skip the err.note if it is upvar_tuple ? Its definitely not elegant.

nikomatsakis (Jun 10 2019 at 21:28, on Zulip):

@blitzerr I think you have to modify these lines in the same way. From:

        let cause = if !is_upvar_tuple_ty {
            obligation.derived_cause(BuiltinDerivedObligation)
        } else {
            obligation.cause.clone()
};

to

        let cause = if !is_upvar_tuple_ty {
            obligation.derived_cause(BuiltinDerivedObligation)
        } else {
            obligation.derived_cause(HiddenDerivedObligation)
};

also, we should probably rename the is_upvar_tupel_ty etc to hide_derived_obligation or something

blitzerr (Jun 11 2019 at 03:08, on Zulip):
---- [ui] ui/closures/closure-move-sync.rs stdout ----
diff of stderr:

6      |
7      = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
8      = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
-      = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6 recv:&std::sync::mpsc::Receiver<()>]`
+      = note: required because it appears within the type `(&std::sync::mpsc::Receiver<()>,)`
10     = note: required by `std::thread::spawn`
11
12  error[E0277]: `std::sync::mpsc::Sender<()>` cannot be shared between threads safely

17     |
18     = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<()>`
19     = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Sender<()>`
-      = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42 tx:&std::sync::mpsc::Sender<()>]`
+      = note: required because it appears within the type `(&std::sync::mpsc::Sender<()>,)`
21     = note: required by `std::thread::spawn`
22
23  error: aborting due to 2 previous errors
blitzerr (Jun 11 2019 at 03:09, on Zulip):
--- a/src/librustc/traits/select.rs
+++ b/src/librustc/traits/select.rs
@@ -2829,10 +2829,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     ) -> VtableAutoImplData<PredicateObligation<'tcx>> {
         debug!("vtable_auto_impl: nested={:?}", nested);

-        let cause = if !is_upvar_tuple_ty {
-            obligation.derived_cause(BuiltinDerivedObligation)
+        let cause = if is_upvar_tuple_ty {
+            obligation.derived_cause(HiddenDerivedObligation)
         } else {
-            obligation.cause.clone()
+            obligation.derived_cause(BuiltinDerivedObligation)
         };
         let mut obligations = self.collect_predicates_for_types(
             obligation.param_env,
blitzerr (Jun 11 2019 at 03:20, on Zulip):

@nikomatsakis Unfortunately it does not fixes it.

nikomatsakis (Jun 11 2019 at 17:42, on Zulip):

@blitzerr is this pushed to your branch?

blitzerr (Jun 11 2019 at 17:48, on Zulip):

@nikomatsakis This last bit of change that you suggested yesterday is not pushed to my branch.

nikomatsakis (Jun 11 2019 at 17:49, on Zulip):

can you push it?

blitzerr (Jun 11 2019 at 17:49, on Zulip):

But the things that I did over the weekend is.

blitzerr (Jun 11 2019 at 17:50, on Zulip):

Give me a few minutes. I will push it

blitzerr (Jun 11 2019 at 18:01, on Zulip):

@nikomatsakis Its pushed to branch upvar-tys-squashed

blitzerr (Jun 11 2019 at 18:02, on Zulip):

https://github.com/blitzerr/rust/commits/upvar-tys-squashed

nikomatsakis (Jun 11 2019 at 18:05, on Zulip):

ok

nikomatsakis (Jun 11 2019 at 18:05, on Zulip):

I'm starting a build now

blitzerr (Jun 12 2019 at 01:00, on Zulip):

@nikomatsakis how did it go ?

nikomatsakis (Jun 12 2019 at 14:03, on Zulip):

I reproduced your errors but didn't get a chance to look more deeply yet

nikomatsakis (Jul 10 2019 at 00:12, on Zulip):

did a little digging

nikomatsakis (Jul 10 2019 at 00:12, on Zulip):

somehow the cause is "off by one"

nikomatsakis (Jul 10 2019 at 00:13, on Zulip):

I see e.g. that the obligation causes are like:

nikomatsakis (Jul 10 2019 at 00:13, on Zulip):

the hidden one should be the tuple

nikomatsakis (Jul 10 2019 at 09:35, on Zulip):

wait, no, that's wrong. The problem is more in the note_obligation_cause_code --

nikomatsakis (Jul 10 2019 at 09:37, on Zulip):

when it prints out the (e.g.) "builtin derived obligation", it is printing the self type from the parent

nikomatsakis (Jul 10 2019 at 09:37, on Zulip):

but that's the one we want to skip

nikomatsakis (Jul 10 2019 at 09:40, on Zulip):

so @blitzerr what needs to happen is that in roughly this code here, we need to not necessarily print the type from the parent_trait_ref -- we need to look at the parent_code and, if it is a HiddenDerivedObligation, then we need to iterate to its parent

blitzerr (Jul 10 2019 at 13:52, on Zulip):

Thanks a lot @nikomatsakis . I will give this a shot.

nikomatsakis (Jul 10 2019 at 18:09, on Zulip):

Cool :)

blitzerr (Jul 16 2019 at 17:08, on Zulip):

@nikomatsakis :point_up:

nikomatsakis (Jul 25 2019 at 15:48, on Zulip):

@blitzerr did it work? :)

blitzerr (Jul 28 2019 at 19:36, on Zulip):

Sorry for the delay, @nikomatsakis . Unfortunately it didn't

blitzerr (Jul 28 2019 at 19:36, on Zulip):
           ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
                if let ObligationCauseCode::HiddenDerivedObligation(ref idata) = *data.parent_code {
                    err.note("Test Note: Found hidden type. So skipping ..");
                    let parent_trait_ref = self.resolve_type_vars_if_possible(&idata.parent_trait_ref);
                    let parent_predicate = parent_trait_ref.to_predicate();
                    self.note_obligation_cause_code(err,
                                                    &parent_predicate,
                                                    &idata.parent_code,
                                                    obligated_types);

                }
                let parent_trait_ref = self.resolve_type_vars_if_possible(&data.parent_trait_ref);
                let parent_predicate = parent_trait_ref.to_predicate();
                let ty = parent_trait_ref.skip_binder().self_ty();
                err.note(&format!("required because it appears within the type `{}`", ty));
                obligated_types.push(ty);

                if !self.is_recursive_obligation(obligated_types, &data.parent_code) {
                    self.note_obligation_cause_code(err,
                                                    &parent_predicate,
                                                    &data.parent_code,
                                                    obligated_types);
                }
            }
            ObligationCauseCode::HiddenDerivedObligation(ref data) => {
                if let ObligationCauseCode::HiddenDerivedObligation(ref idata) = *data.parent_code {
                    err.note("Test Note: ObligationCauseCode::HiddenDerivedObligation ..");
                    let parent_trait_ref = self.resolve_type_vars_if_possible(&idata.parent_trait_ref);
                    let parent_predicate = parent_trait_ref.to_predicate();
                    self.note_obligation_cause_code(err,
                                                    &parent_predicate,
                                                    &idata.parent_code,
                                                    obligated_types);

                }
            }
blitzerr (Jul 28 2019 at 19:37, on Zulip):

That's the change I made in fn note_obligation_cause_code.

blitzerr (Jul 28 2019 at 19:37, on Zulip):

The output after the test err.notes are:

blitzerr (Jul 28 2019 at 19:37, on Zulip):

blitzerr (Jul 30 2019 at 18:08, on Zulip):

@nikomatsakis :wave:

nikomatsakis (Aug 27 2019 at 18:11, on Zulip):

Sorry for the delay, nikomatsakis . Unfortunately it didn't

I see @blitzerr that this didn't work, eh? Did you push the changes by any chance?

blitzerr (Aug 27 2019 at 18:11, on Zulip):

hi @nikomatsakis

blitzerr (Aug 27 2019 at 18:11, on Zulip):

The changes are pushed

blitzerr (Aug 27 2019 at 18:13, on Zulip):

@nikomatsakis Please find them here : https://github.com/blitzerr/rust/commit/2af330a80b5fce8dd0761cae6bf041c445c783e0

nikomatsakis (Aug 27 2019 at 18:13, on Zulip):

ok, I'm running locally :)

nikomatsakis (Aug 27 2019 at 18:35, on Zulip):

@blitzerr so I see a lot of tests with a new note:

+   = note: ExprAssignable + MatchExpr etc.
nikomatsakis (Aug 27 2019 at 18:36, on Zulip):

ugh but also some other discrepancies

nikomatsakis (Aug 27 2019 at 18:36, on Zulip):

man this so annoying

blitzerr (Aug 27 2019 at 18:36, on Zulip):

My RUSTC_LOG was not working so I added test logs to err.note("

nikomatsakis (Aug 27 2019 at 18:37, on Zulip):

ok yes I see that

blitzerr (Aug 27 2019 at 18:37, on Zulip):

To track the control flow. Sorry about that.

nikomatsakis (Aug 27 2019 at 18:37, on Zulip):

I think it's RUST_LOG

nikomatsakis (Aug 27 2019 at 18:37, on Zulip):

since your branch is so old

nikomatsakis (Aug 27 2019 at 18:37, on Zulip):

in any case, i'm removing those and re-running :)

blitzerr (Aug 27 2019 at 18:42, on Zulip):

The latest failed tests are tracked here

blitzerr (Aug 27 2019 at 18:42, on Zulip):

@nikomatsakis :point_of_information:

blitzerr (Aug 27 2019 at 18:43, on Zulip):

The way I was looking at it is to get it to just print

-           = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6 recv:&std::sync::mpsc::Receiver<()>]```
somehow
blitzerr (Aug 27 2019 at 18:44, on Zulip):

In the latest one:

+           = note: required because it appears within the type `(&std::sync::mpsc::Receiver<()>,)`

this is gone

blitzerr (Aug 28 2019 at 14:21, on Zulip):

@nikomatsakis all fixed now ? :slight_smile:

blitzerr (Sep 12 2019 at 15:17, on Zulip):

@nikomatsakis
Following up from today's compiler meeting, you suggest that we merge to the latest master and then fix things - was that you thinking out loud or you wanted me to try that. The issue is that the merge will bring in new issues and we won't know whether the cause is the refactoring or the merging.
What do you think ?

nikomatsakis (Sep 12 2019 at 20:46, on Zulip):

@blitzerr well I don't think we should literally merge

nikomatsakis (Sep 12 2019 at 20:46, on Zulip):

However, I do think we've lost a lot of momentum over these test failures and ultimately I don't think they're that big of a deal

nikomatsakis (Sep 12 2019 at 20:46, on Zulip):

So I think what I would recommend is to take the PR and try to "rebase" the key commits over to master

nikomatsakis (Sep 12 2019 at 20:46, on Zulip):

effectively, re-implement

nikomatsakis (Sep 12 2019 at 20:47, on Zulip):

I guess I have to review the diff / history to see what that means exactly

nikomatsakis (Sep 12 2019 at 20:47, on Zulip):

but at this point it seems like the right approach to me

Last update: Nov 17 2019 at 07:50UTC