rewrite the splice drop algo to the same std Vec uses

My initial idea stashing the tail aside didnt worked out as well as i hoped for.
Also adds a good deal of tests generated by xmacros. Some may be removed later, but I
tried do cover some corner cases (begin/middle/end, larger/same/shorter replacements)

Since we using a a lot unsafe code eventually all code paths should be checked
(see cargo mutants). The existing tests passing under miri.

Author: cehteh

src/drain.rs

@@ -32,7 +32,7 @@ pub struct Drain<'a, H, T> {
     /// Index of tail to preserve
     pub(super) tail_start: usize,
     /// End index of tail to preserve
-    pub(super) tail_end: usize,
+    pub(super) tail_len: usize,
     /// Current remaining range to remove
     pub(super) iter: slice::Iter<'a, T>,
     pub(super) vec: NonNull<HeaderVec<H, T>>,
@@ -113,16 +113,12 @@ impl<H, T> Drain<'_, H, T> {
             if tail != (start + unyielded_len) {
                 let src = source_vec.as_ptr().add(tail);
                 let dst = start_ptr.add(unyielded_len);
-                ptr::copy(src, dst, this.tail_len());
+                ptr::copy(src, dst, this.tail_len);
             }
 
-            source_vec.set_len(start + unyielded_len + this.tail_len());
+            source_vec.set_len(start + unyielded_len + this.tail_len);
         }
     }
-
-    pub(crate) fn tail_len(&self) -> usize {
-        self.tail_end - self.tail_start
-    }
 }
 
 impl<H, T> AsRef<[T]> for Drain<'_, H, T> {
