Skip to content

Require #[pin_v2] for direct &pin borrows of ADTs#2

Merged
frank-king merged 1 commit intofrank-king:feature/pin-borrowckfrom
P8L1:enforce-pin-v2-for-direct-pin-borrows
Apr 29, 2026
Merged

Require #[pin_v2] for direct &pin borrows of ADTs#2
frank-king merged 1 commit intofrank-king:feature/pin-borrowckfrom
P8L1:enforce-pin-v2-for-direct-pin-borrows

Conversation

@P8L1
Copy link
Copy Markdown

@P8L1 P8L1 commented Apr 28, 2026

Summary

Require direct user-written &pin borrows of ADTs to use #[pin_v2].

The check runs during HIR type checking for explicit hir::BorrowKind::Pin, so it
rejects direct &pin mut / &pin const borrow expressions on non-#[pin_v2]
ADTs without affecting compiler-generated AutoBorrow::Pin adjustments or
coercions.

What changed

  • Added a direct-borrow diagnostic for attempting to directly pin an ADT that is
    not #[pin_v2].
  • Added a typeck check in FnCtxt::check_expr_addr_of for explicit user-written
    &pin borrows.
  • Reused AdtDef::is_pin_project() as the source of truth for #[pin_v2].
  • Updated existing pin ergonomics tests that should continue reaching borrowck by
    marking their local ADTs #[pin_v2].
  • Added focused UI coverage for direct &pin mut and &pin const borrows of a
    non-#[pin_v2] ADT.

Why

The active pin ergonomics borrowck PR still had an unchecked task to forbid
&pin borrows for ADTs without #[pin_v2].

Pattern projection already rejects non-#[pin_v2] ADTs in related cases, but
direct user-written &pin mut foo and &pin const foo borrow expressions were
still accepted.

This PR enforces that direct rule while avoiding compiler-generated transient pin
adjustments.

Tests

Passed:

./x test tests/ui/pin-ergonomics/direct-borrow-requires-pin-v2.rs --stage 1 --test-args --force-rerun
./x test tests/ui/pin-ergonomics/borrow.rs --stage 1 --test-args --force-rerun
./x test tests/ui/pin-ergonomics/borrow-unpin.rs --stage 1 --test-args --force-rerun
./x test tests/ui/pin-ergonomics/borrow-pinned-projection.rs --stage 1 --test-args --force-rerun
./x test tests/ui/pin-ergonomics/user-type-projection.rs --stage 1 --test-args --force-rerun
./x test tests/ui/pin-ergonomics/pin_v2-attr.rs --stage 1
./x test tests/ui/pin-ergonomics/impl-unpin.rs --stage 1
./x test tests/ui/pin-ergonomics/pattern-matching.rs --stage 1
./x test tests/ui/pin-ergonomics/pattern-matching-mix-deref-pattern.rs --stage 1
./x test tests/ui/pin-ergonomics/pin-coercion.rs --stage 1
./x test tests/ui/pin-ergonomics/pin-coercion-unpin-roundtrip.rs --stage 1
./x test tests/ui/pin-ergonomics/pin-coercion-get-mut-then-as-mut.rs --stage 1
git diff --check
./x fmt --check

Notes for reviewers

The main review questions are:

  • Should generic type parameters remain accepted by this direct ADT-only rule?
  • Should direct &pin of the Pin ADT itself have any special exception, or
    should the literal ADT rule apply?
  • Is the machine-applicable #[pin_v2] suggestion appropriate here, matching
    the existing projection diagnostic style?

Related

@P8L1
Copy link
Copy Markdown
Author

P8L1 commented Apr 28, 2026

The CI failure appears to come from src/ci/scripts/verify-channel.sh because this PR targets the feature/pin-borrowck branch rather than the normal Rust main branch.

The focused local checks passed and are listed in the PR body. This PR is intended as a patch against the active pin-borrowck feature branch for rust-lang#153693, not as an independent PR directly to main.

@frank-king
Copy link
Copy Markdown
Owner

Grateful for your help! Let's merge it first and see if the original PR can pass the CI.

@frank-king frank-king merged commit d7f4bbd into frank-king:feature/pin-borrowck Apr 29, 2026
2 of 11 checks passed
@P8L1
Copy link
Copy Markdown
Author

