Skip to content

Commit a8d18b9

Browse files
committed
apply some of the feedback
1 parent 4059889 commit a8d18b9

File tree

2 files changed

+34
-25
lines changed

2 files changed

+34
-25
lines changed

src/libcore/marker.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -597,11 +597,11 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
597597

598598
/// Types which can be safely moved after being pinned.
599599
///
600-
/// Since Rust itself has no notion of immovable types, and will consider moves
600+
/// Since Rust itself has no notion of immovable types, and considers moves
601601
/// (e.g. through assignment or [`mem::replace`]) to always be safe,
602602
/// this trait cannot prevent types from moving by itself.
603603
///
604-
/// Instead it can be used to prevent moves through the type system,
604+
/// Instead it is used to prevent moves through the type system,
605605
/// by controlling the behavior of pointers wrapped in the [`Pin`] wrapper,
606606
/// which "pin" the type in place by not allowing it to be moved out of them.
607607
/// See the [`pin module`] documentation for more information on pinning.
@@ -610,7 +610,7 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
610610
/// which then allows it to move out with functions such as [`mem::replace`].
611611
///
612612
/// `Unpin` has no consequence at all for non-pinned data. In particular,
613-
/// [`mem::replace`] will happily move `!Unpin` data. However, you cannot use
613+
/// [`mem::replace`] happily moves `!Unpin` data. However, you cannot use
614614
/// [`mem::replace`] on data wrapped inside a [`Pin`], and *that* is what makes
615615
/// this system work.
616616
///

