Prevent very long compilation runtimes in LateBoundRegionNameCollector

[b-naber]

Fixes #83150

On recursive types such as in the example given in #83150, the current implementation of LateBoundRegionNameCollector has very long compilation runtimes. To prevent those we store the types visited in the middle::ty::Visitor implementation of LateBoundRegionNameCollector in a SsoHashSet.

[rust-highfive]

r? @lcnr

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

[b-naber]

This test passes for an unexplainable reason. When I compile this file I get the expected error:

error[E0275]: overflow evaluating the requirement `Map<&mut Map<&mut Map<&mut Map<&mut Map<.....

But the test suite cannot find an error.

[lcnr]

i expect you to need at least // build-pass here, as the overly large types should only get created during mono item collection

[lcnr]

I don't think that using type length limit is the best approach here.

The issue here is that the type T is &mut Map<T_prev, closure [T_prev]> so the type size scales by 2^depth instead of just depth.

Using https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/sso/struct.SsoHashSet.html in visit_ty seems like a potentially better solution to me

[lcnr]

i expect you to need at least // build-pass here, as the overly large types should only get created during mono item collection

[lcnr]

infinitely recursed types

nit: there are no infinite types. Types are finite trees, so infinite types are impossible.
We can get very large types though, which causes the hang here.

[b-naber]

@lcnr Thanks for the review.

I don't understand how you would want to use SsoHashSet. Do you want to fill SsoHashSet until some given capacity is reached and then break control?

[lcnr]

I don't want to break control at all here. Afaict this is not an infinite hang, this is a finite, but incredibly large perf regression.

I want to use a SsoHashSet to not look multiple times into identical parts of our type

[b-naber]

@lcnr Thanks, that clarifies things. You're right, this isn't a hang.

I updated the PR to use the SsoHashSet. There's still a problem with the test though. When I changed this to build-pass the test fails, but it also fails when I add an expected error to the test with the reason that the test compiled successfully?!

[b-naber]

There's also a problem with another test, seems as if `LateBoundRegionNameCollector' has different functionality if we skip types we've seen before.

[b-naber]

@lcnr Can you take another look? What do we do about the tests?

[lcnr]

i think the correct attribute here is // build-fail. By default we only try to check (like cargo check) ui tests afaik.

[b-naber]

@lcnr Yes, that's the correct attribute. Thanks for the review and the help.

[lcnr]

Suggested change

	type_collector: SsoHashSet::with_capacity(1000),
	type_collector: SsoHashSet::new(),

we don't want to allocate here by default.

[lcnr]

Suggested change

	//~^ overflow evaluating
	//~^ overflow evaluating

[lcnr]

please update the PR name and description, then this should be ready after a perf run.

[b-naber]

@lcnr This is ready for the perf run.

[bors]

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

[lcnr]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 3194b26 with merge 9fb1f198bce56ca989b0203c337bedffb89b96a1...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9fb1f198bce56ca989b0203c337bedffb89b96a1 (9fb1f198bce56ca989b0203c337bedffb89b96a1)

[rust-timer]

Queued 9fb1f198bce56ca989b0203c337bedffb89b96a1 with parent b1b0a15, future comparison URL.

[rust-timer]

Finished benchmarking try commit (9fb1f198bce56ca989b0203c337bedffb89b96a1): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

[lcnr]

@bors r+

[bors]

📌 Commit 3194b26 has been approved by lcnr

[bors]

⌛ Testing commit 3194b26 with merge d203fce...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing d203fce to master...

[estebank]

Should this be backported to 1.52? @rust-lang/compiler

---

Context: this fixes a t-hang stable-to-stable regression that has been reported multiple times caused by big type names, the latest report being #84102