Add bidirectional where clauses on RPITIT synthesized GATs

[spastorino]

Given the following:

struct MyStruct<'a, T>(&'a T);

trait MyTrait<'a, T> {
    fn my_fn<'b, 'c, 'd, V>(item: &'c String) -> impl Sized + 'a + 'b + 'c where V: 'b, V: 'd;
}

impl<'a, T> MyTrait<'a, T> for MyStruct<'a, T> {
    fn my_fn<'b, 'c, 'd, V>(_: &'c String) -> impl Sized + 'a + 'b + 'c
    where
        V: 'b,
        V: 'd,
    {
        unimplemented!();
    }
}

We need the desugaring to be:

trait MyTrait<'a, T> {
    type MyFn<'bf, 'df, Vf, 'a2, 'b2, 'c2>: Sized + 'a2 + 'b2 + 'c2 where Vf: 'b2, 'a2: 'a, 'a: 'a2, 'b2: 'bf, 'bf: 'b2;

    fn my_fn<'b, 'c, 'd, V>(item: &'c String) -> MyStruct<'a>::MyFn<'b, 'd, V, 'a, 'b, 'c> where V: 'b, V: 'd {
        type opaque<'a3, 'b3, 'c3>;
    };
}
    
impl<'a, T> MyIter<'a, T> for MyStruct<'a, T> {
    type MyFn<'bf, 'df, Vf, 'a2, 'b2, 'c2> = impl Sized + 'a2 + 'b2 + 'c2 where Vf: b2, 'a2: 'a, 'a: 'a2, 'b2: 'bf, 'bf: 'b2;

    fn my_fn<'b, 'c, 'd, V>(_: &'c String) -> MyStruct<'a>::MyFn<'a, 'b, 'c, V> where V: 'b, V: 'd {
        type opaque<'a3, 'b3, 'c3>;
        unimplemented!();
    }
}

This PR adds the where clauses for the MyFn generated GATs.

This is a draft with a very ugly solution so we can make comments over concrete code.

r? @compiler-errors

[spastorino]

@compiler-errors you told me that this could be done using named_region or something like that. I was reusing this idea https://github.com/rust-lang/rust/pull/112682/files#diff-6d9f47a8bce6ee31fd3e867514607c8503a11cf91616ce418393ba9304a22c16R435. In case Github doesn't take you there line 435 on the right side.

[spastorino]

I guess you meant named_bound_var.

[compiler-errors]

I guess that's fine. I think using named_bound_var might be easier but I guess it doesn't matter.

[spastorino]

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)

Uhh this is likely ungreat perf wise.

[spastorino]

@compiler-errors this is ungreat :). Unsure how bad would be the impact, we should probably move bidir_outlives out from the OpaqueTy variant or came up with better ideas to improve the size of ItemKind.

[oli-obk]

You could intern the use of OpaqueTy in https://github.com/rust-lang/rust/blob/b425640d7552a352bb8a10f766c437204ba28b60/compiler/rustc_hir/src/hir.rs#L3314 (make it &'hir OpaqueTy<'hir>)

[compiler-errors]

Yeah this is the best way of doing it.

[rust-timer]

Finished benchmarking commit (0c07919285caf69c4888d889cd23b4f62578282f): comparison URL.

Overall result: ✅ improvements - no action needed

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 may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.4%, -0.4%]	1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.5%	[3.5%, 3.5%]	1
Regressions ❌ (secondary)	2.8%	[2.1%, 3.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.5%	[3.5%, 3.5%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 663.81s -> 660.746s (-0.46%)

[spastorino]

@compiler-errors now I think this is ready to merge. Perf is back to normal.

[compiler-errors]

I still need to review this for correctness, but yeah, I saw the perf results.

[spastorino]

Rebased ☝️ after conflicts on predicates_of.rs

[compiler-errors]

Can you rename bidir_outlives in the hir::OpaqueTy to something like lifetime_mapping, and add a doc comment that explains what it is, and note that it's only populated if in_trait is true (because it's unneeded for other opaques)?

r=me after that and the nits below

[compiler-errors]

@bors r+

[bors]

📌 Commit 6f4a51e has been approved by compiler-errors

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6f4a51e with merge 3307274...

[bors]

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 3307274 to master...

[rust-timer]

Finished benchmarking commit (3307274): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.8%, 0.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.9%	[-1.9%, -1.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.6%	[-1.9%, 0.8%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.7%, -0.5%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.6%	[-0.7%, -0.5%]	4

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 660.993s -> 662.146s (0.17%)