UnsafePinned: implement coroutine lowering changes by b-naber · Pull Request #152946 · rust-lang/rust

b-naber · 2026-02-21T20:49:42Z

Implements the lowering part for #125735

This introduces a visitor that looks for self-referential fields in the coroutine struct and wraps all of these types into UnsafePinned. I've also included a commit where we change the condition for using noalias. That change is based on my understanding of the RFC, but is likely wrong given the change in #151365. I've included it for now to further discuss this.

cc @RalfJung

rustbot · 2026-02-21T20:49:45Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-02-21T20:49:48Z

r? @mati865

rustbot has assigned @mati865.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler, mir, mir-opt
compiler, mir, mir-opt expanded to 68 candidates
Random selection from 15 candidates

@b-naber

…rk-Simulacrum extend unpin noalias tests to cover mutable references rust-lang#152946 made a change to the logic for this attribute that the test should have flagged as problematic -- but the test only checked `Box`, not `&mut`, and those have independent code paths. So extend the test to also cover `&mut`. @b-naber would be nice if you could confirm that the added tests do fail with your PR.

@b-naber

Rollup merge of #152970 - RalfJung:unpin-noalias-tests, r=Mark-Simulacrum extend unpin noalias tests to cover mutable references #152946 made a change to the logic for this attribute that the test should have flagged as problematic -- but the test only checked `Box`, not `&mut`, and those have independent code paths. So extend the test to also cover `&mut`. @b-naber would be nice if you could confirm that the added tests do fail with your PR.

mati865 · 2026-02-23T13:09:15Z

I'm not a good fit for this code, perhaps r? @RalfJung ?

Feel free to reroll

rustbot · 2026-02-23T13:09:19Z

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

RalfJung · 2026-02-23T13:57:00Z

Definitely not, I don't know the coroutine lowering code at all.

@rustbot reroll

b-naber · 2026-02-23T14:28:42Z

@cjgillot Could you maybe review this?

rustbot · 2026-02-23T17:46:27Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@b-naber

…crum extend unpin noalias tests to cover mutable references rust-lang/rust#152946 made a change to the logic for this attribute that the test should have flagged as problematic -- but the test only checked `Box`, not `&mut`, and those have independent code paths. So extend the test to also cover `&mut`. @b-naber would be nice if you could confirm that the added tests do fail with your PR.

b-naber · 2026-03-06T12:25:20Z

@rustbot author

RalfJung · 2026-03-18T21:03:30Z

That test failing is a great sign! Quoting from the test description

//! FIXME: This test should pass! However, `async fn` does not yet use `UnsafePinned`.
//! This is a regression test for <https://github.com/rust-lang/rust/issues/137750>:
//! `UnsafePinned` must include the effects of `UnsafeCell`.

rustbot · 2026-03-18T21:38:18Z

The Miri subtree was changed

cc @rust-lang/miri

RalfJung · 2026-03-18T21:53:52Z

-//! FIXME: This test should pass! However, `async fn` does not yet use `UnsafePinned`.
-//! This is a regression test for <https://github.com/rust-lang/rust/issues/137750>:
-//! `UnsafePinned` must include the effects of `UnsafeCell`.
+// run-pass


