remove more trivial regions in evaluate_added_goals_and_make_canonical_response

[sjwang05]

cc #161575. I don't think it fixes the issue per se, since the most principled fix would be to just deny unconstrained lifetime args like we do for types and consts already. At the very least, though, it Makes Things Go Faster.

In the example from #161575 (comment), each 'unconstrained appears only once in the entire response: in the rhs of the 'a: 'unconstrained#N bound. Since they are mentioned nowhere else and are created only when proving our own nested goals, these outlives constraints are all satisfiable by setting 'unconstrained := 'empty, which tells us nothing about 'a. Therefore, (I think) that makes it safe to treat all of these constraints as trivial and drop them entirely, drop all of these requirements entirely, similar to what we already do with reflexive or duplicate region constraints. In other words, if a ReVar appears only once in the entire response, and that place is the rhs of an outlives constraint, then it is safe to drop that constraint.

I'm a little worried about the perf impact of the visitor on "normal" code, but fwiw even a 100-deep nested version of the reproducer compiles in about 0.05s on my machine.

r? lcnr

[sjwang05]

Fun fact: the number of outlives constraints grows exactly as fib(2 * depth) :D

[ShoyuVanilla]

@bors try @rust-timer queue

[ShoyuVanilla]

Shouldn't we consider the universes of region vars?

View changes since the review

[sjwang05]

Normally the leak check runs before our removal code and rejects bad universe naming, but with -Zno-leak-check this program gets rejected by nightly but accepted by this stage1:

//@ compile-flags: -Znext-solver -Zno-leak-check

//! Make sure we don't drop trivial-looking region constraints that would otherwise fail
//! leak check.

trait Trait {}
trait Other<'a, 'b> {}

struct Foo;
// We need this indirection because something direct like `for<'a> &'a (): 'b` gives us a
// TypeOutlives constraint, whereas we want to be testing how we handle RegionOutlives, and
// only `impl Other for Bar`'s where-clause can give us that.
impl<'b> Trait for Foo where for<'a> Bar: Other<'a, 'b> {}

struct Bar;
impl<'a, 'b> Other<'a, 'b> for Bar where 'a: 'b {}

fn f<T: Trait>(_: T) {}

fn main() { f(Foo); }
//~^ ERROR higher-ranked lifetime error

so I added a can_name check to the retain (which I think should be sufficient?) and also added that program as a uitest.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 0be4976 (0be497690edf5f58dfd2a6cfff82660c9c75ffa6)
Base parent: 5321a4f (5321a4f40c957cf3587c055e77461febc2ebc865)

[rust-timer]

Finished benchmarking commit (0be4976): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 402.71 MiB -> 402.86 MiB (0.04%)

[Kobzol]

(Sorry, a rustc-perf failure, looking into it)

[ShoyuVanilla]

Would rerunning with the same artifact make sense?

[Kobzol]

Yup, I deleted the result from the DB, and will rerun the artifact for you (once the underlying bug is fixed and deployed, which should happen in ~20 minutes).

[ShoyuVanilla]

Yup, I deleted the result from the DB, and will rerun the artifact for you (once the underlying bug is fixed and deployed, which should happen in ~20 minutes).

Huge thanks! 😄

[Kobzol]

@rust-timer build 0be4976

[rust-timer]

Finished benchmarking commit (0be4976): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -1.8%, secondary -2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.8%	[-2.2%, -1.1%]	3
Improvements ✅ (secondary)	-2.6%	[-3.6%, -1.2%]	4
All ❌✅ (primary)	-1.8%	[-2.2%, -1.1%]	3

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 477.635s -> 477.178s (-0.10%)
Artifact size: 402.71 MiB -> 402.86 MiB (0.04%)