Stream: t-compiler/wg-nll

Topic: #52739


davidtwco (Aug 06 2018 at 14:14, on Zulip):

@nikomatsakis any thoughts on what to do for this issue #52739 (and #52742)?

nikomatsakis (Aug 06 2018 at 14:16, on Zulip):

are you talking @davidtwco about this error message?

error[E0623]: lifetime mismatch
  --> src/main.rs:13:5
   |
12 | fn func(foo: Foo<'_, '_>, bar: Bar<'_>) {
   |              -----------       ------- these two types are declared with different lifetimes...
13 |     foo.y = bar.z;
   |     ^^^^^^^^^^^^^ ...but data from `bar` flows into `foo` here
nikomatsakis (Aug 06 2018 at 14:16, on Zulip):

if so, I do :)

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

I'm curious actually -- do you still have that branch where you disabled nice region errors?

davidtwco (Aug 06 2018 at 14:17, on Zulip):

Just in general about what nice region errors we want to keep.

davidtwco (Aug 06 2018 at 14:17, on Zulip):

I do.

davidtwco (Aug 06 2018 at 14:17, on Zulip):

It's about a week old, but it exists.

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

I see -- so --- my sense is that this is a case where our "normal errors" are as good or better

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

well I'm a bit torn

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

I wonder if @Esteban Küber is around ...

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

/me doubts it

nikomatsakis (Aug 06 2018 at 14:17, on Zulip):

anyway, I think that our normal msg is better than that "nice" message in this case

nikomatsakis (Aug 06 2018 at 14:18, on Zulip):

but I wonder if there is some hybrid that is better than both

nikomatsakis (Aug 06 2018 at 14:18, on Zulip):

let me see I am trying to decide what to do with today :)

nikomatsakis (Aug 06 2018 at 14:18, on Zulip):

let me put this on the list

nikomatsakis (Aug 06 2018 at 14:18, on Zulip):

I think maybe I would focus a bit of time on this, and a bit on assessing the crater run results

davidtwco (Aug 06 2018 at 14:19, on Zulip):

Sounds good. Let me know when we decide on what we want to do with these.

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

as a simple step

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

we have this code:

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):
    pub fn try_report(&self) -> Option<ErrorReported> {
        self.try_report_named_anon_conflict()
            .or_else(|| self.try_report_anon_anon_conflict())
            .or_else(|| self.try_report_outlives_closure())
            .or_else(|| self.try_report_static_impl_trait())
    }
nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

that's the code you turned off, basically

davidtwco (Aug 06 2018 at 14:19, on Zulip):

Yeah.

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

I wonder if we want to have a "NLL variant" of this

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

that disables some of them but keeps others?

nikomatsakis (Aug 06 2018 at 14:19, on Zulip):

some of those (e.g., outlives closure) don't trigger for NLL anyway I guess

nikomatsakis (Aug 06 2018 at 14:20, on Zulip):

I am thinking that named_anon_conflict may still be better

nikomatsakis (Aug 06 2018 at 14:20, on Zulip):

at least in some cases, not sure

davidtwco (Aug 06 2018 at 14:20, on Zulip):

named_anon_conflict is better when the suggestion is correct.

davidtwco (Aug 06 2018 at 14:21, on Zulip):

anon_anon_conflict is worse in all cases, I think.

davidtwco (Aug 06 2018 at 14:21, on Zulip):

and yeah, outlives_closure doesn't come up.

nikomatsakis (Aug 06 2018 at 14:21, on Zulip):

I agree

nikomatsakis (Aug 06 2018 at 14:21, on Zulip):

the "when the suggestion is correct" makes me think though

nikomatsakis (Aug 06 2018 at 14:21, on Zulip):

that ultimately we ought to reimplemented named_anon_conflict

nikomatsakis (Aug 06 2018 at 14:21, on Zulip):

but that requires us to make a bit more progress I think

nikomatsakis (Aug 06 2018 at 14:22, on Zulip):

on being able to explain why a lifetime is constrained

nikomatsakis (Aug 06 2018 at 14:22, on Zulip):

right now we just cite one span, I think to do significantly better -- at least in some cases -- we would want to cite >1 place?

nikomatsakis (Aug 06 2018 at 14:33, on Zulip):

@davidtwco to start then maybe open a PR that removes everything but "named-anon-conflict" for NLL mode? (not sure about that final case)

davidtwco (Aug 06 2018 at 14:33, on Zulip):

Sure thing, I'll get started on that @nikomatsakis.

nikomatsakis (Aug 06 2018 at 14:34, on Zulip):

re: #52742, I'd be happy to mentor through a fix there, though I have to refresh my memory. I think it'd be something we could solve by adding some more cases to the "region-name" code.

davidtwco (Aug 06 2018 at 16:05, on Zulip):

@nikomatsakis Oops, didn't see that last message. I had thought that both #52739 and #52742 were just "figure out which nice region errors to keep".

nikomatsakis (Aug 06 2018 at 16:06, on Zulip):

well maybe it is — but I was thinking that, at least in some cases, we should highlight regions in the impl declaration

davidtwco (Aug 06 2018 at 16:37, on Zulip):

@nikomatsakis Submitted #53115 with the disabling all but named_anon_conflict in NLL mode. One strange test change. Could probably also add the fix for #52742 to this PR.

davidtwco (Aug 06 2018 at 16:37, on Zulip):

(when such a fix exists)

nikomatsakis (Aug 06 2018 at 16:52, on Zulip):

@davidtwco left some notes https://github.com/rust-lang/rust/pull/53115#pullrequestreview-143673844

davidtwco (Aug 06 2018 at 17:07, on Zulip):

For the potential issues to open, might be worth adding them to #52663 - I've replied to the reviews where they are already on that list.

davidtwco (Aug 06 2018 at 17:08, on Zulip):

Interestingly, the try_report_from_nll change fixed the strangeness with that one test case in the migrate mode - presumably something to do with the is_mir_borrowck implementation.

nikomatsakis (Aug 06 2018 at 17:15, on Zulip):

@davidtwco I think it's because the code was being invoked from the AST borrow checker

nikomatsakis (Aug 06 2018 at 17:15, on Zulip):

I thought that might be the case...

nikomatsakis (Aug 06 2018 at 17:16, on Zulip):

I agree with adding them to the list in #52663, sure

nikomatsakis (Aug 06 2018 at 17:16, on Zulip):

not sure that distinct issues are useful

nikomatsakis (Aug 06 2018 at 17:16, on Zulip):

but I'd like to keep those ideas around

davidtwco (Aug 06 2018 at 17:16, on Zulip):

Pushed a commit that changes it to a try_report_from_nll approach.

davidtwco (Aug 06 2018 at 17:23, on Zulip):

Also added a new comment to #52663 with the new potential improvement (the rest were already there).

nikomatsakis (Aug 06 2018 at 17:24, on Zulip):

@davidtwco ty

davidtwco (Aug 06 2018 at 18:44, on Zulip):

@nikomatsakis fixed the compile-fail failures (only looked at ui locally) - will need another r+ if good.

nikomatsakis (Aug 06 2018 at 18:49, on Zulip):

ok

Last update: Nov 21 2019 at 14:05UTC