Make the Vec impl be slightly more careful with ZSTs and alignment.

This also incidentally makes the ZST code and final code have the same formatting for the divide.

src/vec/vec-final.md

@@ -239,21 +239,22 @@ impl<T> Iterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                let result = ptr::read(self.start);
+                if mem::size_of::<T>() == 0 {
-                self.start = if mem::size_of::<T>() == 0 {
+                    self.start = (self.start as usize + 1) as *const _;
-                    (self.start as usize + 1) as *const _
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.start.offset(1)
+                    let old_ptr = self.start;
-                };
+                    self.start = self.start.offset(1);
-                Some(result)
+                    Some(ptr::read(old_ptr))
+                }
             }
         }
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
         let elem_size = mem::size_of::<T>();
-        let len = (self.end as usize - self.start as usize) /
+        let len = (self.end as usize - self.start as usize)
-                  if elem_size == 0 { 1 } else { elem_size };
+                  / if elem_size == 0 { 1 } else { elem_size };
         (len, Some(len))
     }
 }
@@ -264,15 +265,16 @@ impl<T> DoubleEndedIterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                self.end = if mem::size_of::<T>() == 0 {
+                if mem::size_of::<T>() == 0 {
-                    (self.end as usize - 1) as *const _
+                    self.end = (self.end as usize - 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.end.offset(-1)
+                    self.end = self.end.offset(-1);
-                };
                     Some(ptr::read(self.end))
                 }
             }
         }
+    }
 }
 
 pub struct IntoIter<T> {

src/vec/vec-zsts.md

@@ -130,13 +130,11 @@ Now we have a different bug. Instead of our iterators not running at all, our
 iterators now run *forever*. We need to do the same trick in our iterator impls.
 Also, our size_hint computation code will divide by 0 for ZSTs. Since we'll
 basically be treating the two pointers as if they point to bytes, we'll just
-map size 0 to divide by 1.
+map size 0 to divide by 1. Here's what `next` will be:
 
 <!-- ignore: simplified code -->
 ```rust,ignore
-impl<T> Iterator for RawValIter<T> {
+fn next(&mut self) -> Option<T> {
-    type Item = T;
-    fn next(&mut self) -> Option<T> {
     if self.start == self.end {
         None
     } else {
@@ -150,6 +148,43 @@ impl<T> Iterator for RawValIter<T> {
             Some(result)
         }
     }
+}
+```
+
+Do you see the "bug"? No one else did! The original author only noticed the
+problem when linking to this page years later. This code is kind of dubious
+because abusing the iterator pointers to be *counters* makes them unaligned!
+Our *one job* when using ZSTs is to keep pointers aligned! *forehead slap*
+
+Raw pointers don't need to be aligned at all times, so the basic trick of
+using pointers as counters is *fine*, but they *should* definitely be aligned
+when passed to `ptr::read`! This is *possibly* needless pedantry
+because `ptr::read` is a noop for a ZST, but let's be a *little* more
+responsible and read from `NonNull::dangling` on the ZST path.
+
+(Alternatively you could call `read_unaligned` on the ZST path. Either is fine,
+because either way we're making up a value from nothing and it all compiles
+to doing nothing.)
+
+<!-- ignore: simplified code -->
+```rust,ignore
+impl<T> Iterator for RawValIter<T> {
+    type Item = T;
+    fn next(&mut self) -> Option<T> {
+        if self.start == self.end {
+            None
+        } else {
+            unsafe {
+                if mem::size_of::<T>() == 0 {
+                    self.start = (self.start as usize + 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
+                } else {
+                    let old_ptr = self.start;
+                    self.start = self.start.offset(1);
+                    Some(ptr::read(old_ptr))
+                }
+            }
+        }
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -166,15 +201,16 @@ impl<T> DoubleEndedIterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                self.end = if mem::size_of::<T>() == 0 {
+                if mem::size_of::<T>() == 0 {
-                    (self.end as usize - 1) as *const _
+                    self.end = (self.end as usize - 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.end.offset(-1)
+                    self.end = self.end.offset(-1);
-                };
                     Some(ptr::read(self.end))
                 }
             }
         }
+    }
 }
 ```