Make PinCoerceUnsized require Deref

[theemathas]

Also, delete impls on non-Deref types.

Pin doesn't do anything useful for non-Deref types, so PinCoerceUnsized on such types makes no sense.

This is a breaking change, since stable code can observe the deleted PinCoerceUnsized impls by uselessly coercing between such types inside a Pin.

There is still some strange behavior, such as Pin<&mut i32> being able to coerce to Pin<&dyn Send>, but not being able to coerce to Pin<&i32>. However, I don't think it's possible to fix this.

Fixes #145081

[rustbot]

r? @tgross35

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

[jdonszelmann]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 94e8487 (94e848735e78b58442e0b82e9c60f8f430840389, parent: e0e204f3e97ad5f79524b9c259dc38df606ed82c)

[saethlin]

@craterbot check

[craterbot]

👌 Experiment pr-149218 created and queued.
🤖 Automatically detected try build 94e8487
🔍 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]

🚧 Experiment pr-149218 is now running

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

[Darksonn]

I added these impls because otherwise the introduction of PinCoerceUnsized would have been a breaking change. I'm in favor of removing them.

[craterbot]

🎉 Experiment pr-149218 is completed!
📊 6 regressed and 5 fixed (743358 total)
📊 1966 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 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

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-149218/retry-regressed-list.txt ↩

[workingjubilee]

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-149218/retry-regressed-list.txt

[craterbot]

👌 Experiment pr-149218-1 created and queued.
🤖 Automatically detected try build 94e8487
🔍 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

[theemathas]

Crater results are clean.

Which team does this need an FCP with?

[bors]

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

[Darksonn]

Probably T-lang?

[rustbot]

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.

[tgross35]

I don't have any concerns with the library changes here, but this design change is indeed owned by @rust-lang/lang.

r? lang

(Sorry, I should have rerolled a long time ago)

[tmandry]

The lang team discussed this today. Everyone present was in agreement that we should remove the impls.

Adding the Deref supertrait was more controversial. @traviscross commented that Pin<*const u32> could be a useful type for communicating invariants – in a world where Pin::new_unchecked() did not have a Deref bound on the impl. In that case we might want the coercion in this example to work:

fn bar(x: Pin<&i32>) -> Pin<*const dyn Send> {
    x
}

Note: This trait is unstable, so we could leave the Deref supertrait as an open question for when it stabilizes. We could also use a sealed supertrait with a blanket impl to require for now that implementers of PinCoerceUnsized also implement Deref, without allowing users to rely on that supertrait relationship.

[traviscross]

Proposal: Let's merge this PR, but in accepting this let's leave open the question of whether Deref should be a supertrait of PinCoerceUnsized or whether we'd be better served doing something else.

@rfcbot fcp merge lang,libs-api

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @nikomatsakis
• @scottmcm
• @the8472
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[Darksonn]

Let's merge this PR, but in accepting this let's leave open the question of whether Deref should be a supertrait of PinCoerceUnsized or whether we'd be better served doing something else.

Yes, absolutely. This FCP should be about whether we are ok with the breaking change of removing all unsizing coercions where the source or target type is any of the following:

• Pin<NonNull<T>>
• Pin<*const T>
• Pin<*mut T>
• Pin<rc::Weak<T>>
• Pin<sync::Weak<T>>
• Pin<UnsafeCell<P>>
• Pin<Cell<P>>
• Pin<RefCell<P>>
• (and a few more unstable types)

with the justification that you can't construct any of these types because all Pin<P> constructors require P: Deref. Edit: Apparently you can construct Pin<*mut T> in some edge cases, see below.

How to express unsizing coercions in the trait system should be kept as a separate topic. It happens to involve PinCoerceUnsized right now, but all of the involved traits are unstable.

[theemathas]

with the justification that you can't construct any of these types because all Pin<P> constructors require P: Deref.

You can construct a Pin<*const dyn Trait> or a Pin<*mut dyn Trait> by unsize-coercing from a Pin<&mut T>

[Darksonn]

I see, I did not realize that. However I believe this PR would remove that coercion too, since impl CoerceUnsized for Pin requires PinCoerceUnsized for both the source and target pointer type.

[Darksonn]

I guess my list is also missing some Pin<Pin<_>> cases, since Pin<Pin<P>> can be unsizing coerced iff Pin<P> can.

[tmandry]

Agreed with the plan to revisit the PinCoerceUnsized/Deref relationship when we go to stabilize that. The removed coercion from Pin<&mut T> to Pin<*{const,mut} dyn Trait> can be added back later.

@rfcbot reviewed

[nikomatsakis]

@rfcbot reviewed