This comment does nothing, please remove it. (Why did you even add this? Even in the rustc ui test suite this would do nothing. It's spelled // @run-pass there. But the Miri test suite is a little different.)

There's another test that uses this: https://github.com/rust-lang/rust/blob/main/src/tools/miri/tests/pass/packed-struct-dyn-trait.rs, which is why I used this.

Ah, not sure how that snuck in, thanks for pointing that out. But you'll notice most pass tests don't have such an annotation.

rustbot · 2026-03-21T12:22:15Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

b-naber · 2026-03-21T14:32:27Z

@rustbot ready

Zoxc · 2026-05-10T14:19:38Z

+
+        borrowed_locals_cursor2.seek_before_primary_effect(loc);
+        let borrowed_locals = borrowed_locals_cursor2.get();
+        locals_live_due_to_borrow.union(borrowed_locals);


Wouldn't it make sense to update locals_live_due_to_borrow inside !movable? As the movable case can't have self-referential fields.

View changes since the review

Zoxc · 2026-05-10T14:50:26Z

I am not making much sense of find_pinned_self_referential_fields. Do you have an simple example where it would be useful? And why removing UnsafePinned would be safe for that case?

cjgillot · 2026-05-23T22:06:25Z

+        let mut projection = base.projection.to_vec();
+        // self.field (UnsafePinned<T>)
+        projection.push(ProjectionElem::Field(idx, ty));
+        // value (UnsafeCell<T>)
+        projection.push(ProjectionElem::Field(FieldIdx::from_u32(0), unsafe_cell_ty));
+        // value (T)
+        projection.push(ProjectionElem::Field(FieldIdx::from_u32(0), original_ty));
+
+        let place = Place { local: base.local, projection: self.tcx.mk_place_elems(&projection) };


Let's only intern one set of projections.

Suggested change

let mut projection = base.projection.to_vec();

// self.field (UnsafePinned<T>)

projection.push(ProjectionElem::Field(idx, ty));

// value (UnsafeCell<T>)

projection.push(ProjectionElem::Field(FieldIdx::from_u32(0), unsafe_cell_ty));

// value (T)

projection.push(ProjectionElem::Field(FieldIdx::from_u32(0), original_ty));

let place = Place { local: base.local, projection: self.tcx.mk_place_elems(&projection) };

let place = Place {

local: SELF_ARG,

projection: self.tcx.mk_place_elems(&[

// self as variant

ProjectionElem::Variant(None, variant_index),

// self.field (UnsafePinned<T>)

ProjectionElem::Field(idx, ty),

// value (UnsafeCell<T>)

ProjectionElem::Field(FieldIdx::from_u32(0), unsafe_cell_ty),

// value (T)

ProjectionElem::Field(FieldIdx::from_u32(0), original_ty),

]),

};

View changes since the review

cjgillot · 2026-05-23T22:06:58Z

+            storage_conflicts,
+            storage_liveness: storage_liveness_map,
+        },
+        self_referential_fields,


Why not make this a field inside LivenessInfo?

View changes since the review

cjgillot · 2026-05-23T22:07:55Z


+    // Every Local that is live due to an outstanding borrow is a self-referential field of
+    // the coroutine struct.
+    let mut locals_live_due_to_borrow = DenseBitSet::new_empty(body.local_decls.len());


Why not call it self_referential_fields directly?

View changes since the review

cjgillot · 2026-05-24T08:55:53Z

+struct PinnedLocalsFinder {
+    pin_fn: DefId,
+    pinned_locals: Vec<(Local, Location)>,
+}
+
+impl PinnedLocalsFinder {
+    fn new(pin_fn: DefId) -> Self {
+        PinnedLocalsFinder { pin_fn, pinned_locals: vec![] }
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for PinnedLocalsFinder {
+    fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        match &terminator.kind {
+            TerminatorKind::Call { func, args, .. } => {
+                if let Some((fn_def, _)) = func.const_fn_def() {
+                    if fn_def == self.pin_fn {
+                        assert!(args.len() == 1);
+                        match args[0].node {
+                            Operand::Move(pinned_place) => {
+                                self.pinned_locals.push((pinned_place.local, location));
+                            }
+                            _ => bug!("expected Operand::Move for argument to Pin::new_unchecked"),
+                        }
+                    }
+                }
+            }
+            _ => {}
+        }
+        self.super_terminator(terminator, location);
+    }
+}


A loop over basic blocks inside find_locals_needing_unsafe_pinned will be more concise and easier to understand.

View changes since the review

cjgillot · 2026-05-24T08:57:08Z

+                            Operand::Move(pinned_place) => {
+                                self.pinned_locals.push((pinned_place.local, location));
+                            }
+                            _ => bug!("expected Operand::Move for argument to Pin::new_unchecked"),


Please avoid asserting too much about what happens to MIR before this pass. The operand could get demoted to a Copy and it will work just as well. And we could get a constant (why not), and this method would do nothing.

View changes since the review

cjgillot · 2026-05-24T09:02:49Z

 }
+
+/// Finds all `Local`s that need to be wrapped in `UnsafePinned`. These are all self-referential
+/// fields of the coroutine struct which aren't pinned.


Do you mind expanding a bit? What kind of MIR pattern are you looking for?

View changes since the review

cjgillot · 2026-05-24T09:03:30Z

+                for &pred in body.basic_blocks.predecessors()[bb].iter() {
+                    let pred_last_stmt_idx = body.basic_blocks[pred].statements.len();
+                    worklist.push((pred, pred_last_stmt_idx, tracked_local));
+                }


What if this block has 2 predecessors, each borrowing a different local?

View changes since the review

cjgillot · 2026-05-24T09:04:52Z

+
+    let pinned_targets =
+        find_pinned_self_referential_fields(body, &pinned_locals[..], &self_referential_fields);
+    debug!(?pinned_targets);


IIUC, pinned_targets is the set of locals l for which we find a Pin::new_unchecked(&mut l) in MIR.
What if such l is directly accessed in another part of MIR?