@@ -165,7 +161,7 @@ impl<H, T> Drop for Drain<'_, H, T> {
 
         impl<H, T> Drop for DropGuard<'_, '_, H, T> {
             fn drop(&mut self) {
-                if self.0.tail_len() > 0 {
+                if self.0.tail_len > 0 {
                     unsafe {
                         let source_vec = self.0.vec.as_mut();
                         // memmove back untouched tail, update to new length
@@ -174,9 +170,9 @@ impl<H, T> Drop for Drain<'_, H, T> {
                         if tail != start {
                             let src = source_vec.as_ptr().add(tail);
                             let dst = source_vec.as_mut_ptr().add(start);
-                            ptr::copy(src, dst, self.0.tail_len());
+                            ptr::copy(src, dst, self.0.tail_len);
                         }
-                        source_vec.set_len(start + self.0.tail_len());
+                        source_vec.set_len(start + self.0.tail_len);
                     }
                 }
             }

src/lib.rs

@@ -861,7 +861,7 @@ impl<H, T> HeaderVec<H, T> {
             let range_slice = slice::from_raw_parts(self.as_ptr().add(start), end - start);
             Drain {
                 tail_start: end,
-                tail_end: len,
+                tail_len: len - end,
                 iter: range_slice.iter(),
                 vec: NonNull::from(self),
             }

src/splice.rs

@@ -1,6 +1,6 @@
 #![cfg(feature = "std")]
 
-use core::{any::type_name, fmt, ptr};
+use core::{any::type_name, fmt, ptr, slice};
 
 use crate::{Drain, WeakFixupFn};
 
@@ -77,104 +77,87 @@ impl<H, I: Iterator> Drop for Splice<'_, H, I> {
         // Which means we can replace the slice::Iter with pointers that won't point to deallocated
         // memory, so that Drain::drop is still allowed to call iter.len(), otherwise it would break
         // the ptr.sub_ptr contract.
-        self.drain.iter = [].iter();
-
-        // We will use the replace_with iterator to append elements in place on self.drain.vec.
-        // When this hits the tail then elements are moved from the tail to tmp_tail.
-        // When the tail is or becomes empty by that, then the remaining elements can be extended to the vec.
-        //
-        // Finally:
-        // Then have continuous elements in the vec:  |head|replace_with|(old_tail|)spare_capacity|.
-        // The old tail needs to be moved to its final destination.
-        // Perhaps making space for the elements in the tmp_tail.
-        let mut tmp_tail = Vec::new();
 
         unsafe {
             let vec = self.drain.vec.as_mut();
-            loop {
-                if self.drain.tail_len() == 0 {
-                    // If the tail is empty, we can just extend the vector with the remaining elements.
-                    // but we may have stashed some tmp_tail away and should reserve for that.
-                    // PLANNED: should become 'extend_reserve()'
-                    vec.reserve_intern(
-                        self.replace_with.size_hint().0 + tmp_tail.len(),
-                        false,
-                        &mut self.weak_fixup,
-                    );
-                    vec.extend(self.replace_with.by_ref());
-                    // in case the size_hint was not exact (or returned 0) we need to reserve for the tmp_tail
-                    // in most cases this will not allocate. later we expect that we have this space reserved.
-                    vec.reserve_intern(tmp_tail.len(), false, &mut self.weak_fixup);
-                    break;
-                } else if let Some(next) = self.replace_with.next() {
-                    if vec.len_exact() >= self.drain.tail_start && self.drain.tail_len() > 0 {
-                        // move one element from the tail to the tmp_tail
-                        // We reserve for as much elements are hinted by replace_with or the remaining tail,
-                        // whatever is smaller.
-                        tmp_tail
-                            .reserve(self.replace_with.size_hint().0.min(self.drain.tail_len()));
-                        tmp_tail.push(ptr::read(vec.as_ptr().add(self.drain.tail_start)));
-                        self.drain.tail_start += 1;
-                    }
-
-                    // since we overwrite the old tail here this will never reallocate.
-                    // PLANNED: vec.push_within_capacity().unwrap_unchecked()
-                    vec.push(next);
-                } else {
-                    // replace_with is depleted
-                    break;
-                }
+
+            if self.drain.tail_len == 0 {
+                vec.extend(self.replace_with.by_ref());
+                return;
             }
 
-            let tail_len = self.drain.tail_len();
-            if tail_len > 0 {
-                // In case we need to shift the tail farther back we need to reserve space for that.
-                // Reserve needs to preserve the tail we have, thus we temporarily set the length to the
-                // tail_end and then restore it after the reserve.
-                let old_len = vec.len_exact();
-                vec.set_len(self.drain.tail_end);
-                vec.reserve_intern(tmp_tail.len(), false, &mut self.weak_fixup);
-                vec.set_len(old_len);
-
-                // now we can move the tail around
-                ptr::copy(
-                    vec.as_ptr().add(self.drain.tail_start),
-                    vec.as_mut_ptr().add(vec.len_exact() + tmp_tail.len()),
-                    tail_len,
-                );
-
-                // all elements are moved from the tail, ensure that Drain drop does nothing.
-                // PLANNED: eventually we may not need use Drain here
-                self.drain.tail_start = self.drain.tail_end;
+            // First fill the range left by drain().
+            if !self.drain.fill(&mut self.replace_with) {
+                return;
             }
 
-            let tmp_tail_len = tmp_tail.len();
-            if !tmp_tail.is_empty() {
-                // When we stashed tail elements to tmp_tail, then fill the gap
-                tmp_tail.set_len(0);
-                ptr::copy_nonoverlapping(
-                    tmp_tail.as_ptr(),
-                    vec.as_mut_ptr().add(vec.len_exact()),
-                    tmp_tail_len,
-                );
+            // There may be more elements. Use the lower bound as an estimate.
+            // FIXME: Is the upper bound a better guess? Or something else?
+            let (lower_bound, _upper_bound) = self.replace_with.size_hint();
+            if lower_bound > 0 {
+                self.drain.move_tail(lower_bound, &mut self.weak_fixup);
+                if !self.drain.fill(&mut self.replace_with) {
+                    return;
+                }
             }
 
-            // finally fix the vec length
-            let new_len = vec.len_exact() + tmp_tail_len + tail_len;
-            vec.set_len(new_len);
+            // Collect any remaining elements.
+            // This is a zero-length vector which does not allocate if `lower_bound` was exact.
+            let mut collected = self
+                .replace_with
+                .by_ref()
+                .collect::<Vec<I::Item>>()
+                .into_iter();
+            // Now we have an exact count.
+            if collected.len() > 0 {
+                self.drain.move_tail(collected.len(), &mut self.weak_fixup);
+                let filled = self.drain.fill(&mut collected);
+                debug_assert!(filled);
+                debug_assert_eq!(collected.len(), 0);
+            }
+        }
+    }
+}
+
+/// Private helper methods for `Splice::drop`
+impl<H, T> Drain<'_, H, T> {
+    /// The range from `self.vec.len` to `self.tail_start` contains elements
+    /// that have been moved out.
+    /// Fill that range as much as possible with new elements from the `replace_with` iterator.
+    /// Returns `true` if we filled the entire range. (`replace_with.next()` didn’t return `None`.)
+    unsafe fn fill<I: Iterator<Item = T>>(&mut self, replace_with: &mut I) -> bool {
+        let vec = unsafe { self.vec.as_mut() };
+        let range_start = vec.len_exact();
+        let range_end = self.tail_start;
+        let range_slice = unsafe {
+            slice::from_raw_parts_mut(vec.as_mut_ptr().add(range_start), range_end - range_start)
+        };
+
+        for place in range_slice {
+            if let Some(new_item) = replace_with.next() {
+                unsafe { ptr::write(place, new_item) };
+                let len = vec.len_exact();
+                vec.set_len(len + 1);
+            } else {
+                return false;
+            }
         }
+        true
+    }
 
-        // IDEA: implement and benchmark batched copying. This leaves a gap in front of the tails which
-        //       needs to be filled before resizing.
-        //       Batch size:
-        //       Moving one element per iteration to the tmp_tail is not efficient to make space for
-        //       a element from the replace_with. Thus we determine a number of elements that we
-        //       transfer in a batch to the tmp_tail. We compute the batch size to be roughly 4kb
-        //       (Common page size on many systems) (or I::Item, whatever is larger) or the size of
-        //       the tail when it is smaller. The later ensure that we do a single reserve with the
-        //       minimum space needed when the tail is smaller than a batch would be .
-        //       let batch_size = (4096 / std::mem::size_of::<I::Item>())
-        //           .max(1)
-        //           .min(self.drain.tail_len);
+    /// Makes room for inserting more elements before the tail.
+    #[track_caller]
+    unsafe fn move_tail(&mut self, additional: usize, weak_fixup: &mut Option<WeakFixupFn<'_>>) {
+        let vec = unsafe { self.vec.as_mut() };
+        let len = self.tail_start + self.tail_len;
+        vec.reserve_intern(len + additional, false, weak_fixup);
+
+        let new_tail_start = self.tail_start + additional;
+        unsafe {
+            let src = vec.as_ptr().add(self.tail_start);
+            let dst = vec.as_mut_ptr().add(new_tail_start);
+            ptr::copy(src, dst, self.tail_len);
+        }
+        self.tail_start = new_tail_start;
     }
 }