Don't anonymize bound region names during typeck

[Aaron1011]

Once this anonymization has performed, we have no
way of recovering the original names during NLL
borrow checking. Keeping the original names allows
error messages in full NLL mode to contain the original
bound region names.

As a result, the typeck results may contain types that
differ only in the names used for their bound regions. However,
anonimization of bound regions does not guarantee that
all distinct types are unqual (e.g. not subtypes of each other).
For example, for<'a> fn(&'a u32, &'a u32) and
for<'b, 'c> fn(&'b u32, &'c u32) are subtypes of each other,
as explained here:

rust/compiler/rustc_infer/src/infer/nll_relate/mod.rs

Lines 682 to 690 in 63cc2bb

	// Reset the ambient variance to covariant. This is needed
	// to correctly handle cases like
	//
	// for<'a> fn(&'a u32, &'a u32) == for<'b, 'c> fn(&'b u32, &'c u32)
	//
	// Somewhat surprisingly, these two types are actually
	// **equal**, even though the one on the right looks more
	// polymorphic. The reason is due to subtyping. To see it,
	// consider that each function can call the other:

Therefore, any code handling types with higher-ranked regions already
needs to handle the case where two distinct Tys are 'actually'
equal.

[rust-highfive]

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

[Aaron1011]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 78013f2 with merge 1ebe71fdc5fb1f660c40031059cb73ee72696e87...

[bors]

☀️ Try build successful - checks-actions
Build commit: 1ebe71fdc5fb1f660c40031059cb73ee72696e87 (1ebe71fdc5fb1f660c40031059cb73ee72696e87)

[rust-timer]

Queued 1ebe71fdc5fb1f660c40031059cb73ee72696e87 with parent ac8dd1b, future comparison URL.

[rust-timer]

Finished benchmarking commit (1ebe71fdc5fb1f660c40031059cb73ee72696e87): comparison url.

Summary: This change led to moderate relevant regressions 😿 in compiler performance.

• Moderate regression in instruction counts (up to 0.6% on full builds of coercions)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[estebank]

Changes look good.

[estebank]

Isn't there one of these already somewhere in the codebase?

[Aaron1011]

I thought so too, but I couldn't find one.

[estebank]

I must have been thinking of the following, which looks like the exact opposite 😅

rust/compiler/rustc_typeck/src/hir_wf_check.rs

Lines 169 to 188 in be76bdf

	struct EraseAllBoundRegions<'tcx> {
	tcx: TyCtxt<'tcx>,
	}
	// Higher ranked regions are complicated.
	// To make matters worse, the HIR WF check can instantiate them
	// outside of a `Binder`, due to the way we (ab)use
	// `ItemCtxt::to_ty`. To make things simpler, we just erase all
	// of them, regardless of depth. At worse, this will give
	// us an inaccurate span for an error message, but cannot
	// lead to unsoundess (we call `delay_span_bug` at the start
	// of `diagnostic_hir_wf_check`).
	impl<'tcx> TypeFolder<'tcx> for EraseAllBoundRegions<'tcx> {
	fn tcx<'a>(&'a self) -> TyCtxt<'tcx> {
	self.tcx
	}
	fn fold_region(&mut self, r: Region<'tcx>) -> Region<'tcx> {
	if let ty::ReLateBound(..) = r { &ty::ReErased } else { r }
	}
	}

[estebank]

r? @estebank
@bors r+

[bors]

📌 Commit 78013f2 has been approved by estebank

[bors]

⌛ Testing commit 78013f2 with merge 67c26d34632e929389f62e064008339f1bc9cf7d...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

The job i686-mingw-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Finished dev [unoptimized + debuginfo] target(s) in 0.32s
Set({"src/tools/tidy"}) not skipped for "bootstrap::test::Tidy" -- not in ["src/test/ui"]
Skipping Suite("src/test/ui") because it is excluded
Suite("src/test/run-pass-valgrind") not skipped for "bootstrap::test::RunPassValgrind" -- not in ["src/test/ui"]
thread 'main' panicked at '"lldb" "-P" failed Output { status: ExitStatus(ExitStatus(3221225781)), stdout: "", stderr: "" }', src\bootstrap\test.rs:1394:40
Build completed unsuccessfully in 0:00:01
Build completed unsuccessfully in 0:00:01
make: *** [Makefile:80: ci-mingw-subset-1] Error 1

[ehuss]

@bors retry
Issue with Windows LLDB

[bors]

⌛ Testing commit 78013f2 with merge 69c1c6a...

[bors]

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 69c1c6a to master...

[rust-timer]

Finished benchmarking commit (69c1c6a): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression