Improve SubSupConflict with a named and an anonymous lifetime parameter #42701

[cengiz-io]

Hello!

This fixes#42701.

UPDATE 01

Tests are producing different results between different env builds.
This inconsistency might take a long time to investigate and fix. So, be patient

UPDATE 02

Changed an FxHashMap with a BTreeMap. Inconsistency seems to be resolved for now.

[rust-highfive]

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

[gaurikholkar-zz]

#42700
The first snippet of code, that's how the ui test must output

[gaurikholkar-zz]

This is of type ConcreteFailure, not SubSupConflict

[gaurikholkar-zz]

You will have to remove the is_self_anon check. Maybe you can try an example without self, that should work.

[nikomatsakis]

r? @nikomatsakis

[nikomatsakis]

I think this is a good test case:

structFoo{field:i32,}fnfoo2<'a>(a:&'aFoo,x:&i32) -> &'ai32{iftrue{let p:&i32 = &a.field;&*p
}else{&*x
}}fnmain(){}

[arielb1]

Hi @cengizio! Nice PR! Can you add @nikomatsakis's test so we can see how the errors look and fix this?

[gaurikholkar-zz]

@nikomatsakis can you suggest a test case for anon_anon conflicts too?

[nikomatsakis]

@gaurikholkar I guess that's a bit harder. If you just remove the named lifetime, then the elision rules will fail. You can use an &self to still create a conflict, but then we get into the rules that disable anon-anon reporting around self, I believe.

[bors]

☔ The latest upstream changes (presumably #44079) made this pull request unmergeable. Please resolve the merge conflicts.

[nikomatsakis]

@cengizio

Maybe try this example too?

structFoo{field:i32,}fnfoo2<'a,'b>(a:&'aFoo,x:&'bi32) -> &'ai32{iftrue{let p:&i32 = &a.field;&*p
}else{&*x
}}fnmain(){}

I imagine this will exercise the "different lifetimes" code a bit.

[arielb1]

You have a tidy error:

[00:03:51] tidy error: /checkout/src/test/ui/lifetime-errors/42701_one_named_and_one_anonymous.rs:12: trailing whitespace

[arielb1]

This PR is basically ready to land, you just have to fix the tidy error. Just a ping to make sure this isn't getting lost.

[cengiz-io]

Hello @arielb1

Thanks for notifying. I'm still trying to resolve this example case #44167 (comment)

Whenever that finishes, I'll fix tidy error and push them.

[cengiz-io]

Just fixed tidy error.

Also, while rebasing, it turned out to change 3 ui tests. I'm not sure about this because it was an implicit change. 314bb98

@nikomatsakis maybe can shed some light to this implicit change

[cengiz-io]

There seems to be some changes which are breaking other tests also. Back to the drawing board...

[nikomatsakis]

@cengizio those 3 ui tests appear to be bug fixes =) that is, thanks to your change, the 'new and improved' errors are triggering more often. seems good!

[nikomatsakis]

@cengizio also those test errors are not unexpected. I think you just have to update the tests to change them to expect the new error message. However, these tests are a bit surprising to me:

object-lifetime-default-from-box-error.rs
issue-40288-2.rs

I'm not sure what's up with those two cases!

[cengiz-io]

@nikomatsakisupdate-references.sh doesn't help much on compile-fail tests (there's a bash issue but even solving that didn't work out) so I'm updating them manually.

I will also check those two surprising cases and report here.
Thank you!

[nikomatsakis]

@cengizio yes, compile-fail tests have to be edited manually.

[cengiz-io]

@nikomatsakis@arielb1@gaurikholkar hello again!

I've updated the test cases.

NOTE: I decided to make test assertions in compile-fail tests as specific as possible. Please let me know if that's not the general case.

[gaurikholkar-zz]

@cengizio this test fails
src/test/compile-fail/associated-types/cache/project-fn-ret-invariant.rs

[01:00:24] Error {
[01:00:24] line_num: 50,
[01:00:24] kind: Some(
[01:00:24] Error
[01:00:24] ),
[01:00:24] msg: "50:5: 50:6: lifetime mismatch [E0623]"
[01:00:24] }
[01:00:24] ]
[01:00:24]

I guess you have given the wrong column no for the error

[cengiz-io]

@gaurikholkar thanks for noticing. The annoying part is, all tests are passing on my local machine. (with latest HEAD).

Let's see if this fixes the problem.

[cengiz-io]

Argh! There seems to be line differences between my git clone and travis' copy. I'll investigate this further tomorrow.

[gaurikholkar-zz]

gaurikholkar-zz@cb0d746
Maybe this should work

[gaurikholkar-zz]

Ignore the indentation mess

[bors]

☀️ Test successful - status-travis
State: approved= try=True

[kennytm]

@rust-lang/infra Perf check wanted (44a477ee0833eef8007499fec6ebd88c5289da8a).

[nikomatsakis]

Looks like try build is ready. cc @rust-lang/infra

[Mark-Simulacrum]

http://perf.rust-lang.org/compare.html?start=fc77b623d3072005f35e320e652f2b31695d493c&end=44a477ee0833eef8007499fec6ebd88c5289da8a&stat=instructions%3Au

[nikomatsakis]

OK so -- the perf results seem fine, but @cengizio and I were discussing that maybe it's still worth replacing the BTreeMap with a Vec and a FxHashSet for duplicate checking. That would achieve the same results and seems better.

[nikomatsakis]

@arielb1 and I were talking. We think we'll r+ this PR and then do any perf experiments in follow-up work.

@bors r+

[bors]

📌 Commit dad7d94 has been approved by nikomatsakis

[bors]

⌛ Testing commit dad7d94 with merge 5b1b7b6c84a3e8e723a0416485a228abf756918c...

[bors]

💔 Test failed - status-travis

[nikomatsakis]

@bors r+

[bors]

📌 Commit f53fc57 has been approved by nikomatsakis

[bors]

⌛ Testing commit f53fc57 with merge 79cfce3...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 79cfce3 to master...