Consider captured regions for opaque type region liveness. by jackh726 · Pull Request #156027 · rust-lang/rust

jackh726 · 2026-05-01T03:45:47Z

For opaques, when we're calculating liveness for opaques, we want to consider any captured lifetimes that can outlive the opaque type, which is more than just the outlives bounds.

r? lqd

lqd · 2026-05-01T12:25:07Z

Until I can look at this next week, @bors try @rust-timer queue

Consider captured regions for opaque type region liveness.

rust-bors · 2026-05-01T14:33:24Z

☀️ Try build successful (CI)
Build commit: 97f0332 (97f0332b563f5fd8ae70b57a13a7b23b27db3012, parent: 0469a92a76c327df972cb6c1356934b7a0c6b86d)

rust-timer · 2026-05-01T15:16:31Z

Finished benchmarking commit (97f0332): 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 -2.3%)

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)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.3%, -2.3%]	1

Cycles

Results (secondary 2.0%)

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)	4.1%	[3.3%, 4.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.2%, secondary 0.2%)

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

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.1%, 0.4%]	33
Regressions ❌ (secondary)	0.2%	[0.0%, 0.4%]	24
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.1%, 0.4%]	33

Bootstrap: 481.713s -> 483.106s (0.29%)
Artifact size: 390.96 MiB -> 390.99 MiB (0.01%)

lqd

I also started down this direction so some of this makes sense to me. We stopped because it felt fragile and test coverage was, and still is, weak. So with that caveat this generally feels fine to me, but again, I don't know enough about this area to say what problems there could be.

This direction felt less nice than what lcnr described in the t-types meeting where we described the issue, so I'll defer to them.

View changes since this review

lqd · 2026-05-04T09:23:57Z

+    // Thinking about it, I was originally a bit concerned about something like `'a: 'static`, and
+    // whether or not we need to mark `'a` as live. I don't think *today* we do, since I think regions
+    // that outlive `'static` are special enough, but I *could* imagine some world where we need to be
+    // more careful about this. Given I can't find a test that goes wrong, I'm going to leave in this
+    // optimization.


I was also a bit worried, and we should expand test coverage here. I'm not confident enough in any of this to trust the two tests we have.

lqd · 2026-05-04T10:00:10Z

+    // Unfortunately, we have to use a new `InferCtxt` each call, because
+    // region constraints get added and solved there and we need to test each
+    // call individually.


This is also unfortunate, combined with the fact that it's being called a quadratic number of times (thankfully the N should be small in most cases).

jackh726 · 2026-06-03T11:28:36Z

@bors try

Consider captured regions for opaque type region liveness.

rust-bors · 2026-06-03T13:40:17Z

☀️ Try build successful (CI)
Build commit: 8e819ff (8e819ff5434466e9a823e03871a1775ac7d84ddf, parent: 53509ca37e3b507887607c2f4a7f23bd4838f099)

jackh726 · 2026-06-03T15:05:15Z

@craterbot check

craterbot · 2026-06-03T15:05:39Z

👌 Experiment pr-156027 created and queued.
🤖 Automatically detected try build 8e819ff
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

jackh726 · 2026-06-04T14:21:35Z

@craterbot p=2

We want to get this in so that we can do a nightly preview of Polonius

craterbot · 2026-06-04T14:21:37Z

📝 Configuration of the pr-156027 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2026-06-05T06:57:03Z

🚧 Experiment pr-156027 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2026-06-07T15:33:26Z

🎉 Experiment pr-156027 is completed!
📊 65 regressed and 1 fixed (964870 total)
📊 5391 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-156027/retry-regressed-list.txt ↩

lqd · 2026-06-15T14:15:48Z

@bors try @rust-timer queue

Consider captured regions for opaque type region liveness.

rust-bors · 2026-06-15T16:24:49Z

☀️ Try build successful (CI)
Build commit: 15410a9 (15410a9b8f02f49884fc00943ebd7e2fcc8eed78, parent: 5ff740e85e8569289a93573ad25f1295dfae0323)

rust-timer · 2026-06-15T17:25:47Z

Finished benchmarking commit (15410a9): 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 -0.5%, secondary -0.1%)

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

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	1.4%	[1.4%, 1.4%]	1
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-1.7%	[-1.7%, -1.7%]	1
All ❌✅ (primary)	-0.5%	[-2.7%, 1.7%]	2

Cycles

Results (primary 2.1%)

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

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

Binary size

Results (primary 0.2%, secondary 0.2%)

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

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.0%, 0.8%]	82
Regressions ❌ (secondary)	0.2%	[0.0%, 0.8%]	49
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.8%]	82

