From ae406aa5287a9e025abb72343aaceec98458c117 Mon Sep 17 00:00:00 2001 From: nils <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 21 Nov 2022 23:48:20 +0100 Subject: [PATCH] Improve chapter about `Vec` (#381) Co-authored-by: Daniel Henry-Mantilla --- src/vec/vec-alloc.md | 1 - src/vec/vec-final.md | 2 -- src/vec/vec-insert-remove.md | 16 ++++++++++------ src/vec/vec-into-iter.md | 14 ++++++-------- src/vec/vec-layout.md | 19 ++++++++----------- src/vec/vec-raw.md | 2 -- src/vec/vec-zsts.md | 5 ++--- 7 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/vec/vec-alloc.md b/src/vec/vec-alloc.md index 6c69c93..2495473 100644 --- a/src/vec/vec-alloc.md +++ b/src/vec/vec-alloc.md @@ -28,7 +28,6 @@ impl Vec { ptr: NonNull::dangling(), len: 0, cap: 0, - _marker: PhantomData, } } } diff --git a/src/vec/vec-final.md b/src/vec/vec-final.md index 1f8af37..696391d 100644 --- a/src/vec/vec-final.md +++ b/src/vec/vec-final.md @@ -10,7 +10,6 @@ use std::ptr::{self, NonNull}; struct RawVec { ptr: NonNull, cap: usize, - _marker: PhantomData, } unsafe impl Send for RawVec {} @@ -25,7 +24,6 @@ impl RawVec { RawVec { ptr: NonNull::dangling(), cap: cap, - _marker: PhantomData, } } diff --git a/src/vec/vec-insert-remove.md b/src/vec/vec-insert-remove.md index 57283f9..722e20c 100644 --- a/src/vec/vec-insert-remove.md +++ b/src/vec/vec-insert-remove.md @@ -22,9 +22,11 @@ pub fn insert(&mut self, index: usize, elem: T) { unsafe { // ptr::copy(src, dest, len): "copy from src to dest len elems" - ptr::copy(self.ptr.as_ptr().add(index), - self.ptr.as_ptr().add(index + 1), - self.len - index); + ptr::copy( + self.ptr.as_ptr().add(index), + self.ptr.as_ptr().add(index + 1), + self.len - index, + ); ptr::write(self.ptr.as_ptr().add(index), elem); self.len += 1; } @@ -42,9 +44,11 @@ pub fn remove(&mut self, index: usize) -> T { unsafe { self.len -= 1; let result = ptr::read(self.ptr.as_ptr().add(index)); - ptr::copy(self.ptr.as_ptr().add(index + 1), - self.ptr.as_ptr().add(index), - self.len - index); + ptr::copy( + self.ptr.as_ptr().add(index + 1), + self.ptr.as_ptr().add(index), + self.len - index, + ); result } } diff --git a/src/vec/vec-into-iter.md b/src/vec/vec-into-iter.md index 61782e3..a3a3c8c 100644 --- a/src/vec/vec-into-iter.md +++ b/src/vec/vec-into-iter.md @@ -49,7 +49,6 @@ pub struct IntoIter { cap: usize, start: *const T, end: *const T, - _marker: PhantomData, } ``` @@ -61,13 +60,13 @@ impl IntoIterator for Vec { type Item = T; type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - // Can't destructure Vec since it's Drop - let ptr = self.ptr; - let cap = self.cap; - let len = self.len; - // Make sure not to drop Vec since that would free the buffer - mem::forget(self); + let vec = ManuallyDrop::new(self); + + // Can't destructure Vec since it's Drop + let ptr = vec.ptr; + let cap = vec.cap; + let len = vec.len; unsafe { IntoIter { @@ -80,7 +79,6 @@ impl IntoIterator for Vec { } else { ptr.as_ptr().add(len) }, - _marker: PhantomData, } } } diff --git a/src/vec/vec-layout.md b/src/vec/vec-layout.md index c1c1afc..9129952 100644 --- a/src/vec/vec-layout.md +++ b/src/vec/vec-layout.md @@ -15,13 +15,10 @@ pub struct Vec { } ``` -And indeed this would compile. Unfortunately, it would be incorrect. First, the +And indeed this would compile. Unfortunately, it would be too strict. The compiler will give us too strict variance. So a `&Vec<&'static str>` -couldn't be used where an `&Vec<&'a str>` was expected. More importantly, it -will give incorrect ownership information to the drop checker, as it will -conservatively assume we don't own any values of type `T`. See [the chapter -on ownership and lifetimes][ownership] for all the details on variance and -drop check. +couldn't be used where a `&Vec<&'a str>` was expected. See [the chapter +on ownership and lifetimes][ownership] for all the details on variance. As we saw in the ownership chapter, the standard library uses `Unique` in place of `*mut T` when it has a raw pointer to an allocation that it owns. Unique is unstable, @@ -30,16 +27,16 @@ so we'd like to not use it if possible, though. As a recap, Unique is a wrapper around a raw pointer that declares that: * We are covariant over `T` -* We may own a value of type `T` (for drop check) +* We may own a value of type `T` (this is not relevant for our example here, but see + [the chapter on PhantomData][phantom-data] on why the real `std::vec::Vec` needs this) * We are Send/Sync if `T` is Send/Sync * Our pointer is never null (so `Option>` is null-pointer-optimized) We can implement all of the above requirements in stable Rust. To do this, instead of using `Unique` we will use [`NonNull`][NonNull], another wrapper around a raw pointer, which gives us two of the above properties, namely it is covariant -over `T` and is declared to never be null. By adding a `PhantomData` (for drop -check) and implementing Send/Sync if `T` is, we get the same results as using -`Unique`: +over `T` and is declared to never be null. By implementing Send/Sync if `T` is, +we get the same results as using `Unique`: ```rust use std::ptr::NonNull; @@ -49,7 +46,6 @@ pub struct Vec { ptr: NonNull, cap: usize, len: usize, - _marker: PhantomData, } unsafe impl Send for Vec {} @@ -58,4 +54,5 @@ unsafe impl Sync for Vec {} ``` [ownership]: ../ownership.html +[phantom-data]: ../phantom-data.md [NonNull]: ../../std/ptr/struct.NonNull.html diff --git a/src/vec/vec-raw.md b/src/vec/vec-raw.md index e86537b..728feaa 100644 --- a/src/vec/vec-raw.md +++ b/src/vec/vec-raw.md @@ -13,7 +13,6 @@ allocating, growing, and freeing: struct RawVec { ptr: NonNull, cap: usize, - _marker: PhantomData, } unsafe impl Send for RawVec {} @@ -25,7 +24,6 @@ impl RawVec { RawVec { ptr: NonNull::dangling(), cap: 0, - _marker: PhantomData, } } diff --git a/src/vec/vec-zsts.md b/src/vec/vec-zsts.md index 73a97ba..8f25297 100644 --- a/src/vec/vec-zsts.md +++ b/src/vec/vec-zsts.md @@ -33,14 +33,13 @@ method of `RawVec`. ```rust,ignore impl RawVec { fn new() -> Self { - // !0 is usize::MAX. This branch should be stripped at compile time. - let cap = if mem::size_of::() == 0 { !0 } else { 0 }; + // This branch should be stripped at compile time. + let cap = if mem::size_of::() == 0 { usize::MAX } else { 0 }; // `NonNull::dangling()` doubles as "unallocated" and "zero-sized allocation" RawVec { ptr: NonNull::dangling(), cap: cap, - _marker: PhantomData, } }