Skip to content

Commit

Permalink
Improve chapter about Vec<T> (#381)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Henry-Mantilla <[email protected]>
  • Loading branch information
Noratrieb and danielhenrymantilla authored Nov 21, 2022
1 parent 03fbddb commit ae406aa
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 33 deletions.
1 change: 0 additions & 1 deletion src/vec/vec-alloc.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ impl<T> Vec<T> {
ptr: NonNull::dangling(),
len: 0,
cap: 0,
_marker: PhantomData,
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/vec/vec-final.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::ptr::{self, NonNull};
struct RawVec<T> {
ptr: NonNull<T>,
cap: usize,
_marker: PhantomData<T>,
}

unsafe impl<T: Send> Send for RawVec<T> {}
Expand All @@ -25,7 +24,6 @@ impl<T> RawVec<T> {
RawVec {
ptr: NonNull::dangling(),
cap: cap,
_marker: PhantomData,
}
}

Expand Down
16 changes: 10 additions & 6 deletions src/vec/vec-insert-remove.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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
}
}
Expand Down
14 changes: 6 additions & 8 deletions src/vec/vec-into-iter.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ pub struct IntoIter<T> {
cap: usize,
start: *const T,
end: *const T,
_marker: PhantomData<T>,
}
```

Expand All @@ -61,13 +60,13 @@ impl<T> IntoIterator for Vec<T> {
type Item = T;
type IntoIter = IntoIter<T>;
fn into_iter(self) -> IntoIter<T> {
// 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 {
Expand All @@ -80,7 +79,6 @@ impl<T> IntoIterator for Vec<T> {
} else {
ptr.as_ptr().add(len)
},
_marker: PhantomData,
}
}
}
Expand Down
19 changes: 8 additions & 11 deletions src/vec/vec-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@ pub struct Vec<T> {
}
```

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<T>` in place of
`*mut T` when it has a raw pointer to an allocation that it owns. Unique is unstable,
Expand All @@ -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<T>` needs this)
* We are Send/Sync if `T` is Send/Sync
* Our pointer is never null (so `Option<Vec<T>>` is null-pointer-optimized)

We can implement all of the above requirements in stable Rust. To do this, instead
of using `Unique<T>` we will use [`NonNull<T>`][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<T>` (for drop
check) and implementing Send/Sync if `T` is, we get the same results as using
`Unique<T>`:
over `T` and is declared to never be null. By implementing Send/Sync if `T` is,
we get the same results as using `Unique<T>`:

```rust
use std::ptr::NonNull;
Expand All @@ -49,7 +46,6 @@ pub struct Vec<T> {
ptr: NonNull<T>,
cap: usize,
len: usize,
_marker: PhantomData<T>,
}

unsafe impl<T: Send> Send for Vec<T> {}
Expand All @@ -58,4 +54,5 @@ unsafe impl<T: Sync> Sync for Vec<T> {}
```

[ownership]: ../ownership.html
[phantom-data]: ../phantom-data.md
[NonNull]: ../../std/ptr/struct.NonNull.html
2 changes: 0 additions & 2 deletions src/vec/vec-raw.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ allocating, growing, and freeing:
struct RawVec<T> {
ptr: NonNull<T>,
cap: usize,
_marker: PhantomData<T>,
}
unsafe impl<T: Send> Send for RawVec<T> {}
Expand All @@ -25,7 +24,6 @@ impl<T> RawVec<T> {
RawVec {
ptr: NonNull::dangling(),
cap: 0,
_marker: PhantomData,
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/vec/vec-zsts.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@ method of `RawVec`.
```rust,ignore
impl<T> RawVec<T> {
fn new() -> Self {
// !0 is usize::MAX. This branch should be stripped at compile time.
let cap = if mem::size_of::<T>() == 0 { !0 } else { 0 };
// This branch should be stripped at compile time.
let cap = if mem::size_of::<T>() == 0 { usize::MAX } else { 0 };
// `NonNull::dangling()` doubles as "unallocated" and "zero-sized allocation"
RawVec {
ptr: NonNull::dangling(),
cap: cap,
_marker: PhantomData,
}
}
Expand Down

0 comments on commit ae406aa

Please sign in to comment.