From e61a8a94f7762abcd2e77a3108f766d2f5bb1185 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 21 Feb 2019 15:28:46 +0100 Subject: [PATCH] Apply suggestions from code review Co-Authored-By: RalfJung --- src/libcore/marker.rs | 4 +-- src/libcore/pin.rs | 77 ++++++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 38c9fe79f84..91fd41e97b3 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -612,7 +612,7 @@ unsafe impl Freeze for &mut T {} /// `Unpin` has no consequence at all for non-pinned data. In particular, /// [`mem::replace`] happily moves `!Unpin` data (it works for any `&mut T`, not /// just when `T: Unpin`). However, you cannot use -/// [`mem::replace`] on data wrapped inside a [`Pin`] because you cannot get the +/// [`mem::replace`] on data wrapped inside a [`Pin

`] because you cannot get the /// `&mut T` you need for that, and *that* is what makes this system work. /// /// So this, for example, can only be done on types implementing `Unpin`: @@ -633,7 +633,7 @@ unsafe impl Freeze for &mut T {} /// This trait is automatically implemented for almost every type. /// /// [`mem::replace`]: ../../std/mem/fn.replace.html -/// [`Pin`]: ../pin/struct.Pin.html + /// [`Pin

`]: ../pin/struct.Pin.html /// [`pin module`]: ../../std/pin/index.html #[stable(feature = "pin", since = "1.33.0")] #[cfg_attr(not(stage0), lang = "unpin")] diff --git a/src/libcore/pin.rs b/src/libcore/pin.rs index bcd5c65d381..9ce85309565 100644 --- a/src/libcore/pin.rs +++ b/src/libcore/pin.rs @@ -6,16 +6,16 @@ //! since moving an object with pointers to itself will invalidate them, //! which could cause undefined behavior. //! -//! [`Pin`] ensures that the pointee of any pointer type has a stable location in memory, +//! A [`Pin

`] ensures that the pointee of any pointer type `P` has a stable location in memory, //! meaning it cannot be moved elsewhere and its memory cannot be deallocated //! until it gets dropped. We say that the pointee is "pinned". //! //! By default, all types in Rust are movable. Rust allows passing all types by-value, -//! and common smart-pointer types such as `Box` and `&mut` allow replacing and -//! moving the values they contain: you can move out of a `Box`, or you can use [`mem::swap`]. -//! [`Pin`] wraps a pointer type, so `Pin>` functions much like a regular `Box` -//! (when a `Pin>` gets dropped, so do its contents, and the memory gets deallocated). -//! Similarily, `Pin<&mut T>` is a lot like `&mut T`. However, [`Pin`] does not let clients actually +//! and common smart-pointer types such as `Box` and `&mut T` allow replacing and +//! moving the values they contain: you can move out of a `Box`, or you can use [`mem::swap`]. +//! [`Pin

`] wraps a pointer type `P`, so `Pin>` functions much like a regular `Box`: +//! when a `Pin>` gets dropped, so do its contents, and the memory gets deallocated. +//! Similarily, `Pin<&mut T>` is a lot like `&mut T`. However, [`Pin

`] does not let clients actually //! obtain a `Box` or `&mut T` to pinned data, which implies that you cannot use //! operations such as [`mem::swap`]: //! ``` @@ -28,18 +28,18 @@ //! } //! ``` //! -//! It is worth reiterating that [`Pin`] does *not* change the fact that a Rust compiler -//! considers all types movable. [`mem::swap`] remains callable for any `T`. Instead, `Pin` -//! prevents certain *values* (pointed to by pointers wrapped in `Pin`) from being +//! It is worth reiterating that [`Pin

`] does *not* change the fact that a Rust compiler +//! considers all types movable. [`mem::swap`] remains callable for any `T`. Instead, `Pin

` +//! prevents certain *values* (pointed to by pointers wrapped in `Pin

`) from being //! moved by making it impossible to call methods that require `&mut T` on them //! (like [`mem::swap`]). //! -//! [`Pin`] can be used to wrap any pointer type, and as such it interacts with +//! [`Pin

`] can be used to wrap any pointer type `P`, and as such it interacts with //! [`Deref`] and [`DerefMut`]. A `Pin

` where `P: Deref` should be considered //! as a "`P`-style pointer" to a pinned `P::Target` -- so, a `Pin>` is //! an owned pointer to a pinned `T`, and a `Pin>` is a reference-counted //! pointer to a pinned `T`. -//! For correctness, [`Pin`] relies on the [`Deref`] and [`DerefMut`] implementations +//! For correctness, [`Pin

