Skip to content

Remove impl PinCoerceUnsized for Pin #144896

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.

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

Closed
wants to merge 1 commit into from

Conversation

Darksonn
Copy link
Contributor

@Darksonn Darksonn commented Aug 4, 2025

The safety requirements for PinCoerceUnsized are essentially that the type does not have a malicious Deref or DerefMut impl. However, the Pin type is fundamental, so the end-user can provide their own implementation of DerefMut for Pin<&SomeLocalType>, so it's possible for Pin to have a malicious DerefMut impl.

Thus, remove the unsound implementation of PinCoerceUnsized for the Pin type.

This PR is a breaking change, but it fixes #85099.

@rustbot
Copy link
Collaborator

rustbot commented Aug 4, 2025

r? @fee1-dead

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

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 4, 2025
@Darksonn Darksonn added the A-pin Area: Pin label Aug 4, 2025
@Darksonn
Copy link
Contributor Author

Darksonn commented Aug 4, 2025

Note that all other fundamental types that implement PinCoerceUnsized prevent this unsoundness by having an unconditional impl of either !DerefMut or DerefMut, both of which prevent the end user from providing an unsound impl of DerefMut.

Note that this also means that making Arc or Rc fundamental would be unsound unless an impl of !DerefMut is added.

@fee1-dead
Copy link
Member

r? libs

@bors2 try

@rustbot rustbot assigned thomcc and unassigned fee1-dead Aug 5, 2025
rust-bors bot added a commit that referenced this pull request Aug 5, 2025
Remove impl `PinCoerceUnsized` for `Pin`
@rust-bors
Copy link

rust-bors bot commented Aug 5, 2025

⌛ Trying commit f360584 with merge e658ebe

To cancel the try build, run the command @bors try cancel.

@rust-bors
Copy link

rust-bors bot commented Aug 5, 2025

☀️ Try build successful (CI)
Build commit: e658ebe (e658ebe4ffa1b515cd2ef06dd428d0d063e2ae6b, parent: 0f353363965ebf05e0757f7679c800b39c51a07e)

@fee1-dead
Copy link
Member

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-144896 created and queued.
🤖 Automatically detected try build e658ebe
🔍 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

@craterbot 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 Aug 5, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-144896 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-144896 is completed!
📊 2 regressed and 7 fixed (676659 total)
📰 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

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Aug 6, 2025
@Darksonn
Copy link
Contributor Author

Darksonn commented Aug 8, 2025

Seems like there are no real regressions. The openssl one it lists seems to be unrelated.

@Darksonn Darksonn added T-lang Relevant to the language team T-libs-api Relevant to the library API 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. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 13, 2025
@Darksonn
Copy link
Contributor Author

@rustbot label +I-lang-nominated

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Aug 13, 2025
@traviscross traviscross added the P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang label Aug 13, 2025
@traviscross
Copy link
Contributor

We talked about this in the lang call. Why does this remove the impl rather than taking one of the approaches that had seemed more favored in the discussion in #85099?

@Darksonn
Copy link
Contributor Author

You're right. Looking at this again I think the right solution is preventing the end-user from implementing DerefMut. That would make the impl of PinCoerceUnsized follow the safety requirements.

@Darksonn Darksonn closed this Aug 14, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 14, 2025
@traviscross traviscross removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pin Area: Pin T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team T-libs-api Relevant to the library API 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
Development

Successfully merging this pull request may close these issues.

A Pin unsoundness involving an impl DerefMut for Pin<&dyn LocalTrait>
6 participants