Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Pin stabilization #56939

Merged
merged 5 commits into from
Dec 24, 2018
Merged

Pin stabilization #56939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20d694a
Update Pin API to match the one proposed for stabilization
cramertj Dec 18, 2018
610bcaf
Stabilize Pin
cramertj Dec 18, 2018
3005bf3
Pin stabilization: fix doctests
cramertj Dec 18, 2018
684fe9a
Rename Box/Arc/Rc::pinned to ::pin
cramertj Dec 18, 2018
861df06
Fix Unpin docs link
cramertj Dec 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/liballoc/boxed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,11 @@ impl<T> Box<T> {
box x
}

#[unstable(feature = "pin", issue = "49150")]
/// Constructs a new `Pin<Box<T>>`. If `T` does not implement `Unpin`, then
/// `x` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn pinned(x: T) -> Pin<Box<T>> {
pub fn pin(x: T) -> Pin<Box<T>> {
(box x).into()
}
}
Expand Down Expand Up @@ -446,7 +448,7 @@ impl<T> From<T> for Box<T> {
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<T> From<Box<T>> for Pin<Box<T>> {
fn from(boxed: Box<T>) -> Self {
// It's not possible to move or replace the insides of a `Pin<Box<T>>`
Expand Down Expand Up @@ -813,7 +815,7 @@ impl<T: ?Sized> AsMut<T> for Box<T> {
* implementation of `Unpin` (where `T: Unpin`) would be valid/safe, and
* could have a method to project a Pin<T> from it.
*/
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<T: ?Sized> Unpin for Box<T> { }

#[unstable(feature = "generator_trait", issue = "43122")]
Expand Down
1 change: 0 additions & 1 deletion src/liballoc/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
#![feature(nll)]
#![feature(optin_builtin_traits)]
#![feature(pattern)]
#![feature(pin)]
#![feature(ptr_internals)]
#![feature(ptr_offset_from)]
#![feature(rustc_attrs)]
Expand Down
8 changes: 5 additions & 3 deletions src/liballoc/rc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,10 @@ impl<T> Rc<T> {
}
}

#[unstable(feature = "pin", issue = "49150")]
pub fn pinned(value: T) -> Pin<Rc<T>> {
/// Constructs a new `Pin<Rc<T>>`. If `T` does not implement `Unpin`, then
/// `value` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
pub fn pin(value: T) -> Pin<Rc<T>> {
unsafe { Pin::new_unchecked(Rc::new(value)) }
}

Expand Down Expand Up @@ -1931,5 +1933,5 @@ impl<T: ?Sized> AsRef<T> for Rc<T> {
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<T: ?Sized> Unpin for Rc<T> { }
8 changes: 5 additions & 3 deletions src/liballoc/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,10 @@ impl<T> Arc<T> {
Arc { ptr: Box::into_raw_non_null(x), phantom: PhantomData }
}

#[unstable(feature = "pin", issue = "49150")]
pub fn pinned(data: T) -> Pin<Arc<T>> {
/// Constructs a new `Pin<Arc<T>>`. If `T` does not implement `Unpin`, then
/// `data` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
pub fn pin(data: T) -> Pin<Arc<T>> {
unsafe { Pin::new_unchecked(Arc::new(data)) }
}

Expand Down Expand Up @@ -2047,5 +2049,5 @@ impl<T: ?Sized> AsRef<T> for Arc<T> {
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<T: ?Sized> Unpin for Arc<T> { }
2 changes: 1 addition & 1 deletion src/libcore/future/future.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<'a, F: ?Sized + Future + Unpin> Future for &'a mut F {

impl<P> Future for Pin<P>
where
P: ops::DerefMut,
P: Unpin + ops::DerefMut,
P::Target: Future,
{
type Output = <<P as ops::Deref>::Target as Future>::Output;
Expand Down
11 changes: 5 additions & 6 deletions src/libcore/marker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,6 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
/// So this, for example, can only be done on types implementing `Unpin`:
///
/// ```rust
/// #![feature(pin)]
/// use std::mem::replace;
/// use std::pin::Pin;
///
Expand All @@ -637,23 +636,23 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
/// [`replace`]: ../../std/mem/fn.replace.html
/// [`Pin`]: ../pin/struct.Pin.html
/// [`pin module`]: ../../std/pin/index.html
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
pub auto trait Unpin {}

/// A marker type which does not implement `Unpin`.
///
/// If a type contains a `PhantomPinned`, it will not implement `Unpin` by default.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct PhantomPinned;

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl !Unpin for PhantomPinned {}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<'a, T: ?Sized + 'a> Unpin for &'a T {}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<'a, T: ?Sized + 'a> Unpin for &'a mut T {}

/// Implementations of `Copy` for primitive types.
Expand Down
6 changes: 3 additions & 3 deletions src/libcore/option.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl<T> Option<T> {

/// Converts from `Pin<&Option<T>>` to `Option<Pin<&T>>`
#[inline]
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
cramertj marked this conversation as resolved.
Show resolved Hide resolved
pub fn as_pin_ref<'a>(self: Pin<&'a Option<T>>) -> Option<Pin<&'a T>> {
unsafe {
Pin::get_ref(self).as_ref().map(|x| Pin::new_unchecked(x))
Expand All @@ -282,10 +282,10 @@ impl<T> Option<T> {

/// Converts from `Pin<&mut Option<T>>` to `Option<Pin<&mut T>>`
#[inline]
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
pub fn as_pin_mut<'a>(self: Pin<&'a mut Option<T>>) -> Option<Pin<&'a mut T>> {
unsafe {
Pin::get_mut_unchecked(self).as_mut().map(|x| Pin::new_unchecked(x))
Pin::get_unchecked_mut(self).as_mut().map(|x| Pin::new_unchecked(x))
}
}

Expand Down
81 changes: 37 additions & 44 deletions src/libcore/pin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,13 @@
//! are always freely movable, even if the data they point to isn't.
//!
//! [`Pin`]: struct.Pin.html
//! [`Unpin`]: trait.Unpin.html
//! [`Unpin`]: ../../std/marker/trait.Unpin.html
//! [`swap`]: ../../std/mem/fn.swap.html
//! [`Box`]: ../../std/boxed/struct.Box.html
//!
//! # Examples
//!
//! ```rust
//! #![feature(pin)]
//!
//! use std::pin::Pin;
//! use std::marker::PhantomPinned;
//! use std::ptr::NonNull;
Expand Down Expand Up @@ -72,13 +70,13 @@
//! slice: NonNull::dangling(),
//! _pin: PhantomPinned,
//! };
//! let mut boxed = Box::pinned(res);
//! let mut boxed = Box::pin(res);
//!
//! let slice = NonNull::from(&boxed.data);
//! // we know this is safe because modifying a field doesn't move the whole struct
//! unsafe {
//! let mut_ref: Pin<&mut Self> = Pin::as_mut(&mut boxed);
//! Pin::get_mut_unchecked(mut_ref).slice = slice;
//! Pin::get_unchecked_mut(mut_ref).slice = slice;
//! }
//! boxed
//! }
Expand All @@ -97,15 +95,12 @@
//! // std::mem::swap(&mut *still_unmoved, &mut *new_unmoved);
//! ```

#![unstable(feature = "pin", issue = "49150")]
#![stable(feature = "pin", since = "1.33.0")]

use fmt;
use marker::Sized;
use marker::{Sized, Unpin};
use ops::{Deref, DerefMut, Receiver, CoerceUnsized, DispatchFromDyn};

#[doc(inline)]
pub use marker::Unpin;

/// A pinned pointer.
///
/// This is a wrapper around a kind of pointer which makes that pointer "pin" its
Expand All @@ -119,8 +114,9 @@ pub use marker::Unpin;
//
// Note: the derives below are allowed because they all only use `&P`, so they
// cannot move the value behind `pointer`.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[fundamental]
#[repr(transparent)]
#[derive(Copy, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
pub struct Pin<P> {
pointer: P,
Expand All @@ -132,7 +128,7 @@ where
{
/// Construct a new `Pin` around a pointer to some data of a type that
/// implements `Unpin`.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn new(pointer: P) -> Pin<P> {
// Safety: the value pointed to is `Unpin`, and so has no requirements
Expand All @@ -154,14 +150,14 @@ impl<P: Deref> Pin<P> {
///
/// If `pointer` dereferences to an `Unpin` type, `Pin::new` should be used
/// instead.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub unsafe fn new_unchecked(pointer: P) -> Pin<P> {
Pin { pointer }
}

/// Get a pinned shared reference from this pinned pointer.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn as_ref(self: &Pin<P>) -> Pin<&P::Target> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked with @withoutboats in person about this a bit as I was slightly concerned about this, so I just wanted to make sure to write down some thoughts. The AsRef trait is the libs team's worst offender in terms of "thorns in our side about backwards compatibility", as adding AsRef anywhere seems to break due to subtle inference changes.

Along those lines, I think it definitely makes sense to be consistent with self vs not in Pin, but I just wanted to raise this and make sure it'd been considered. I think @withoutboats mentioned they'd talk with you @cramertj about the ergonomic importance of self-vs-not. It seems like the primary motivation for self everywhere was ergonomics, and I'd just want to make sure it's well motivated to take such a common name like as_ref!

Concretely my worry is that we may not be able to add inherent self-based methods to Pin in the future if they cause too much breakage in the ecosystem (due to shadowing conflicts), which would cause us to have two idioms.

Again though, just wanted to make sure I brought that up and ensure it was weighed!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ergonomic improvement from self in as_mut and set at least are pretty important because it's common to see them chained, which gets pretty ugly otherwise. The most common instance of this is Pin::get_mut_unchecked(Pin::as_mut(self)) inside futures-rs (it's pretty common to see Pin::as_mut used for reborrowing in combination with other methods). set is another one where I personally just get it wrong every time and try to spell it self.set(value) rather than Pin::set(Pin::as_mut(self), value);-- I think you can probably see why ;).

I'm totally sympathetic to the concerns here and am interested in any ideas you might have for making this easier. One thing I will say is that I think it's much more common to call Pin methods on a Pin<P<T>> than it is to call e.g. Rc methods on an Rc<T>, so I'd been using that to justify to myself why Pin was "different" in this respect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure thing, I just wanted to double-confirm. I agree that the usage pattern of Pin seems like it favors inherent methods more often is good motivation for breaking the norm.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing @alexcrichton and I talked about was making only as_ref and as_mut associated functions, so you'd get Pin::as_mut(self).get_mut_uncheckedand so on. What would you think about that @cramertj?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably want something not called as_mut that did reborrowing in that case. I can define it as a trait in the pin_utils crate or something, but that seems like it kind of defeats the purpose.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(to be clear: I'm fine making this change if it's the one @rust-lang/libs wants, it's just much more ergonomic and easier to read the chained-method version)

unsafe { Pin::new_unchecked(&*self.pointer) }
Expand All @@ -170,14 +166,14 @@ impl<P: Deref> Pin<P> {

impl<P: DerefMut> Pin<P> {
/// Get a pinned mutable reference from this pinned pointer.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn as_mut(self: &mut Pin<P>) -> Pin<&mut P::Target> {
unsafe { Pin::new_unchecked(&mut *self.pointer) }
}

/// Assign a new value to the memory behind the pinned reference.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn set(mut self: Pin<P>, value: P::Target)
where
Expand All @@ -199,11 +195,11 @@ impl<'a, T: ?Sized> Pin<&'a T> {
/// will not move so long as the argument value does not move (for example,
/// because it is one of the fields of that value), and also that you do
/// not move out of the argument you receive to the interior function.
#[unstable(feature = "pin", issue = "49150")]
pub unsafe fn map_unchecked<U, F>(this: Pin<&'a T>, func: F) -> Pin<&'a U> where
#[stable(feature = "pin", since = "1.33.0")]
pub unsafe fn map_unchecked<U, F>(self: Pin<&'a T>, func: F) -> Pin<&'a U> where
F: FnOnce(&T) -> &U,
{
let pointer = &*this.pointer;
let pointer = &*self.pointer;
let new_pointer = func(pointer);
Pin::new_unchecked(new_pointer)
}
Expand All @@ -215,19 +211,19 @@ impl<'a, T: ?Sized> Pin<&'a T> {
/// that lives for as long as the borrow of the `Pin`, not the lifetime of
/// the `Pin` itself. This method allows turning the `Pin` into a reference
/// with the same lifetime as the original `Pin`.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn get_ref(this: Pin<&'a T>) -> &'a T {
this.pointer
pub fn get_ref(self: Pin<&'a T>) -> &'a T {
self.pointer
}
}

impl<'a, T: ?Sized> Pin<&'a mut T> {
/// Convert this `Pin<&mut T>` into a `Pin<&T>` with the same lifetime.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn into_ref(this: Pin<&'a mut T>) -> Pin<&'a T> {
Pin { pointer: this.pointer }
pub fn into_ref(self: Pin<&'a mut T>) -> Pin<&'a T> {
Pin { pointer: self.pointer }
}

/// Get a mutable reference to the data inside of this `Pin`.
Expand All @@ -239,12 +235,12 @@ impl<'a, T: ?Sized> Pin<&'a mut T> {
/// that lives for as long as the borrow of the `Pin`, not the lifetime of
/// the `Pin` itself. This method allows turning the `Pin` into a reference
/// with the same lifetime as the original `Pin`.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn get_mut(this: Pin<&'a mut T>) -> &'a mut T
pub fn get_mut(self: Pin<&'a mut T>) -> &'a mut T
where T: Unpin,
{
this.pointer
self.pointer
}

/// Get a mutable reference to the data inside of this `Pin`.
Expand All @@ -257,10 +253,10 @@ impl<'a, T: ?Sized> Pin<&'a mut T> {
///
/// If the underlying data is `Unpin`, `Pin::get_mut` should be used
/// instead.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub unsafe fn get_mut_unchecked(this: Pin<&'a mut T>) -> &'a mut T {
this.pointer
pub unsafe fn get_unchecked_mut(self: Pin<&'a mut T>) -> &'a mut T {
self.pointer
}

/// Construct a new pin by mapping the interior value.
Expand All @@ -274,25 +270,25 @@ impl<'a, T: ?Sized> Pin<&'a mut T> {
/// will not move so long as the argument value does not move (for example,
/// because it is one of the fields of that value), and also that you do
/// not move out of the argument you receive to the interior function.
#[unstable(feature = "pin", issue = "49150")]
pub unsafe fn map_unchecked_mut<U, F>(this: Pin<&'a mut T>, func: F) -> Pin<&'a mut U> where
#[stable(feature = "pin", since = "1.33.0")]
pub unsafe fn map_unchecked_mut<U, F>(self: Pin<&'a mut T>, func: F) -> Pin<&'a mut U> where
F: FnOnce(&mut T) -> &mut U,
{
let pointer = Pin::get_mut_unchecked(this);
let pointer = Pin::get_unchecked_mut(self);
let new_pointer = func(pointer);
Pin::new_unchecked(new_pointer)
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P: Deref> Deref for Pin<P> {
type Target = P::Target;
fn deref(&self) -> &P::Target {
Pin::get_ref(Pin::as_ref(self))
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P: DerefMut> DerefMut for Pin<P>
where
P::Target: Unpin
Expand All @@ -305,21 +301,21 @@ where
#[unstable(feature = "receiver_trait", issue = "0")]
impl<P: Receiver> Receiver for Pin<P> {}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P: fmt::Debug> fmt::Debug for Pin<P> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(&self.pointer, f)
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P: fmt::Display> fmt::Display for Pin<P> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Display::fmt(&self.pointer, f)
}
}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P: fmt::Pointer> fmt::Pointer for Pin<P> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Pointer::fmt(&self.pointer, f)
Expand All @@ -331,17 +327,14 @@ impl<P: fmt::Pointer> fmt::Pointer for Pin<P> {
// `Deref<Target=Unpin>` is unsound. Any such impl would probably be unsound
// for other reasons, though, so we just need to take care not to allow such
// impls to land in std.
#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<P, U> CoerceUnsized<Pin<U>> for Pin<P>
where
P: CoerceUnsized<U>,
{}

#[unstable(feature = "pin", issue = "49150")]
#[stable(feature = "pin", since = "1.33.0")]
impl<'a, P, U> DispatchFromDyn<Pin<U>> for Pin<P>
where
P: DispatchFromDyn<U>,
{}

#[unstable(feature = "pin", issue = "49150")]
impl<P> Unpin for Pin<P> {}
Loading