Skip to content

Forbid manual Unpin impls for structurally pinned types #149263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rust-bors merged 1 commit into from
Feb 3, 2026
Merged

Forbid manual Unpin impls for structurally pinned types #149263

rust-bors merged 1 commit into from
Feb 3, 2026
+195 −0

Conversation

@frank-king
Copy link
Contributor

@frank-king frank-king commented Nov 24, 2025

Part of pin_ergonomics. It forbids to impl Unpin for T where T is an ADT marked with #[pin_v2].

@rustbot 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 Nov 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 24, 2025

r? @madsmtm

rustbot has assigned @madsmtm.
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

@madsmtm
Copy link
Contributor

madsmtm commented Dec 4, 2025 •
edited
Loading

The change itself seems simple, but I'd prefer to have someone from t-types review it.
r? types

If no-one there feels like they have the capacity, feel free to assign it back to me.

@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Dec 4, 2025
@rustbot rustbot assigned jackh726 and unassigned madsmtm Dec 4, 2025
@bors
Copy link
Collaborator

bors commented Dec 14, 2025

☔ The latest upstream changes (presumably #146348) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

This comment has been minimized.

Sign in to view
@frank-king frank-king mentioned this pull request Dec 18, 2025
Open
4 tasks
@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 7, 2026

r? BoxyUwU

@rustbot rustbot assigned BoxyUwU and unassigned jackh726 Jan 7, 2026
BoxyUwU
BoxyUwU reviewed Jan 7, 2026
View reviewed changes
@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 7, 2026

What's the reason for wanting to forbid such impls? Is it just because it's probably pointless to have pin_v2 on an Unpin type, or is there some soundness concern here?

@frank-king
Copy link
Contributor Author

frank-king commented Jan 7, 2026 •
edited
Loading

It is for soundness concern.

For a type T annotated with #[pin_v2], it is allowed to project Pin<&mut T> to its field Pin<&mut U> safely (even if U: !Unpin). If T is allowed to impl Unpin manually, then &mut U could be obtained from &mut T that dereferenced by Pin<&mut T>, which breaks the safety contract of Pin<&mut U> for U: !Unpin.

@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 7, 2026 •
edited
Loading

If its soundness critical, I would add tests for impls of unpin for type aliases for adts marked pin_v2, e.g. impl Unpin for <Foo as Identity>::Assoc impl Unpin for Identity<Foo> (with lazy_type_alias feature enabled) and impl Unpin for TAIT (where TAIT is Foo).

rn your logic implicitly ignores all the cases where the self type is a non-adt 🤔 which I think is fine because we enforce that impls of Unpin are always for ADTs? But having tests is good :) It would probably also be good to explicitly match on the TyKind and in non Adt cases (which we expect to be unreachable) delay a bug maybe 🤔 that way you don't get unsoundness here if it goes wrong just an ICE

@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 19, 2026

@rustbot author

@rustbot 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 Jan 19, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 19, 2026

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

@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2026

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.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@frank-king
Copy link
Contributor Author

frank-king commented Jan 31, 2026

@rustbot ready

@rustbot 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 Jan 31, 2026
@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 3, 2026

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 3, 2026

📌 Commit e12acb9 has been approved by BoxyUwU

It is now in the queue for this repository.

@rust-bors 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 Feb 3, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 3, 2026
@JonathanBrouwer
Rollup merge of rust-lang#149263 - frank-king:feature/unpin-check, r=…
d2f1f95 
…BoxyUwU

Forbid manual `Unpin` impls for structurally pinned types

