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 ↩

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.

There should no difference between AliasTyKind and DefId wrt what information they give you. Both are just a DefId, where AliasTyKind also has some information about DefKind in there.

Reconstructing an AliasTy from a DefId is intentionally vaguely annoying 🤔 even though I'd expect that the query system is significantly better at dealing with raw DefIds I guess it's whatever whether to use AliasTy or DefId here

I'm going to leave this as AliasTyKind for now. It's really not clear to me how to make this use DefId correctly: both AliasTyKind::Projection and AliasTyKind::Inherent use DefKind::AssocTy, so I can't disambiguate on that; and, I can't immediately think of what else I would use here. And, passing AliasTy could be okay because it would be item-self generics, but that feels like a recipe for disaster: potentially mitigated with something like EarlyBinder<AliasTy>, but that's really ugly and still prone to failure.

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

lcnr

I still feel unresolved wrt the why of this change, which makes reviewing the concrete behavior kind of unpleasant. gonna spend some time looking at the non-polonius thing this allows us to fix?

View changes since this review

lcnr · 2026-07-03T14:59:33Z

+    ///     (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>>>> {


There should no difference between AliasTyKind and DefId wrt what information they give you. Both are just a DefId, where AliasTyKind also has some information about DefKind in there.

Reconstructing an AliasTy from a DefId is intentionally vaguely annoying 🤔 even though I'd expect that the query system is significantly better at dealing with raw DefIds I guess it's whatever whether to use AliasTy or DefId here

lcnr · 2026-07-03T15:12:15Z

+    let parent_param_env = tcx.param_env(parent_def_id);
+    tracing::debug!(?parent_param_env);
+
+    // Map the outlives regions to the parent regions


I mean concretely for this loop 🤔 this is a lot of nothing which feels unrelated to the actual semantics of this function?

Like, we take self_identity_args and give back a parent_outlives_regions. I do not understand why this is necessary or what exactly it even does. What does parent_outlives_regions represent, why do we need to do this etc?

…arams, use tcx.assumed_wf_types, and test polonius_alpha in gat-outlives-implied test

lqd · 2026-07-09T15:54:19Z

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

NLLs introduced liveness to allow invalidations on dead loans, and its regions are sets of CFG points. The points where the regions are live, the regions they outlive (in a flow-insensitive manner) and vice-versa, and so on are used to compute the CFG points that represent regions. A loan is considered live at a point if the region where it was introduced contains this point (modulo kills, unimportant here).

To support flow-sensitivity, polonius regions are sets of loans, and loan liveness needs to track the outlives relationships between regions more precisely: a loan is considered live if it is contained in a live region. Polonius thus needs the accurate liveness of regions to check if an invalidation is indeed an error. As an NLL superset, if a loan is contained in a dead region and is invalidated, polonius will accept that as well. If the region is unexpectedly dead like in the #153215 examples, this is of course a soundness issue, and this PR fixes the current inaccuracy in liveness for some regions related to aliases.

The approach makes sense to me, and we've talked about it with jack and niko, and despite the complexity I think this is an improvement we need. There may be ways to simplify in the near future, a part of it is caused by the mapping and our current encoding of opaque types, etc. lcnr has told jack that we can land this if I'm also happy with it, and I am. It does get us closer to enabling polonius alpha on nightly.

If there are more improvements to do after this handful of review rounds, I think it's fine to do them in follow-ups.

Let's do this 🚀, @bors r+

rust-bors · 2026-07-09T15:54:22Z

📌 Commit 055af58 has been approved by lqd

It is now in the queue for this repository.

Consider captured regions for opaque type region liveness. Fixes rust-lang#153215 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

…uwer Rollup of 24 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #156027 (Consider captured regions for opaque type region liveness.) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #157561 (rustdoc: do not include extra stuff in span) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158384 (Allow BackwardIncompatibleDropHint in polonius legacy) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158877 (borrowck: Keep returned `path` from `best_blame_constraint()` consistent) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158995 (Use REST API in linkchecker script) - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes)

Consider captured regions for opaque type region liveness. Fixes rust-lang#153215 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

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

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 1, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request May 1, 2026