`] relies on the [`Deref`] and [`DerefMut`] implementations //! to not move out of their `self` parameter, and to only ever return a pointer //! to pinned data when they are called on a pinned pointer. //! @@ -50,11 +50,11 @@ //! This includes all the basic types (`bool`, `i32` and friends, references) //! as well as types consisting solely of these types. //! Types that do not care about pinning implement the [`Unpin`] auto-trait, which -//! nullifies the effect of [`Pin`]. For `T: Unpin`, `Pin>` and `Box` function +//! cancels the effect of [`Pin

`]. For `T: Unpin`, `Pin>` and `Box` function //! identically, as do `Pin<&mut T>` and `&mut T`. //! //! Note that pinning and `Unpin` only affect the pointed-to type, not the pointer -//! type itself that got wrapped in `Pin`. For example, whether or not `Box` is +//! type `P` itself that got wrapped in `Pin

`. For example, whether or not `Box` is //! `Unpin` has no effect on the behavior of `Pin>` (here, `T` is the //! pointed-to type). //! @@ -120,7 +120,7 @@ //! and elements can live on a stack frame that lives shorter than the collection does. //! //! To make this work, every element has pointers to its predecessor and successor in -//! the list. Element can only be added when they are pinned, because moving the elements +//! the list. Elements can only be added when they are pinned, because moving the elements //! around would invalidate the pointers. Moreover, the `Drop` implementation of a linked //! list element will patch the pointers of its predecessor and successor to remove itself //! from the list. @@ -129,17 +129,17 @@ //! could be deallocated or otherwise invalidated without calling `drop`, the pointers into it //! from its neighbouring elements would become invalid, which would break the data structure. //! -//! This is why pinning also comes with a `drop`-related guarantee. +//! Therefore, pinning also comes with a `drop`-related guarantee. //! //! # `Drop` guarantee //! //! The purpose of pinning is to be able to rely on the placement of some data in memory. -//! To make this work, not just moving the data is restricted; deallocating, repurposing or +//! To make this work, not just moving the data is restricted; deallocating, repurposing, or //! otherwise invalidating the memory used to store the data is restricted, too. //! Concretely, for pinned data you have to maintain the invariant //! that *its memory will not get invalidated from the moment it gets pinned until //! when `drop` is called*. Memory can be invalidated by deallocation, but also by -//! replacing a `Some(v)` by `None`, or calling `Vec::set_len` to "kill" some elements +//! replacing a [`Some(v)`] by [`None`], or calling [`Vec::set_len`] to "kill" some elements //! off of a vector. //! //! This is exactly the kind of guarantee that the intrusive linked list from the previous @@ -174,7 +174,7 @@ //! One interesting question arises when considering the interaction of pinning and //! the fields of a struct. When can a struct have a "pinning projection", i.e., //! an operation with type `fn(Pin<&[mut] Struct>) -> Pin<&[mut] Field>`? -//! In a similar vein, when can a generic wrapper type (such as `Vec`, `Box`, or `RefCell`) +//! In a similar vein, when can a generic wrapper type (such as `Vec`, `Box`, or `RefCell`) //! have an operation with type `fn(Pin<&[mut] Wrapper>) -> Pin<&[mut] T>`? //! //! Having a pinning projection for some field means that pinning is "structural": @@ -199,7 +199,7 @@ //! 3. You must make sure that you uphold the [`Drop` guarantee][drop-guarantee]: //! once your wrapper is pinned, the memory that contains the //! content is not overwritten or deallocated without calling the content's destructors. -//! This can be tricky, as witnessed by `VecDeque`: the destructor of `VecDeque` can fail +//! This can be tricky, as witnessed by `VecDeque`: the destructor of `VecDeque` can fail //! to call `drop` on all elements if one of the destructors panics. This violates the //! `Drop` guarantee, because it can lead to elements being deallocated without //! their destructor being called. (`VecDeque` has no pinning projections, so this @@ -208,31 +208,31 @@ //! the fields when your type is pinned. For example, if the wrapper contains an //! `Option` and there is a `take`-like operation with type //! `fn(Pin<&mut Wrapper>) -> Option`, -//! that operation can be used to move a `T` out of a pinned `Wrapper` -- which means +//! that operation can be used to move a `T` out of a pinned `Wrapper` -- which means //! pinning cannot be structural. //! -//! For a more complex example of moving data out of a pinnd type, imagine if `RefCell` +//! For a more complex example of moving data out of a pinned type, imagine if `RefCell` //! had a method `fn get_pin_mut(self: Pin<&mut Self>) -> Pin<&mut T>`. //! Then we could do the following: //! ```compile_fail //! fn exploit_ref_cell(rc: Pin<&mut RefCell) { -//! { let p = rc.as_mut().get_pin_mut(); } // here we get pinned access to the `T` +//! { let p = rc.as_mut().get_pin_mut(); } // Here we get pinned access to the `T`. //! let rc_shr: &RefCell = rc.into_ref().get_ref(); //! let b = rc_shr.borrow_mut(); -//! let content = &mut *b; // and here we have `&mut T` to the same data +//! let content = &mut *b; // And here we have `&mut T` to the same data. //! } //! ``` -//! This is catastrophic, it means we can first pin the content of the `RefCell` +//! This is catastrophic, it means we can first pin the content of the `RefCell` //! (using `RefCell::get_pin_mut`) and then move that content using the mutable //! reference we got later. //! -//! For a type like `Vec`, both possibilites (structural pinning or not) make sense, -//! and the choice is up to the author. A `Vec` with structural pinning could +//! For a type like `Vec`, both possibilites (structural pinning or not) make sense, +//! and the choice is up to the author. A `Vec` with structural pinning could //! have `get_pin`/`get_pin_mut` projections. However, it could *not* allow calling -//! `pop` on a pinned `Vec` because that would move the (structurally pinned) contents! +//! `pop` on a pinned `Vec` because that would move the (structurally pinned) contents! //! Nor could it allow `push`, which might reallocate and thus also move the contents. -//! A `Vec` without structural pinning could `impl Unpin for Vec`, because the contents -//! are never pinned and the `Vec` itself is fine with being moved as well. +//! A `Vec` without structural pinning could `impl Unpin for Vec`, because the contents +//! are never pinned and the `Vec` itself is fine with being moved as well. //! //! In the standard library, pointer types generally do not have structural pinning, //! and thus they do not offer pinning projections. This is why `Box: Unpin` holds for all `T`. @@ -244,13 +244,16 @@ //! whether the content is pinned is entirely independent of whether the pointer is //! pinned, meaning pinning is *not* structural. //! -//! [`Pin`]: struct.Pin.html +//! [`Pin

