Display better error messages for E0282

[KiChjang]

Fixes #36554.

[rust-highfive]

r? @nrc

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

[frewsxcv]

FYI, Travis found some issues.

[KiChjang]

r? @nikomatsakis

[bors]

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

[nikomatsakis]

Looks good! I just left one minor suggestion. The other thing that I'm missing is some kind of dedicated test where we show off a nice error. Maybe a UI test for this case?

fn foo<X>() { }
fn bar() {
    foo();
}

[nikomatsakis]

We can give more information about lattice variables -- at minimum, we can give a span. The this variable is always of a type (either Lub or Glb) that has access to a CombineFields (self.fields, in each case). CombineFields has a field trace of type TypeTrace, and hiding in there is a field cause of type ObligationCause. This is basically "why we are computing the common-supertype or common-subtype of these types". At minimum though it'd be nice to add a method fn cause(&self) -> &OlbligationCause<'tcx> to LatticeDir that returns &self.fields.cause. Then we can create give a span here like LatticeVariable(this.cause().span).

[KiChjang]

Ack, the complication here is that self.fields.cause is an Option<ty::relate::Cause>, and is not an ObligationCause at all! Let me check whether there are other fields that I can use...

[KiChjang]

Found it, self.fields.trace.cause may work.

[nikomatsakis]

...if we give LatticeVariable a span, then every variant will have one.

[KiChjang]

@nikomatsakis There is actually a UI test that demonstrates this and it's at src/test/ui/codemap_tests/repair_span_std_macros.rs. Would that suffice or do you think it's still better to create a new one specifically for this?

[nikomatsakis]

@KiChjang

There is actually a UI test that demonstrates this and it's at src/test/ui/codemap_tests/repair_span_std_macros.rs. Would that suffice or do you think it's still better to create a new one specifically for this?

I think it's better to have a targeted test. For one thing, when doing refactorings, it is often hard to tell if a change to the test's output is acceptable or not; but if there is a focused test that is aimed at this particular feature, it will be more obvious. Another thing that happens from time to time is that when we are doing refactorings some test is hard to port to the new system and winds up getting deleted, thereby inadvertendly removing the only test for this feature. So I think it's best to have simple tests that specifically target your change (in addition to the ones which accidentally do so).

Plus, the scenario in that UI test was not quite the one I suggested, right?

fn foo<X>() { }
fn bar() {
    foo();
}

[nikomatsakis]

(Sorry for the delay.)

[KiChjang]

UI test added.

[nikomatsakis]

@KiChjang

tidy error: /checkout/src/test/ui/missing-items/missing-type-parameter.rs: incorrect license

[KiChjang]

Dumb vim. Fixed license now.

[KiChjang]

Huh... I'm not sure what the error in Travis CI is about this time.

[eddyb]

@bors r=nikomatsakis

[bors]

📌 Commit d24028b has been approved by nikomatsakis

[bors]

⌛ Testing commit d24028b with merge 5e2f37f...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: nikomatsakis
Pushing 5e2f37f to master...