From 3ddcf0929c3dd6bc99ce3f9411b2092440e1211c Mon Sep 17 00:00:00 2001 From: Alexis Beingessner Date: Mon, 20 Jul 2015 11:31:29 -0700 Subject: [PATCH] clean up vec chapter of tarpl --- SUMMARY.md | 1 + vec-alloc.md | 2 +- vec-dealloc.md | 12 ++-- vec-deref.md | 20 +++--- vec-drain.md | 9 ++- vec-insert-remove.md | 9 +-- vec-into-iter.md | 159 ++++--------------------------------------- vec-layout.md | 21 +++--- vec-raw.md | 136 ++++++++++++++++++++++++++++++++++++ 9 files changed, 192 insertions(+), 177 deletions(-) create mode 100644 vec-raw.md diff --git a/SUMMARY.md b/SUMMARY.md index b0d75bf..d8b3487 100644 --- a/SUMMARY.md +++ b/SUMMARY.md @@ -46,6 +46,7 @@ * [Deref](vec-deref.md) * [Insert and Remove](vec-insert-remove.md) * [IntoIter](vec-into-iter.md) + * [RawVec](vec-raw.md) * [Drain](vec-drain.md) * [Handling Zero-Sized Types](vec-zsts.md) * [Final Code](vec-final.md) diff --git a/vec-alloc.md b/vec-alloc.md index e9c9f68..14b9b46 100644 --- a/vec-alloc.md +++ b/vec-alloc.md @@ -46,7 +46,7 @@ Okay, now we can write growing. Roughly, we want to have this logic: if cap == 0: allocate() cap = 1 -else +else: reallocate cap *= 2 ``` diff --git a/vec-dealloc.md b/vec-dealloc.md index 2ae2477..b767caa 100644 --- a/vec-dealloc.md +++ b/vec-dealloc.md @@ -2,13 +2,13 @@ Next we should implement Drop so that we don't massively leak tons of resources. The easiest way is to just call `pop` until it yields None, and then deallocate -our buffer. Note that calling `pop` is uneeded if `T: !Drop`. In theory we can -ask Rust if T needs_drop and omit the calls to `pop`. However in practice LLVM -is *really* good at removing simple side-effect free code like this, so I wouldn't -bother unless you notice it's not being stripped (in this case it is). +our buffer. Note that calling `pop` is unneeded if `T: !Drop`. In theory we can +ask Rust if `T` `needs_drop` and omit the calls to `pop`. However in practice +LLVM is *really* good at removing simple side-effect free code like this, so I +wouldn't bother unless you notice it's not being stripped (in this case it is). -We must not call `heap::deallocate` when `self.cap == 0`, as in this case we haven't -actually allocated any memory. +We must not call `heap::deallocate` when `self.cap == 0`, as in this case we +haven't actually allocated any memory. ```rust,ignore diff --git a/vec-deref.md b/vec-deref.md index 826d763..6460eab 100644 --- a/vec-deref.md +++ b/vec-deref.md @@ -1,13 +1,15 @@ % Deref -Alright! We've got a decent minimal ArrayStack implemented. We can push, we can -pop, and we can clean up after ourselves. However there's a whole mess of functionality -we'd reasonably want. In particular, we have a proper array, but none of the slice -functionality. That's actually pretty easy to solve: we can implement `Deref`. -This will magically make our Vec coerce to and behave like a slice in all sorts of -conditions. +Alright! We've got a decent minimal stack implemented. We can push, we can +pop, and we can clean up after ourselves. However there's a whole mess of +functionality we'd reasonably want. In particular, we have a proper array, but +none of the slice functionality. That's actually pretty easy to solve: we can +implement `Deref`. This will magically make our Vec coerce to, and +behave like, a slice in all sorts of conditions. -All we need is `slice::from_raw_parts`. +All we need is `slice::from_raw_parts`. It will correctly handle empty slices +for us. Later once we set up zero-sized type support it will also Just Work +for those too. ```rust,ignore use std::ops::Deref; @@ -36,5 +38,5 @@ impl DerefMut for Vec { } ``` -Now we have `len`, `first`, `last`, indexing, slicing, sorting, `iter`, `iter_mut`, -and all other sorts of bells and whistles provided by slice. Sweet! +Now we have `len`, `first`, `last`, indexing, slicing, sorting, `iter`, +`iter_mut`, and all other sorts of bells and whistles provided by slice. Sweet! diff --git a/vec-drain.md b/vec-drain.md index df7cf00..b6b2826 100644 --- a/vec-drain.md +++ b/vec-drain.md @@ -2,7 +2,7 @@ Let's move on to Drain. Drain is largely the same as IntoIter, except that instead of consuming the Vec, it borrows the Vec and leaves its allocation -free. For now we'll only implement the "basic" full-range version. +untouched. For now we'll only implement the "basic" full-range version. ```rust,ignore use std::marker::PhantomData; @@ -38,6 +38,9 @@ impl RawValIter { RawValIter { start: slice.as_ptr(), end: if slice.len() == 0 { + // if `len = 0`, then this is not actually allocated memory. + // Need to avoid offsetting because that will give wrong + // information to LLVM via GEP. slice.as_ptr() } else { slice.as_ptr().offset(slice.len() as isize) @@ -137,5 +140,7 @@ impl Vec { } ``` +For more details on the `mem::forget` problem, see the +[section on leaks][leaks]. - +[leaks]: leaking.html diff --git a/vec-insert-remove.md b/vec-insert-remove.md index f21ed22..6f88a77 100644 --- a/vec-insert-remove.md +++ b/vec-insert-remove.md @@ -1,12 +1,13 @@ % Insert and Remove -Something *not* provided but slice is `insert` and `remove`, so let's do those next. +Something *not* provided by slice is `insert` and `remove`, so let's do those +next. Insert needs to shift all the elements at the target index to the right by one. To do this we need to use `ptr::copy`, which is our version of C's `memmove`. -This copies some chunk of memory from one location to another, correctly handling -the case where the source and destination overlap (which will definitely happen -here). +This copies some chunk of memory from one location to another, correctly +handling the case where the source and destination overlap (which will +definitely happen here). If we insert at index `i`, we want to shift the `[i .. len]` to `[i+1 .. len+1]` using the *old* len. diff --git a/vec-into-iter.md b/vec-into-iter.md index d21cf94..566cad7 100644 --- a/vec-into-iter.md +++ b/vec-into-iter.md @@ -11,19 +11,20 @@ allocation. IntoIter needs to be DoubleEnded as well, to enable reading from both ends. Reading from the back could just be implemented as calling `pop`, but reading from the front is harder. We could call `remove(0)` but that would be insanely -expensive. Instead we're going to just use ptr::read to copy values out of either -end of the Vec without mutating the buffer at all. +expensive. Instead we're going to just use ptr::read to copy values out of +either end of the Vec without mutating the buffer at all. To do this we're going to use a very common C idiom for array iteration. We'll -make two pointers; one that points to the start of the array, and one that points -to one-element past the end. When we want an element from one end, we'll read out -the value pointed to at that end and move the pointer over by one. When the two -pointers are equal, we know we're done. +make two pointers; one that points to the start of the array, and one that +points to one-element past the end. When we want an element from one end, we'll +read out the value pointed to at that end and move the pointer over by one. When +the two pointers are equal, we know we're done. Note that the order of read and offset are reversed for `next` and `next_back` For `next_back` the pointer is always *after* the element it wants to read next, while for `next` the pointer is always *at* the element it wants to read next. -To see why this is, consider the case where every element but one has been yielded. +To see why this is, consider the case where every element but one has been +yielded. The array looks like this: @@ -35,6 +36,10 @@ The array looks like this: If E pointed directly at the element it wanted to yield next, it would be indistinguishable from the case where there are no more elements to yield. +Although we don't actually care about it during iteration, we also need to hold +onto the Vec's allocation information in order to free it once IntoIter is +dropped. + So we're going to use the following struct: ```rust,ignore @@ -46,8 +51,8 @@ struct IntoIter { } ``` -One last subtle detail: if our Vec is empty, we want to produce an empty iterator. -This will actually technically fall out doing the naive thing of: +One last subtle detail: if our Vec is empty, we want to produce an empty +iterator. This will actually technically fall out doing the naive thing of: ```text start = ptr @@ -155,139 +160,3 @@ impl Drop for IntoIter { } } ``` - -We've actually reached an interesting situation here: we've duplicated the logic -for specifying a buffer and freeing its memory. Now that we've implemented it and -identified *actual* logic duplication, this is a good time to perform some logic -compression. - -We're going to abstract out the `(ptr, cap)` pair and give them the logic for -allocating, growing, and freeing: - -```rust,ignore - -struct RawVec { - ptr: Unique, - cap: usize, -} - -impl RawVec { - fn new() -> Self { - assert!(mem::size_of::() != 0, "TODO: implement ZST support"); - unsafe { - RawVec { ptr: Unique::new(heap::EMPTY as *mut T), cap: 0 } - } - } - - // unchanged from Vec - fn grow(&mut self) { - unsafe { - let align = mem::align_of::(); - let elem_size = mem::size_of::(); - - let (new_cap, ptr) = if self.cap == 0 { - let ptr = heap::allocate(elem_size, align); - (1, ptr) - } else { - let new_cap = 2 * self.cap; - let ptr = heap::reallocate(*self.ptr as *mut _, - self.cap * elem_size, - new_cap * elem_size, - align); - (new_cap, ptr) - }; - - // If allocate or reallocate fail, we'll get `null` back - if ptr.is_null() { oom() } - - self.ptr = Unique::new(ptr as *mut _); - self.cap = new_cap; - } - } -} - - -impl Drop for RawVec { - fn drop(&mut self) { - if self.cap != 0 { - let align = mem::align_of::(); - let elem_size = mem::size_of::(); - let num_bytes = elem_size * self.cap; - unsafe { - heap::deallocate(*self.ptr as *mut _, num_bytes, align); - } - } - } -} -``` - -And change vec as follows: - -```rust,ignore -pub struct Vec { - buf: RawVec, - len: usize, -} - -impl Vec { - fn ptr(&self) -> *mut T { *self.buf.ptr } - - fn cap(&self) -> usize { self.buf.cap } - - pub fn new() -> Self { - Vec { buf: RawVec::new(), len: 0 } - } - - // push/pop/insert/remove largely unchanged: - // * `self.ptr -> self.ptr()` - // * `self.cap -> self.cap()` - // * `self.grow -> self.buf.grow()` -} - -impl Drop for Vec { - fn drop(&mut self) { - while let Some(_) = self.pop() {} - // deallocation is handled by RawVec - } -} -``` - -And finally we can really simplify IntoIter: - -```rust,ignore -struct IntoIter { - _buf: RawVec, // we don't actually care about this. Just need it to live. - start: *const T, - end: *const T, -} - -// next and next_back literally unchanged since they never referred to the buf - -impl Drop for IntoIter { - fn drop(&mut self) { - // only need to ensure all our elements are read; - // buffer will clean itself up afterwards. - for _ in &mut *self {} - } -} - -impl Vec { - pub fn into_iter(self) -> IntoIter { - unsafe { - // need to use ptr::read to unsafely move the buf out since it's - // not Copy. - let buf = ptr::read(&self.buf); - let len = self.len; - mem::forget(self); - - IntoIter { - start: *buf.ptr, - end: buf.ptr.offset(len as isize), - _buf: buf, - } - } - } -} -``` - -Much better. diff --git a/vec-layout.md b/vec-layout.md index 128ad15..4e44084 100644 --- a/vec-layout.md +++ b/vec-layout.md @@ -13,15 +13,15 @@ pub struct Vec { # fn main() {} ``` -And indeed this would compile. Unfortunately, it would be incorrect. The compiler -will give us too strict variance, so e.g. an `&Vec<&'static str>` couldn't be used -where an `&Vec<&'a str>` was expected. More importantly, it will give incorrect -ownership information to dropck, as it will conservatively assume we don't own -any values of type `T`. See [the chapter on ownership and lifetimes] -(lifetimes.html) for details. +And indeed this would compile. Unfortunately, it would be incorrect. The +compiler will give us too strict variance, so e.g. an `&Vec<&'static str>` +couldn't be used where an `&Vec<&'a str>` was expected. More importantly, it +will give incorrect ownership information to dropck, as it will conservatively +assume we don't own any values of type `T`. See [the chapter on ownership and +lifetimes] (lifetimes.html) for details. -As we saw in the lifetimes chapter, we should use `Unique` in place of `*mut T` -when we have a raw pointer to an allocation we own: +As we saw in the lifetimes chapter, we should use `Unique` in place of +`*mut T` when we have a raw pointer to an allocation we own: ```rust @@ -40,9 +40,10 @@ pub struct Vec { As a recap, Unique is a wrapper around a raw pointer that declares that: -* We own at least one value of type `T` +* We may own a value of type `T` * We are Send/Sync iff `T` is Send/Sync -* Our pointer is never null (and therefore `Option` is null-pointer-optimized) +* Our pointer is never null (and therefore `Option` is + null-pointer-optimized) That last point is subtle. First, it makes `Unique::new` unsafe to call, because putting `null` inside of it is Undefined Behaviour. It also throws a diff --git a/vec-raw.md b/vec-raw.md new file mode 100644 index 0000000..40de019 --- /dev/null +++ b/vec-raw.md @@ -0,0 +1,136 @@ +% RawVec + +We've actually reached an interesting situation here: we've duplicated the logic +for specifying a buffer and freeing its memory. Now that we've implemented it +and identified *actual* logic duplication, this is a good time to perform some +logic compression. + +We're going to abstract out the `(ptr, cap)` pair and give them the logic for +allocating, growing, and freeing: + +```rust,ignore +struct RawVec { + ptr: Unique, + cap: usize, +} + +impl RawVec { + fn new() -> Self { + assert!(mem::size_of::() != 0, "TODO: implement ZST support"); + unsafe { + RawVec { ptr: Unique::new(heap::EMPTY as *mut T), cap: 0 } + } + } + + // unchanged from Vec + fn grow(&mut self) { + unsafe { + let align = mem::align_of::(); + let elem_size = mem::size_of::(); + + let (new_cap, ptr) = if self.cap == 0 { + let ptr = heap::allocate(elem_size, align); + (1, ptr) + } else { + let new_cap = 2 * self.cap; + let ptr = heap::reallocate(*self.ptr as *mut _, + self.cap * elem_size, + new_cap * elem_size, + align); + (new_cap, ptr) + }; + + // If allocate or reallocate fail, we'll get `null` back + if ptr.is_null() { oom() } + + self.ptr = Unique::new(ptr as *mut _); + self.cap = new_cap; + } + } +} + + +impl Drop for RawVec { + fn drop(&mut self) { + if self.cap != 0 { + let align = mem::align_of::(); + let elem_size = mem::size_of::(); + let num_bytes = elem_size * self.cap; + unsafe { + heap::deallocate(*self.ptr as *mut _, num_bytes, align); + } + } + } +} +``` + +And change vec as follows: + +```rust,ignore +pub struct Vec { + buf: RawVec, + len: usize, +} + +impl Vec { + fn ptr(&self) -> *mut T { *self.buf.ptr } + + fn cap(&self) -> usize { self.buf.cap } + + pub fn new() -> Self { + Vec { buf: RawVec::new(), len: 0 } + } + + // push/pop/insert/remove largely unchanged: + // * `self.ptr -> self.ptr()` + // * `self.cap -> self.cap()` + // * `self.grow -> self.buf.grow()` +} + +impl Drop for Vec { + fn drop(&mut self) { + while let Some(_) = self.pop() {} + // deallocation is handled by RawVec + } +} +``` + +And finally we can really simplify IntoIter: + +```rust,ignore +struct IntoIter { + _buf: RawVec, // we don't actually care about this. Just need it to live. + start: *const T, + end: *const T, +} + +// next and next_back literally unchanged since they never referred to the buf + +impl Drop for IntoIter { + fn drop(&mut self) { + // only need to ensure all our elements are read; + // buffer will clean itself up afterwards. + for _ in &mut *self {} + } +} + +impl Vec { + pub fn into_iter(self) -> IntoIter { + unsafe { + // need to use ptr::read to unsafely move the buf out since it's + // not Copy, and Vec implements Drop (so we can't destructure it). + let buf = ptr::read(&self.buf); + let len = self.len; + mem::forget(self); + + IntoIter { + start: *buf.ptr, + end: buf.ptr.offset(len as isize), + _buf: buf, + } + } + } +} +``` + +Much better.