`]: struct.Pin.html //! [`Unpin`]: ../../std/marker/trait.Unpin.html //! [`Deref`]: ../../std/ops/trait.Deref.html //! [`DerefMut`]: ../../std/ops/trait.DerefMut.html //! [`mem::swap`]: ../../std/mem/fn.swap.html //! [`mem::forget`]: ../../std/mem/fn.forget.html -//! [`Box`]: ../../std/boxed/struct.Box.html +//! [`Box`]: ../../std/boxed/struct.Box.html +//! [`Vec::set_len`]: ../../std/vec/struct.Vec.html#method.set_len +//! [`None`]: ../../std/option/enum.Option.html#variant.None +//! [`Some(v)`]: ../../std/option/enum.Option.html#variant.Some //! [drop-impl]: #drop-implementation //! [drop-guarantee]: #drop-guarantee @@ -328,11 +331,11 @@ impl Pin

where P::Target: Unpin, { - /// Construct a new `Pin` around a pointer to some data of a type that + /// Construct a new `Pin

` around a pointer to some data of a type that /// implements [`Unpin`]. /// /// Unlike `Pin::new_unchecked`, this method is safe because the pointer - /// `P` dereferences to an [`Unpin`] type, which nullifies the pinning guarantees. + /// `P` dereferences to an [`Unpin`] type, which cancels the pinning guarantees. /// /// [`Unpin`]: ../../std/marker/trait.Unpin.html #[stable(feature = "pin", since = "1.33.0")] @@ -345,7 +348,7 @@ where } impl Pin

{ - /// Construct a new `Pin` around a reference to some data of a type that + /// Construct a new `Pin

` around a reference to some data of a type that /// may or may not implement `Unpin`. /// /// If `pointer` dereferences to an `Unpin` type, `Pin::new` should be used @@ -379,13 +382,13 @@ impl Pin

{ /// fn move_pinned_ref(mut a: T, mut b: T) { /// unsafe { let p = Pin::new_unchecked(&mut a); } // should mean `a` can never move again /// mem::swap(&mut a, &mut b); - /// // the address of `a` changed to `b`'s stack slot, so `a` got moved even + /// // The address of `a` changed to `b`'s stack slot, so `a` got moved even /// // though we have previously pinned it! /// } /// ``` /// A value, once pinned, must remain pinned forever (unless its type implements `Unpin`). /// - /// Similarily, calling `Pin::new_unchecked` on a `Rc` is unsafe because there could be + /// Similarily, calling `Pin::new_unchecked` on an `Rc` is unsafe because there could be /// aliases to the same data that are not subject to the pinning restrictions: /// ``` /// use std::rc::Rc; @@ -482,7 +485,7 @@ impl<'a, T: ?Sized> Pin<&'a T> { /// It may seem like there is an issue here with interior mutability: in fact, /// it *is* possible to move a `T` out of a `&RefCell`. However, this is /// not a problem as long as there does not also exist a `Pin<&T>` pointing - /// to the same data, and `RefCell` does not let you create a pinned reference + /// to the same data, and `RefCell` does not let you create a pinned reference /// to its contents. See the discussion on ["pinning projections"] for further /// details. ///