Auto merge of #156027 - jackh726:opaque-liveness, r=<try>

97f0332

Consider captured regions for opaque type region liveness.

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 1, 2026

lqd reviewed May 4, 2026

View reviewed changes

lcnr self-assigned this May 5, 2026

This comment has been minimized.

Sign in to view

rust-cloud-vms Bot force-pushed the opaque-liveness branch from 0718cad to ca97579 Compare June 2, 2026 11:34

jackh726 commented Jun 3, 2026

View reviewed changes

Comment thread compiler/rustc_trait_selection/src/traits/outlives_for_liveness.rs Outdated

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jun 3, 2026

Auto merge of #156027 - jackh726:opaque-liveness, r=<try>

8e819ff

Consider captured regions for opaque type region liveness.

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2026

craterbot removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jun 7, 2026

lcnr reviewed Jun 26, 2026

View reviewed changes

Comment thread tests/ui/borrowck/alias-liveness/gat-outlives-implied.rs

lcnr mentioned this pull request Jun 26, 2026

ACP: Option::insert_or_unify rust-lang/libs-team#822

Closed

lcnr reviewed Jun 26, 2026

View reviewed changes

jackh726 added 4 commits June 26, 2026 15:04

Move some tests around for alias-liveness

9abec7c

Consider captured regions for opaque type region liveness.

4e17689

Also extend liveness rules for non-opaque aliases

9ab5a94

Move opaque outlives computation out of live_args_for_alias_from_outl…

cb8b147

…ives_bounds for opaques

rust-cloud-vms Bot force-pushed the opaque-liveness branch from c5d5540 to 39ae191 Compare June 26, 2026 20:17

Address review

b8c8868

rust-cloud-vms Bot force-pushed the opaque-liveness branch from 39ae191 to b8c8868 Compare June 26, 2026 20:31

This was referenced Jun 27, 2026

Nested alias bounds are not considered when collecting free regions for liveness analysis #158463

Open

Stabilize and model Polonius Alpha rust-lang/rust-project-goals#118

Open

lcnr reviewed Jul 3, 2026

View reviewed changes

rust-cloud-vms Bot force-pushed the opaque-liveness branch from bf2a68f to 203af9a Compare July 3, 2026 17:07

This comment has been minimized.

Sign in to view

Rename variable and add comment

1912e4f

rust-cloud-vms Bot force-pushed the opaque-liveness branch from 203af9a to 1912e4f Compare July 3, 2026 17:14

This comment was marked as outdated.

Sign in to view

jackh726 added 2 commits July 9, 2026 00:01

A couple minor things: rename args_known_to_outlive_associated_type_p…

ed9bfaf

…arams, use tcx.assumed_wf_types, and test polonius_alpha in gat-outlives-implied test

Remove compile-fail

055af58

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2026

JonathanBrouwer mentioned this pull request Jul 9, 2026

Rollup of 24 pull requests #159037

Closed

jhpratt mentioned this pull request Jul 9, 2026

Rollup of 25 pull requests #159045

Open

Uh oh!

Uh oh!

Conversation

jackh726 commented May 1, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lqd commented May 1, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented May 1, 2026

Uh oh!

This comment has been minimized.

rust-timer commented May 1, 2026

Overall result: no relevant changes - no action needed

Uh oh!

lqd left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Uh oh!

jackh726 commented Jun 3, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Jun 3, 2026

Uh oh!

jackh726 commented Jun 3, 2026

Uh oh!

craterbot commented Jun 3, 2026

Uh oh!

jackh726 commented Jun 4, 2026

Uh oh!

craterbot commented Jun 4, 2026

Uh oh!

craterbot commented Jun 5, 2026

Uh oh!

craterbot commented Jun 7, 2026

Footnotes

Uh oh!

Uh oh!

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jackh726 commented Jun 26, 2026

Uh oh!

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jackh726 commented May 1, 2026 •

edited by rustbot

Loading

lqd left a comment •

edited by rustbot

Loading

lcnr left a comment •

edited by rustbot

Loading

lcnr left a comment •

edited by rustbot

Loading