View changes since the review

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 Feb 21, 2026

rustbot assigned mati865 Feb 21, 2026

This comment has been minimized.

Sign in to view

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from 50482bd to b226e46 Compare February 21, 2026 20:58

RalfJung reviewed Feb 22, 2026

View reviewed changes

Comment thread compiler/rustc_ty_utils/src/abi.rs Outdated

RalfJung reviewed Feb 22, 2026

View reviewed changes

Comment thread compiler/rustc_ty_utils/src/abi.rs Outdated

RalfJung mentioned this pull request Feb 22, 2026

extend unpin noalias tests to cover mutable references #152970

Merged

rustbot assigned RalfJung and unassigned mati865 Feb 23, 2026

rustbot assigned adwinwhite and unassigned RalfJung Feb 23, 2026

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from b226e46 to d8b3c3d Compare February 23, 2026 17:46

This comment has been minimized.

Sign in to view

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from d8b3c3d to 3d40415 Compare February 23, 2026 20:28

cjgillot self-assigned this Feb 24, 2026

adwinwhite removed their assignment Mar 4, 2026

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

This comment has been minimized.

Sign in to view

RalfJung reviewed Mar 18, 2026

View reviewed changes

Comment thread src/tools/miri/tests/fail/async-shared-mutable.rs

RalfJung reviewed Mar 18, 2026

View reviewed changes

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch 2 times, most recently from 3733432 to 4998cf2 Compare March 18, 2026 22:29

This comment has been minimized.

Sign in to view

rustbot added the T-clippy Relevant to the Clippy team. label Mar 21, 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 Mar 21, 2026

RalfJung mentioned this pull request Apr 14, 2026

Per-byte handling for UnsafePinned rust-lang/miri#4954

Closed

This comment has been minimized.

Sign in to view

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from f5e7edd to 3f2a21b Compare May 8, 2026 17:40

This comment has been minimized.

Sign in to view

b-naber added 2 commits May 8, 2026 17:58

use UnsafePinned for self-referential fields in coroutine struct

d407c7f

add llvm filecheck tests

9ae0516

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from 3f2a21b to 8129bdb Compare May 8, 2026 17:58

This comment has been minimized.

Sign in to view

b-naber added 4 commits May 8, 2026 18:12

add mir-opt test

332b2b0

bless tests

fadf568

update miri async-shared-mutable test for unsafepinned

1554b20

account for UnsafePinned in AwaitHolding clippy lint

4df37a6

rust-cloud-vms Bot force-pushed the coroutine-unsafe-pinned branch from 8129bdb to 4df37a6 Compare May 8, 2026 18:12

Zoxc reviewed May 10, 2026

View reviewed changes

cjgillot reviewed May 23, 2026

View reviewed changes

cjgillot reviewed May 24, 2026

View reviewed changes

Uh oh!

Conversation

b-naber commented Feb 21, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 21, 2026

Uh oh!

rustbot commented Feb 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

mati865 commented Feb 23, 2026

Uh oh!

rustbot commented Feb 23, 2026

Uh oh!

RalfJung commented Feb 23, 2026

Uh oh!

b-naber commented Feb 23, 2026

Uh oh!

rustbot commented Feb 23, 2026

Uh oh!

This comment has been minimized.

b-naber commented Mar 6, 2026

Uh oh!

This comment has been minimized.

RalfJung commented Mar 18, 2026

Uh oh!

rustbot commented Mar 18, 2026

Uh oh!

Uh oh!

RalfJung Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

b-naber Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rustbot commented Mar 21, 2026

Uh oh!

b-naber commented Mar 21, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Zoxc May 10, 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!

Zoxc commented May 10, 2026

Uh oh!

cjgillot May 23, 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!

cjgillot May 23, 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!

cjgillot May 23, 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!

cjgillot May 24, 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!

cjgillot May 24, 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!

cjgillot May 24, 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!

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

Uh oh!

b-naber commented Feb 21, 2026 •

edited by rustbot

Loading

Zoxc May 10, 2026 •

edited by rustbot

Loading

cjgillot May 23, 2026 •

edited by rustbot

Loading

cjgillot May 23, 2026 •

edited by rustbot

Loading

cjgillot May 23, 2026 •

edited by rustbot

Loading

cjgillot May 24, 2026 •

edited by rustbot

Loading

cjgillot May 24, 2026 •

edited by rustbot

Loading

cjgillot May 24, 2026 •

edited by rustbot

Loading

cjgillot May 24, 2026 •

edited by rustbot

Loading

cjgillot May 24, 2026 •

edited by rustbot

Loading