about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJon Gjengset <jon@thesquareplanet.com>2021-01-24 16:43:54 -0800
committerJon Gjengset <jon@thesquareplanet.com>2021-03-28 12:37:09 -0700
commit3b2b5b2914d40aa011d189bfe546084cdee53dbe (patch)
tree5d23738c950c4a4df4561f2582c8933c9e859084
parent0239876020e9a44593e7946af92b35728eede5ae (diff)
downloadrust-3b2b5b2914d40aa011d189bfe546084cdee53dbe.tar.gz
rust-3b2b5b2914d40aa011d189bfe546084cdee53dbe.zip
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<P> Unpin for Pin<P>`
>
> This impl is not justified by our standard justification for unpin
> impls: there is no pointer direction between `Pin<P>` 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<P>` should indicate that
it's safe to project to `Pin<&mut P::Target>` just like for
`Pin::as_mut`.
-rw-r--r--library/core/src/future/future.rs4
-rw-r--r--library/core/src/lib.rs1
-rw-r--r--library/core/src/pin.rs38
3 files changed, 41 insertions, 2 deletions
diff --git a/library/core/src/future/future.rs b/library/core/src/future/future.rs
index e9a99ddb6b1..5a07c4ab22f 100644
--- a/library/core/src/future/future.rs
+++ b/library/core/src/future/future.rs
@@ -111,11 +111,11 @@ impl<F: ?Sized + Future + Unpin> Future for &mut F {
 #[stable(feature = "futures_api", since = "1.36.0")]
 impl<P> Future for Pin<P>
 where
-    P: Unpin + ops::DerefMut<Target: Future>,
+    P: ops::DerefMut<Target: Future>,
 {
     type Output = <<P as ops::Deref>::Target as Future>::Output;
 
     fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        Pin::get_mut(self).as_mut().poll(cx)
+        <P::Target as Future>::poll(self.as_deref_mut(), cx)
     }
 }
diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs
index f47ad7e6bc2..07424e18e4e 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 b2de0e16a17..bc562153713 100644
--- a/library/core/src/pin.rs
+++ b/library/core/src/pin.rs
@@ -793,6 +793,44 @@ impl<T: ?Sized> Pin<&'static T> {
     }
 }
 
+impl<'a, P: DerefMut> Pin<&'a mut Pin<P>> {
+    /// Gets a pinned mutable reference from this nested pinned pointer.
+    ///
+    /// This is a generic method to go from `Pin<&mut Pin<Pointer<T>>>` to `Pin<&mut T>`. It is
+    /// safe because the existence of a `Pin<Pointer<T>>` 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<P>>
+        //
+        // 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<P>>`
+        //
+        // The existence of `Pin<P>` is sufficient to guarantee #1: since we already have a
+        // `Pin<P>`, 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<P>` to move? That is not possible, since `Pin<&mut P::Target>` no longer retains
+        // any access to the `P` itself, much less the `Pin<P>`.
+        unsafe { self.get_unchecked_mut() }.as_mut()
+    }
+}
+
 impl<T: ?Sized> Pin<&'static mut T> {
     /// Get a pinned mutable reference from a static mutable reference.
     ///