Consider alias bounds when computing liveness in NLL (but this time sound hopefully)

[compiler-errors]

This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing ReErased hack we use instead.

r? @aliemjay -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :>

Fixes #116794 as well

---

(mostly copied from #116040 and reworked slightly)

Background

Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark all of the regions in their args¹ as live, leading to bugs like #42940.

In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias.

For example:

trait Captures<'a> {}
impl<T> Captures<'_> for T {}

fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {}

fn test_two_mut(mut x: ()) {
    let _f1 = capture(&mut x);
    let _f2 = capture(&mut x);
    //~^ ERROR cannot borrow `x` as mutable more than once at a time
}

In the example above, we should be able to deduce from the 'static bound on capture's opaque that even though 'o is a captured region, it can never show up in the opaque's hidden type, and can soundly be ignored for liveness purposes.

The Fix

We apply a simple version of RFC 1214's OutlivesProjectionEnv and OutlivesProjectionTraitDef rules to NLL's make_all_regions_live computation.

Specifically, when we encounter an alias type, we:

1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is 'static, and in that case, prefer 'static. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque.
2. Otherwise, walk through the alias's args recursively, as we do today.

Limitation: Multiple choices

This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice.

If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of 'static, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future.

Limitation: Opaque hidden types

Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail:

trait Captures<'a> {}
impl<T> Captures<'_> for T {}

fn a() -> impl Sized + 'static {
    b(&vec![])
}

fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {}

We need to have existential regions to avoid unsoundness when an opaque captures a region which is not represented in its own substs but which outlives a region that does.

Read more

Context: #115822 (comment) (for the liveness case)
More context: #42940 (comment) (for the opaque capture case, which this does not fix)

Footnotes

1. except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques. ↩

[aliemjay]

Looks good.
You can r=me after resolving the nits and the FCP.

[aliemjay]

This should fix #116794. you can add a regression test to close the issue.

[aliemjay]

@rfcbot fcp merge

See the description for details.

[rfcbot]

Team member @aliemjay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @aliemjay
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nikomatsakis]

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[traviscross]

@rustbot labels +A-impl-trait +F-type_alias_impl_trait +F-generic_associated_types

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[aliemjay]

@bors r+ rollup=never

[bors]

📌 Commit f0e385d has been approved by aliemjay

It is now in the queue for this repository.

[bors]

⌛ Testing commit f0e385d with merge ec2b311...

[bors]

☀️ Test successful - checks-actions
Approved by: aliemjay
Pushing ec2b311 to master...

[rust-timer]

Finished benchmarking commit (ec2b311): 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
Regressions ❌ (secondary)	4.1%	[2.6%, 5.6%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

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: 636.145s -> 635.323s (-0.13%)
Artifact size: 304.44 MiB -> 304.45 MiB (0.01%)

[Manishearth]

This caused a regression: #117455