Skip to content

Commit aab057e

Browse files
JohnTitorgitbot
authored and
gitbot
committed
Rollup merge of rust-lang#81363 - jonhoo:no-unpin-in-pin-future-impl, r=m-ou-se
Remove P: Unpin bound on impl Future for Pin We can safely produce a `Pin<&mut P::Target>` without moving out of the `Pin` by using `Pin::as_mut` directly. The `Unpin` bound was originally added in rust-lang#56939 following the recommendation of ``@withoutboats`` in rust-lang#55766 (comment) 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, as demonstrated by the change requiring no unsafe code and by the existence of `Pin::as_mut`.
2 parents d30f325 + 0966231 commit aab057e

File tree

3 files changed

+41
-2
lines changed

3 files changed

+41
-2
lines changed

core/src/future/future.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,11 @@ impl<F: ?Sized + Future + Unpin> Future for &mut F {
111111
#[stable(feature = "futures_api", since = "1.36.0")]
112112
impl<P> Future for Pin<P>
113113
where
114-
P: Unpin + ops::DerefMut<Target: Future>,
114+
P: ops::DerefMut<Target: Future>,
115115
{
116116
type Output = <<P as ops::Deref>::Target as Future>::Output;
117117

118118
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
119-
Pin::get_mut(self).as_mut().poll(cx)
119+
<P::Target as Future>::poll(self.as_deref_mut(), cx)
120120
}
121121
}

core/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
#![feature(exhaustive_patterns)]
127127
#![feature(no_core)]
128128
#![feature(auto_traits)]
129+
#![feature(pin_deref_mut)]
129130
#![feature(prelude_import)]
130131
#![feature(ptr_metadata)]
131132
#![feature(repr_simd, platform_intrinsics)]

core/src/pin.rs

+38
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,44 @@ impl<T: ?Sized> Pin<&'static T> {
802802
}
803803
}
804804

805+
impl<'a, P: DerefMut> Pin<&'a mut Pin<P>> {
806+
/// Gets a pinned mutable reference from this nested pinned pointer.
807+
///
808+
/// This is a generic method to go from `Pin<&mut Pin<Pointer<T>>>` to `Pin<&mut T>`. It is
809+
/// safe because the existence of a `Pin<Pointer<T>>` ensures that the pointee, `T`, cannot
810+
/// move in the future, and this method does not enable the pointee to move. "Malicious"
811+
/// implementations of `P::DerefMut` are likewise ruled out by the contract of
812+
/// `Pin::new_unchecked`.
813+
#[unstable(feature = "pin_deref_mut", issue = "86918")]
814+
#[inline(always)]
815+
pub fn as_deref_mut(self) -> Pin<&'a mut P::Target> {
816+
// SAFETY: What we're asserting here is that going from
817+
//
818+
// Pin<&mut Pin<P>>
819+
//
820+
// to
821+
//
822+
// Pin<&mut P::Target>
823+
//
824+
// is safe.
825+
//
826+
// We need to ensure that two things hold for that to be the case:
827+
//
828+
// 1) Once we give out a `Pin<&mut P::Target>`, an `&mut P::Target` will not be given out.
829+
// 2) By giving out a `Pin<&mut P::Target>`, we do not risk of violating `Pin<&mut Pin<P>>`
830+
//
831+
// The existence of `Pin<P>` is sufficient to guarantee #1: since we already have a
832+
// `Pin<P>`, it must already uphold the pinning guarantees, which must mean that
833+
// `Pin<&mut P::Target>` does as well, since `Pin::as_mut` is safe. We do not have to rely
834+
// on the fact that P is _also_ pinned.
835+
//
836+
// For #2, we need to ensure that code given a `Pin<&mut P::Target>` cannot cause the
837+
// `Pin<P>` to move? That is not possible, since `Pin<&mut P::Target>` no longer retains
838+
// any access to the `P` itself, much less the `Pin<P>`.
839+
unsafe { self.get_unchecked_mut() }.as_mut()
840+
}
841+
}
842+
805843
impl<T: ?Sized> Pin<&'static mut T> {
806844
/// Get a pinned mutable reference from a static mutable reference.
807845
///

0 commit comments

Comments
 (0)