src/libcore/pin.rs

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@
3232
//! # `Unpin`
3333
//!
3434
//! However, these restrictions are usually not necessary. Many types are always freely
35-
//! movable, even when pinned. These types implement the [`Unpin`] auto-trait, which
35+
//! movable, even when pinned, because they do not rely on having a stable address.
36+
//! These types implement the [`Unpin`] auto-trait, which
3637
//! nullifies the effect of [`Pin`]. For `T: Unpin`, `Pin<Box<T>>` and `Box<T>` function
3738
//! identically, as do `Pin<&mut T>` and `&mut T`.
3839
//!
@@ -99,20 +100,22 @@
99100
//! # `Drop` guarantee
100101
//!
101102
//! The purpose of pinning is to be able to rely on the placement of some data in memory.
102-
//! To make this work, not just moving the data is restricted; deallocating or overwriting
103-
//! it is restricted, too. Concretely, for pinned data you have to maintain the invariant
104-
//! that *it will not get overwritten or deallocated until `drop` was called*.
105-
//! ("Overwriting" here refers to other ways of invalidating storage, such as switching
106-
//! from one enum variant to another.)
103+
//! To make this work, not just moving the data is restricted; deallocating, repurposing or
104+
//! otherwise invalidating the memory used to store the data is restricted, too.
105+
//! Concretely, for pinned data you have to maintain the invariant
106+
//! that *its memory will not get invalidated from the momentit gets pinned until
107+
//! when `drop` is called*. Memory can be invalidated by deallocation, but also by
108+
//! replacing a `Some(v)` by `None`, or calling `Vec::set_len` to "kill" some elements
109+
//! off of a vector.
107110
//!
108111
//! The purpose of this guarantee is to allow data structures that store pointers
109112
//! to pinned data. For example, in an intrusive doubly-linked list, every element
110113
//! will have pointers to its predecessor and successor in the list. Every element
111114
//! will be pinned, because moving the elements around would invalidate the pointers.
112-
//! Moreover, the `Drop` implemenetation of a linked list element will patch the pointers
115+
//! Moreover, the `Drop` implementation of a linked list element will patch the pointers
113116
//! of its predecessor and successor to remove itself from the list. Clearly, if an element
114117
//! could be deallocated or overwritten without calling `drop`, the pointers into it
115-
//! from its neighbouring elements would become invalid, breaking the data structure.
118+
//! from its neighbouring elements would become invalid, which would break the data structure.
116119
//!
117120
//! Notice that this guarantee does *not* mean that memory does not leak! It is still
118121
//! completely okay not to ever call `drop` on a pinned element (e.g., you can still
@@ -130,7 +133,9 @@
130133
//! a problem in safe code because implementing a type that relies on pinning
131134
//! requires unsafe code, but be aware that deciding to make use of pinning
132135
//! in your type (for example by implementing some operation on `Pin<&[mut] Self>`)
133-
//! has consequences for your `Drop` implemenetation as well.
136+
//! has consequences for your `Drop` implementation as well: if an element
137+
//! of your type could have been pinned, you must treat Drop as implicitly taking
138+
//! `Pin<&mut Self>`.
134139
//!
135140
//! # Projections and Structural Pinning
136141
//!
@@ -151,12 +156,12 @@
151156
//! whether projections are provided. However, there are a couple requirements to be
152157
//! upheld when adding projection operations:
153158
//!
154-
//! 1. The container must only be [`Unpin`] if all its fields are `Unpin`. This is the default,
155-
//! but `Unpin` is a safe trait, so as the author of the container it is your responsibility
156-
//! *not* to add something like `impl<T> Unpin for Container<T>`. (Notice that adding a
157-
//! projection operation requires unsafe code, so the fact that `Unpin` is a safe trait
158-
//! does not break the principle that you only have to worry about any of this if
159-
//! you use `unsafe`.)
159+
//! 1. The container must only be [`Unpin`] if all the fields one can project to are
160+
//! `Unpin`. This is the default, but `Unpin` is a safe trait, so as the author of
161+
//! the container it is your responsibility *not* to add something like
162+
//! `impl<T> Unpin for Container<T>`. (Notice that adding a projection operation
163+
//! requires unsafe code, so the fact that `Unpin` is a safe trait does not break
164+
//! the principle that you only have to worry about any of this if you use `unsafe`.)
160165
//! 2. The destructor of the container must not move out of its argument. This is the exact
161166
//! point that was raised in the [previous section][drop-impl]: `drop` takes `&mut self`,
162167
//! but the container (and hence its fields) might have been pinned before.
@@ -293,9 +298,13 @@ impl<P: Deref> Pin<P> {
293298
/// # Safety
294299
///
295300
/// This constructor is unsafe because we cannot guarantee that the data
296-
/// pointed to by `pointer` is pinned. If the constructed `Pin<P>` does
301+
/// pointed to by `pointer` is pinned forever. If the constructed `Pin<P>` does
297302
/// not guarantee that the data `P` points to is pinned, constructing a
298-
/// `Pin<P>` is undefined behavior.
303+
/// `Pin<P>` is unsafe. In particular, calling `Pin::new_unchecked`
304+
/// on an `&'a mut T` is unsafe because while you are able to pin it for the given
305+
/// lifetime `'a`, you have no control over whether it is kept pinned once `'a`
306+
/// ends. A value, once pinned, must remain pinned forever
307+
/// (unless its type implements `Unpin`).
299308
///
300309
/// By using this method, you are making a promise about the `P::Deref` and
301310
/// `P::DerefMut` implementations, if they exist. Most importantly, they
@@ -305,25 +314,25 @@ impl<P: Deref> Pin<P> {
305314
/// Moreover, by calling this method you promise that the reference `P`
306315
/// dereferences to will not be moved out of again; in particular, it
307316
/// must not be possible to obtain a `&mut P::Target` and then
308-
/// move out of that reference (using, for example [`replace`]).
317+
/// move out of that reference (using, for example [`mem::swap`]).
309318
///
310319
/// For example, the following is a *violation* of `Pin`'s safety:
311320
/// ```
312321
/// use std::mem;
313322
/// use std::pin::Pin;
314323
///
315-
/// fn foo<T>(mut a: T, b: T) {
324+
/// fn foo<T>(mut a: T, mut b: T) {
316325
/// unsafe { let p = Pin::new_unchecked(&mut a); } // should mean `a` can never move again
317-
/// let a2 = mem::replace(&mut a, b);
318-
/// // the address of `a` changed to `a2`'s stack slot, so `a` got moved even
326+
/// mem::swap(&mut a, &mut b);
327+
/// // the address of `a` changed to `b`'s stack slot, so `a` got moved even
319328
/// // though we have previously pinned it!
320329
/// }
321330
/// ```
322331
///
323332
/// If `pointer` dereferences to an `Unpin` type, `Pin::new` should be used
324333
/// instead.
325334
///
326-
/// [`replace`]: ../../std/mem/fn.replace.html
335+
/// [`mem::swap`]: ../../std/mem/fn.swap.html
327336
#[stable(feature = "pin", since = "1.33.0")]
328337
#[inline(always)]
329338
pub unsafe fn new_unchecked(pointer: P) -> Pin<P> {

0 commit comments

Comments
 (0)