From 3b2b5b2914d40aa011d189bfe546084cdee53dbe Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Sun, 24 Jan 2021 16:43:54 -0800 Subject: [PATCH 1/3] Remove P: Unpin bound on impl Future for Pin The `Unpin` bound was originally added in #56939 following the recommendation of @withoutboats in https://github.com/rust-lang/rust/issues/55766#issue-378417538 That comment does not give explicit justification for why the bound should be added. The relevant context was: > [ ] Remove `impl

Unpin for Pin

` > > This impl is not justified by our standard justification for unpin > impls: there is no pointer direction between `Pin

` and `P`. Its > usefulness is covered by the impls for pointers themselves. > > This futures impl (link to the impl changed in this PR) will need to > change to add a `P: Unpin` bound. The decision to remove the unconditional impl of `Unpin for Pin` is sound (these days there is just an auto-impl for when `P: Unpin`). But, I think the decision to also add the `Unpin` bound for `impl Future` may have been unnecessary. Or if that's not the case, I'd be very interested to have the argument for why written down somewhere. The bound _appears_ to not be needed, since the presence of a `Pin

` should indicate that it's safe to project to `Pin<&mut P::Target>` just like for `Pin::as_mut`. --- library/core/src/future/future.rs | 4 ++-- library/core/src/lib.rs | 1 + library/core/src/pin.rs | 38 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/library/core/src/future/future.rs b/library/core/src/future/future.rs index e9a99ddb6b1bd..5a07c4ab22f3b 100644 --- a/library/core/src/future/future.rs +++ b/library/core/src/future/future.rs @@ -111,11 +111,11 @@ impl Future for &mut F { #[stable(feature = "futures_api", since = "1.36.0")] impl

Future for Pin

where - P: Unpin + ops::DerefMut, + P: ops::DerefMut, { type Output = <

::Target as Future>::Output; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - Pin::get_mut(self).as_mut().poll(cx) + ::poll(self.as_deref_mut(), cx) } } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index f47ad7e6bc214..07424e18e4e2b 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -127,6 +127,7 @@ #![feature(no_core)] #![feature(auto_traits)] #![cfg_attr(bootstrap, feature(or_patterns))] +#![feature(pin_deref_mut)] #![feature(prelude_import)] #![cfg_attr(not(bootstrap), feature(ptr_metadata))] #![feature(repr_simd, platform_intrinsics)] diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index b2de0e16a17bb..bc562153713be 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -793,6 +793,44 @@ impl Pin<&'static T> { } } +impl<'a, P: DerefMut> Pin<&'a mut Pin

> { + /// Gets a pinned mutable reference from this nested pinned pointer. + /// + /// This is a generic method to go from `Pin<&mut Pin>>` to `Pin<&mut T>`. It is + /// safe because the existence of a `Pin>` ensures that the pointee, `T`, cannot + /// move in the future, and this method does not enable the pointee to move. "Malicious" + /// implementations of `Pointer::DerefMut` are likewise ruled out by the contract of + /// `Pin::new_unchecked`. + #[unstable(feature = "pin_deref_mut", issue = "none")] + #[inline(always)] + pub fn as_deref_mut(self) -> Pin<&'a mut P::Target> { + // SAFETY: What we're asserting here is that going from + // + // Pin<&mut Pin

> + // + // to + // + // Pin<&mut P::Target> + // + // is safe. + // + // We need to ensure that two things hold for that to be the case: + // + // 1) Once we give out a `Pin<&mut P::Target>`, an `&mut P::Target` will not be given out. + // 2) By giving out a `Pin<&mut P::Target>`, we do not risk of violating `Pin<&mut Pin

>` + // + // The existence of `Pin

` is sufficient to guarantee #1: since we already have a + // `Pin

`, it must already uphold the pinning guarantees, which must mean that + // `Pin<&mut P::Target>` does as well, since `Pin::as_mut` is safe. We do not have to rely + // on the fact that P is _also_ pinned. + // + // For #2, we need to ensure that code given a `Pin<&mut P::Target>` cannot cause the + // `Pin

` to move? That is not possible, since `Pin<&mut P::Target>` no longer retains + // any access to the `P` itself, much less the `Pin

`. + unsafe { self.get_unchecked_mut() }.as_mut() + } +} + impl Pin<&'static mut T> { /// Get a pinned mutable reference from a static mutable reference. /// From b5e5a182caa6efb7359978a6ff596f403cb4c099 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 15 Jun 2021 20:47:43 -0400 Subject: [PATCH 2/3] Update library/core/src/pin.rs Co-authored-by: Ralf Jung --- library/core/src/pin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index 7eb2029013e62..85411bac946a4 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -808,7 +808,7 @@ impl<'a, P: DerefMut> Pin<&'a mut Pin

> { /// This is a generic method to go from `Pin<&mut Pin>>` to `Pin<&mut T>`. It is /// safe because the existence of a `Pin>` ensures that the pointee, `T`, cannot /// move in the future, and this method does not enable the pointee to move. "Malicious" - /// implementations of `Pointer::DerefMut` are likewise ruled out by the contract of + /// implementations of `P::DerefMut` are likewise ruled out by the contract of /// `Pin::new_unchecked`. #[unstable(feature = "pin_deref_mut", issue = "none")] #[inline(always)] From cf402921222bd6b3152c6ed55c7039887d12a4c0 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 6 Jul 2021 16:59:14 -0700 Subject: [PATCH 3/3] Link tracking issue for pin_deref_mut --- library/core/src/pin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index 85411bac946a4..6b1a12ed18c0f 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -810,7 +810,7 @@ impl<'a, P: DerefMut> Pin<&'a mut Pin

> { /// move in the future, and this method does not enable the pointee to move. "Malicious" /// implementations of `P::DerefMut` are likewise ruled out by the contract of /// `Pin::new_unchecked`. - #[unstable(feature = "pin_deref_mut", issue = "none")] + #[unstable(feature = "pin_deref_mut", issue = "86918")] #[inline(always)] pub fn as_deref_mut(self) -> Pin<&'a mut P::Target> { // SAFETY: What we're asserting here is that going from