Part of [`pin_ergonomics`](rust-lang#130494). It forbids to `impl Unpin for T` where `T` is an ADT marked with `#[pin_v2]`.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 3, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 3, 2026
@JonathanBrouwer
Rollup merge of rust-lang#149263 - frank-king:feature/unpin-check, r=…
d7e48e4 
…BoxyUwU

Forbid manual `Unpin` impls for structurally pinned types

Part of [`pin_ergonomics`](rust-lang#130494). It forbids to `impl Unpin for T` where `T` is an ADT marked with `#[pin_v2]`.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 3, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 3, 2026
@JonathanBrouwer
Rollup merge of rust-lang#149263 - frank-king:feature/unpin-check, r=…
9c7e49e 
…BoxyUwU

Forbid manual `Unpin` impls for structurally pinned types

Part of [`pin_ergonomics`](rust-lang#130494). It forbids to `impl Unpin for T` where `T` is an ADT marked with `#[pin_v2]`.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 3, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 3, 2026
@JonathanBrouwer
Rollup merge of rust-lang#149263 - frank-king:feature/unpin-check, r=…
ac613ed 
…BoxyUwU

Forbid manual `Unpin` impls for structurally pinned types

Part of [`pin_ergonomics`](rust-lang#130494). It forbids to `impl Unpin for T` where `T` is an ADT marked with `#[pin_v2]`.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 3, 2026
Closed
rust-bors bot pushed a commit that referenced this pull request Feb 3, 2026
@bors
Auto merge of #152055 - JonathanBrouwer:rollup-ui1ex7W, r=JonathanBro…
2ee04fa 
…uwer

Rollup of 13 pull requests

Successful merges:

 - #152034 (Show largest job duration changes in hours and minutes in the post-merge report)
 - #152039 (coverage: Add a test case for a previously-unknown span mismatch)
 - #152045 (Convert to inline diagnostics in `rustc_infer`)
 - #149263 (Forbid manual `Unpin` impls for structurally pinned types)
 - #151754 (Check proj's parent is trait or not when checking dyn compatibility)
 - #151848 (Port `rustc_mir` to attribute parser)
 - #151874 (error on unsized AnonConsts)
 - #151944 (Convert to inline diagnostics in `rustc_attr_parsing`)
 - #152012 (Use `DEVELOPER_DIR` instead of a custom `xcode-select` script)
 - #152019 (`NativeLib` cleanups)
 - #152029 (disable socket tests in Miri)
 - #152041 (Convert to inline diagnostics in `rustc_query_system`)
 - #152050 (Try to fix `rustdoc-gui/globals.goml` flakyness)

Failed merges:

 - #152046 (Use glob imports for attribute parsers)
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 3, 2026
Merged
rust-bors bot pushed a commit that referenced this pull request Feb 3, 2026
@bors
Auto merge of #152063 - JonathanBrouwer:rollup-EwTnVaj, r=JonathanBro…
01572a2 
…uwer

Rollup of 12 pull requests

Successful merges:

 - #152034 (Show largest job duration changes in hours and minutes in the post-merge report)
 - #152039 (coverage: Add a test case for a previously-unknown span mismatch)
 - #152045 (Convert to inline diagnostics in `rustc_infer`)
 - #149263 (Forbid manual `Unpin` impls for structurally pinned types)
 - #151754 (Check proj's parent is trait or not when checking dyn compatibility)
 - #151848 (Port `rustc_mir` to attribute parser)
 - #151874 (error on unsized AnonConsts)
 - #151944 (Convert to inline diagnostics in `rustc_attr_parsing`)
 - #152019 (`NativeLib` cleanups)
 - #152029 (disable socket tests in Miri)
 - #152041 (Convert to inline diagnostics in `rustc_query_system`)
 - #152050 (Try to fix `rustdoc-gui/globals.goml` flakyness)

Failed merges:

 - #152046 (Use glob imports for attribute parsers)
rust-bors bot pushed a commit that referenced this pull request Feb 3, 2026
@bors
Auto merge of #152063 - JonathanBrouwer:rollup-EwTnVaj, r=JonathanBro…
366a1b9 
…uwer

Rollup of 12 pull requests

Successful merges:

 - #152034 (Show largest job duration changes in hours and minutes in the post-merge report)
 - #152039 (coverage: Add a test case for a previously-unknown span mismatch)
 - #152045 (Convert to inline diagnostics in `rustc_infer`)
 - #149263 (Forbid manual `Unpin` impls for structurally pinned types)
 - #151754 (Check proj's parent is trait or not when checking dyn compatibility)
 - #151848 (Port `rustc_mir` to attribute parser)
 - #151874 (error on unsized AnonConsts)
 - #151944 (Convert to inline diagnostics in `rustc_attr_parsing`)
 - #152019 (`NativeLib` cleanups)
 - #152029 (disable socket tests in Miri)
 - #152041 (Convert to inline diagnostics in `rustc_query_system`)
 - #152050 (Try to fix `rustdoc-gui/globals.goml` flakyness)

Failed merges:

 - #152046 (Use glob imports for attribute parsers)
@rust-bors rust-bors bot merged commit b3b66b2 into rust-lang:main Feb 3, 2026
11 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Feb 3, 2026
rust-timer added a commit that referenced this pull request Feb 3, 2026
@rust-timer
Unrolled build for #149263
95dcb3b 
Rollup merge of #149263 - frank-king:feature/unpin-check, r=BoxyUwU

Forbid manual `Unpin` impls for structurally pinned types

Part of [`pin_ergonomics`](#130494). It forbids to `impl Unpin for T` where `T` is an ADT marked with `#[pin_v2]`.
@JonathanBrouwer
Copy link
Contributor

JonathanBrouwer commented Feb 4, 2026

@rust-timer build 95dcb3b

@rust-timer
Copy link
Collaborator

rust-timer commented Feb 4, 2026

Queued 95dcb3b with parent 55407b8, future comparison URL.
There is currently 1 preceding artifact in the queue.
It will probably take at least ~2.0 hours until the benchmark run finishes.

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

Reviewers

@BoxyUwU BoxyUwU BoxyUwU left review comments

Assignees

@BoxyUwU BoxyUwU

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.95.0

Development

Successfully merging this pull request may close these issues.

9 participants

@frank-king @rustbot @madsmtm @bors @BoxyUwU @rust-log-analyzer @JonathanBrouwer @rust-timer @jackh726