P8L1 commented Apr 29, 2026

I saw the CI failure in https://github.com/rust-lang/rust/actions/runs/25086407095/job/73502863053 after my merged helper PR.

I’m reproducing the exact failing pin-coercion.rs path against the current feature/pin-borrowck branch now, including the stage-2 test path, before making any further changes.

If this was caused by my patch, I’ll follow up with a focused fix PR. Sorry for the churn.

fn check_pin_borrow_of_adt_requires_pin_v2(&self, ty: Ty<'tcx>, span: Span) {
let ty = self.structurally_resolve_type(span, ty);
if let Some(adt) = ty.ty_adt_def()
&& !adt.is_pin_project()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having read the failed CI cases thoroughly, instead of directly reporting cannot directly pin an ADT that is not `#[pin_v2]` , I now prefer checking if ADT: Unpin first and report this error only for ADTs that are !Unpin.

struct Foo; // no `#[pin_v2]`, but `Foo: Unpin`

// For `Foo: Unpin`, given that `&pin mut Foo` and `&mut Foo` are mutually coercible,
// I don't think it is reasonable to allow `&mut Foo` but not `&pin mut Foo` here.
let mut x: Pin<&mut _> = &pin mut Foo;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a follow-up PR for the Unpin gate change here: #4

It keeps the diagnostic limited to explicit &pin borrows, but only emits it for non-#[pin_v2] ADTs that are known/proven to be !Unpin.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this behavior? In case that pinned borrow may be inconsistent with the pinned patterns.

pub struct NotUnpin(PhantomPinned);

pub struct GenericAdt<T> {
    pub x: T,
}

// An ill-formed unconditional `Unpin` for `GenericAdt<T>`.
impl<T> Unpin for GenericAdt<T> {}

pub fn unsound_pin_borrow<T>(
    input: &pin mut GenericAdt<T>,
) -> &pin mut T {
    // This should be an error, or else you can get an `&pin mut NotUnpin`
    // and later an `&mut NotUnpin` via `&mut (*input).x`
    // as `GenericAdt<T>` is `Unpin`.
    &pin mut input.x
}

let mut g = GenericAdt { x: NotUnpin(PhantomPinned) };
let f = &pin mut g; // Ok because of your new added rule.
let x: &pin mut NotUnpin = unsound_pin_borrow(f);
// This will be rejected in pinned patterns checks.
// let ref pin mut GenericAdt { x } = f; 

drop(f); // Kills f, but the pinnedness of x won't expire.

let f: &mut _ = f; // Allowed by coercion.
let x: &mut NotUnpin = &mut f.x; // Unsound! Breaks `Pin`' s safety contract.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think allowing &pin borrows for GenericAdt<T> is unsound if it is unknown to be Unpin or !Unpin.

struct NotUnpin(PhantomPinned);

struct GenericAdt<T>(T);

impl Drop for GenericAdt<T> {
    // Anyway, an `&mut GenericAdt<T>` is obtained.
    fn drop(&mut self) { /* ... */ }
}

fn foo(mut input: GenericAdt<NotUnpin>) {
    let x: &pin mut _ = &pin mut input; // Allowed by your new rule
    // At `<GenericAdt<NotUnpin> as Drop>::drop`, it obtains an
    // `&mut GenericAdt<NotUnpin>`, which breaks `Pin`'s safety contract.
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

Refer to: #4

Direct &pin no longer relies only on the outer type satisfying Unpin. Generic ADTs are now rejected unless they are #[pin_v2], and generic/unknown field targets like T are rejected instead of being accepted conservatively.

This prevents a manual impl<T> Unpin for Wrapper<T> from bypassing structural pin-safety restrictions and keeps the direct &pin borrow path aligned with pinned-pattern projection safety.

I added regression coverage for the generic field projection case, the unconditional manual impl<T> Unpin wrapper, and the generic Drop wrapper. I also updated the diagnostic wording from “ADT” to “type” because one rejected case can target a generic parameter rather than an ADT directly.

Tested with the focused pin-ergonomics suite, tidy, fmt --check, and git diff --check. I did not run the full tests/ui suite locally because it is too broad/expensive, but the focused pin-ergonomics suite passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants