Eliminate lifting by nnethercote · Pull Request #155921 · rust-lang/rust

nnethercote · 2026-04-28T12:24:08Z

The Lift trait exists to work around some lifetime mismatches. But with a small number of lifetime tweaks we can eliminate it entirely. Details in individual commits.

Various places where `lift` just isn't necessary.

By moving the lifetimes from the closures to the enclosing function, we no longer need to call `lift` within them.

By using 'tcx in the right places and avoiding HRTBs, we can avoid some more lifting.

As in the previous commits, increased use of 'tcx and avoidance of HRTBs avoids the need for some lifting.

Once again, increased use of 'tcx avoids the need for some lifting.

Thanks to the previous commits, none of this code used any more and it can all be removed. Specifically: - The `Lift` trait. - The `Lift` and `Lift_Generic` proc macros, and all their uses. - The declarative macros used to impl `Lift` for various types. - The `rustc_macros::lift` and `rustc_type_ir::lift` modules. This commit also eliminates two FIXME comments.

nnethercote · 2026-04-28T12:24:28Z

@bors try @rust-timer queue

Eliminate lifting

rust-bors · 2026-04-28T14:32:26Z

☀️ Try build successful (CI)
Build commit: c6cff6a (c6cff6a9c066d48cbf8465c04a7b60ac0a4671bf, parent: 4ddb0b7f8ecda9dbdbcbc0519c9988badbd65d1c)

rust-timer · 2026-04-28T15:38:38Z

Finished benchmarking commit (c6cff6a): 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.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

Cycles

Results (secondary -2.5%)

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

Binary size

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

Bootstrap: 489.39s -> 486.341s (-0.62%)
Artifact size: 393.41 MiB -> 393.42 MiB (0.00%)

nnethercote · 2026-04-28T21:13:30Z

 /// will do all required instantiations as they run.
-///
-/// Note: the `Lift` impl is currently not used by rustc, but is used by
-/// rustc_codegen_cranelift when the `jit` feature is enabled.


@bjorn3: I think this comment is out of date? I can't find any matches for \blift or \bLift in rustc_codegen_cranelift.

View changes since the review

@bjorn3 said this on Zulip:

cg_clif used to support lazy codegen in jit mode. That lifted an Instance received from the codegeb stub to the TyCtxt to codegen using. I removed this support a while back.

So the comment is out of date and can be safely removed.

nnethercote · 2026-04-28T21:27:07Z

        let ptr = tlv.get();
        assert!(!ptr.is_null());
-        f(unsafe { *(ptr as *const &CompilerInterface<'_>) })
+        f(unsafe { *(ptr as *const &CompilerInterface<'tcx>) })


I think this change is ok but it does involve unsafe so I will highlight it.

View changes since the review

Same here, a safety comment would help!

nnethercote · 2026-04-28T23:32:27Z

r? @oli-obk

rustbot · 2026-04-28T23:32:32Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Zalathar · 2026-04-29T02:51:23Z

+pub fn with_context_opt<'a, 'tcx: 'a, F, R>(f: F) -> R
 where
-    F: for<'a, 'tcx> FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R,
+    F: FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R,


Here and in related functions, the caller gets to dictate what 'tcx is, but there's no guarantee that the thread-local context actually lives that long, which could be a soundness problem if the return type R contains things with lifetime 'tcx.

In other words, I suspect that the for<'tcx> might be load-bearing for soundness.

View changes since the review

yes, that is my conclusion, too, this is unsound. In contrast to the Symbol unsoundness, where you need to move one Symbol from a TyCtxt instance to another, here you can just stack allocate a value and call this function with the short lifetimes, and it will assume your object lifes for the entirety of TyCtxt.

I don't remember if we have code that actually causes a problem here tho... Still, we could just fix that code to not require 'tcx lifetimes

teor2345

I had a skim and this looks good, just keen to get some safety comments on the unsafe code changes.

View changes since this review

teor2345 · 2026-04-29T02:54:53Z

        // Ensure that `ImplicitCtxt` is `DynSync`.
        sync::assert_dyn_sync::<ImplicitCtxt<'_, '_>>();

        unsafe { f(Some(downcast(context))) }


As @Zalathar said:

The parts that remove for<'tcx> bounds from thread-local-context callbacks seem like the scariest bits to me, as it might be unsound to let the caller dictate what 'tcx is.

It would be great to have some safety comments both here and on downcast(). (And maybe erase().)

Separately, I'm curious how query_depth is synced between implicit and explicit contexts (if at all). But I'm not sure if removing Lift will make any existing issues worse.

teor2345 · 2026-04-29T02:55:30Z

        let ptr = tlv.get();
        assert!(!ptr.is_null());
-        f(unsafe { *(ptr as *const &CompilerInterface<'_>) })
+        f(unsafe { *(ptr as *const &CompilerInterface<'tcx>) })


Same here, a safety comment would help!

oli-obk

I think what you want is achievable, but you'd need to remove all the 'tcx lifetimes from the pretty printing infra and thus allow pretty printing arbitrary Ty<'_>, and thus subsequently are able to just not lift inside display impls while keeping the hkl on with_context_opt.

View changes since this review

oli-obk · 2026-04-29T04:51:29Z

+pub fn with_context_opt<'a, 'tcx: 'a, F, R>(f: F) -> R
 where
-    F: for<'a, 'tcx> FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R,
+    F: FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R,


yes, that is my conclusion, too, this is unsound. In contrast to the Symbol unsoundness, where you need to move one Symbol from a TyCtxt instance to another, here you can just stack allocate a value and call this function with the short lifetimes, and it will assume your object lifes for the entirety of TyCtxt.

I don't remember if we have code that actually causes a problem here tho... Still, we could just fix that code to not require 'tcx lifetimes

rustbot · 2026-04-29T04:53:55Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

nnethercote added 6 commits April 28, 2026 22:08

Remove unnecessary lift calls.

d3157c3

Various places where `lift` just isn't necessary.

Adjust lifetimes on tls::with* functions.

78d73d1

By moving the lifetimes from the closures to the enclosing function, we no longer need to call `lift` within them.

Adjust lifetimes on FmtPrinter and Print.

fc61b67

By using 'tcx in the right places and avoiding HRTBs, we can avoid some more lifting.

Adjust lifetimes on RustcInternal.

ca51624

As in the previous commits, increased use of 'tcx and avoidance of HRTBs avoids the need for some lifting.

Adjust lifetimes on Stable.

4e4a2a7

Once again, increased use of 'tcx avoids the need for some lifting.

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 Apr 28, 2026

rust-bors Bot pushed a commit that referenced this pull request Apr 28, 2026

Auto merge of #155921 - nnethercote:rm-Lift, r=<try>

c6cff6a

Eliminate lifting

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 Apr 28, 2026

nnethercote commented Apr 28, 2026

View reviewed changes

nnethercote marked this pull request as ready for review April 28, 2026 23:32

rustbot assigned oli-obk Apr 28, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 28, 2026

Zalathar reviewed Apr 29, 2026

View reviewed changes

teor2345 reviewed Apr 29, 2026

View reviewed changes

oli-obk requested changes Apr 29, 2026

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2026

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 29, 2026

Uh oh!

Conversation

nnethercote commented Apr 28, 2026

Uh oh!

nnethercote commented Apr 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Apr 28, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 28, 2026

Overall result: no relevant changes - no action needed

Uh oh!

nnethercote Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nnethercote Apr 28, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

nnethercote commented Apr 28, 2026

Uh oh!

rustbot commented Apr 28, 2026

Uh oh!

Zalathar Apr 29, 2026 • 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!

oli-obk Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teor2345 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk 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!

oli-obk Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote Apr 28, 2026 •

edited

Loading

nnethercote Apr 28, 2026 •

edited by rustbot

Loading

Zalathar Apr 29, 2026 •

edited by rustbot

Loading

oli-obk Apr 29, 2026 •

edited

Loading

teor2345 left a comment •

edited by rustbot

Loading

oli-obk left a comment •

edited by rustbot

Loading

oli-obk Apr 29, 2026 •

edited

Loading