Bootstrap: 518.77s -> 521.39s (0.51%)
Artifact size: 401.47 MiB -> 401.09 MiB (-0.10%)

lcnr · 2026-06-26T09:53:10Z

+// Test to show what happens if we were not careful and allowed invariant
+// lifetimes to escape through a GAT.


why does the "invariant" matter? Could just as well be contravariant, couldn't it?

View changes since the review

Hmm, potentially? Would be an interesting test case to get an contravariant lifetime through (since the &mut is going to force invariance).

This is a preexisting test anyways.

lcnr

Mainly a bunch of nits. From reading https://hackmd.io/rvBf2GtiRGKqUVIdDd5g2g I am still left feeling unclear why polonius cares about liveness the way it does, what additional constraints this puts on liveness (e.g. cases where something has to outlive 'static are fine for liveness apparently).

The other question I have is whether you see this having benefits outside of this specific need of polonius? I think yes?

View changes since this review

lcnr · 2026-06-26T10:01:18Z

+    ///     (and so is `T`, since `T: 'static` implies `T: 'a`)
+    ///   - `bar` outlives `'static`, so we know that no args are potentially live and we can return an empty set
+    ///   - `baz` has no outlives bound, so return `None` and let the caller decide what to do
+    query live_args_for_alias_from_outlives_bounds(kind: ty::AliasTyKind<'tcx>) -> &'tcx Option<ty::EarlyBinder<'tcx, Vec<ty::GenericArg<'tcx>>>> {


feel like as a query we want to just take a DefId and have a wrapper function that calls this query if necessary?

See https://github.com/rust-lang/rust/pull/156027/changes#diff-5d86c3fc35a603e984030459eb67d4870a975e53a460c331f15e48813ce8ce0eR40

The issue is that we search through the bounds on the item and need to match against the self type. Probably what this indicates is that item_bounds should potentially return something akin to ExistentialPredicate where there is no self type. But that's definitely out of scope for the current PR.

lcnr · 2026-06-26T10:04:35Z

+    // If any of the outlives bounds are `'static`, then we know the alias
+    // doesn't capture *any* regions, so we can skip visiting any regions at all.
+    //
+    // I was originally a bit concerned about something like `'a: 'static`, and
+    // whether or not we need to mark `'a` as live. I don't think *today* we do,
+    // since I think regions that outlive `'static` are special enough, but I
+    // *could* imagine some world where we need to be more careful about this.
+    // Given I can't find a test that goes wrong, I'm going to leave in this optimization.


This does really hinge on "how do liveness and outlives requirements interact", which still feels unclear to me.

I adjusted this comment a bit. I feel pretty strongly positive that we're good here.

lcnr · 2026-06-26T10:47:32Z

+        // A free region (e.g. `for<'a> T::Assoc<'a, 'b>: 'b`): no higher-ranked
+        // arg of the alias can be proven (by a caller) to outlive a free region
+        // today, and pinned args only ever match universal regions (which are
+        // always live), so we conservatively treat this as giving no
+        // restriction.
+        //
+        // NB: if we ever get implied bounds inside binders, a bound var *could*
+        // be assumed to outlive a free region (e.g. `for<'a> T::Assoc<'x, 'a>: 'x`
+        // with a declared `'b: 'a` on the assoc type implies `'a: 'x` under the
+        // binder), so this would need revisiting -- though the result would
+        // still only need to include bound-var positions.


vaguely confused by this. I feel like we would want to already consider the implied bounds from free -> bound regions here? Or well, this feels very easy to miss with -Zassumptions-on-binders

This comment just wasn't correct. The tl;dr is that we could consider fewer lifetimes as live with the free region as the outlived region. That would allow more code to compile that does today, so sticking with the conservative choice for now (i.e. don't restrict any arg liveness).

…ives_bounds for opaques

jackh726 · 2026-06-26T20:30:46Z

I am still left feeling unclear why polonius cares about liveness the way it does

Polonius cares about liveness because for an outlives relationship to be relevant at a given point, both regions must be live.

what additional constraints this puts on liveness (e.g. cases where something has to outlive 'static are fine for liveness apparently).

So, I think the key think about 'static (and any other free region) is that outlives requirements involving those must hold over the entire function.

The other question I have is whether you see this having benefits outside of this specific need of polonius? I think yes?

Yes, it does. This logic lets us consider few alias args as live outside borrowck too (which is needed to fix #42940). I have an (outdated) branch that does this: https://github.com/jackh726/rust/tree/opaque-liveness-42490

rustbot assigned lqd May 1, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 1, 2026