Repository: rust-itertools/itertools
Pull request #911 (2024-04-09 07:42:48)

Remove `ZipSlices` from benchmarks
`ZipSlices` was removed in 0.5.0.
Is there a point of keeping this around?!

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/rust-itertools/itertools/pull/911?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 94.36%. Comparing base [(`6814180`)](https://app.codecov.io/gh/rust-itertools/itertools/commit/68141809aabec2816d3250820c8d6608f3b2293f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) to head [(`e859686`)](https://app.codecov.io/gh/rust-itertools/itertools/pull/911?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Report is 44 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
- Coverage   94.38%   94.36%   -0.03%     
==========================================
  Files          48       48              
  Lines        6665     6815     +150     
==========================================
+ Hits         6291     6431     +140     
- Misses        374      384      +10     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/rust-itertools/itertools/pull/911?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/benches/bench1.rs b/benches/bench1.rs
index f718ce2..53e77b0 100644
--- a/benches/bench1.rs
+++ b/benches/bench1.rs
@@ -7,10 +7,6 @@ use std::cmp;
 use std::iter::repeat;
 use std::ops::{Add, Range};
 
-mod extra;
-
-use crate::extra::ZipSlices;
-
 fn slice_iter(c: &mut Criterion) {
     let xs: Vec<_> = repeat(1i32).take(20).collect();
 
@@ -120,72 +116,6 @@ fn zip_slices_ziptuple(c: &mut Criterion) {
     });
 }
 
-fn zipslices(c: &mut Criterion) {
-    let xs = vec![0; 1024];
-    let ys = vec![0; 768];
-    let xs = black_box(xs);
-    let ys = black_box(ys);
-
-    c.bench_function("zipslices", move |b| {
-        b.iter(|| {
-            for (&x, &y) in ZipSlices::new(&xs, &ys) {
-                black_box(x);
-                black_box(y);
-            }
-        })
-    });
-}
-
-fn zipslices_mut(c: &mut Criterion) {
-    let xs = vec![0; 1024];
-    let ys = vec![0; 768];
-    let xs = black_box(xs);
-    let mut ys = black_box(ys);
-
-    c.bench_function("zipslices mut", move |b| {
-        b.iter(|| {
-            for (&x, &mut y) in ZipSlices::from_slices(&xs[..], &mut ys[..]) {
-                black_box(x);
-                black_box(y);
-            }
-        })
-    });
-}
-
-fn zipdot_i32_zipslices(c: &mut Criterion) {
-    let xs = vec![2; 1024];
-    let ys = vec![2; 768];
-    let xs = black_box(xs);
-    let ys = black_box(ys);
-
-    c.bench_function("zipdot i32 zipslices", move |b| {
-        b.iter(|| {
-            let mut s = 0i32;
-            for (&x, &y) in ZipSlices::new(&xs, &ys) {
-                s += x * y;
-            }
-            s
-        })
-    });
-}
-
-fn zipdot_f32_zipslices(c: &mut Criterion) {
-    let xs = vec![2f32; 1024];
-    let ys = vec![2f32; 768];
-    let xs = black_box(xs);
-    let ys = black_box(ys);
-
-    c.bench_function("zipdot f32 zipslices", move |b| {
-        b.iter(|| {
-            let mut s = 0.;
-            for (&x, &y) in ZipSlices::new(&xs, &ys) {
-                s += x * y;
-            }
-            s
-        })
-    });
-}
-
 fn zip_checked_counted_loop(c: &mut Criterion) {
     let xs = vec![0; 1024];
     let ys = vec![0; 768];
@@ -801,10 +731,6 @@ criterion_group!(
     zipdot_f32_default_zip,
     zip_default_zip3,
     zip_slices_ziptuple,
-    zipslices,
-    zipslices_mut,
-    zipdot_i32_zipslices,
-    zipdot_f32_zipslices,
     zip_checked_counted_loop,
     zipdot_i32_checked_counted_loop,
     zipdot_f32_checked_counted_loop,
diff --git a/benches/extra/mod.rs b/benches/extra/mod.rs
deleted file mode 100644
index 52fe5cc..0000000
--- a/benches/extra/mod.rs
+++ /dev/null
@@ -1,2 +0,0 @@
-pub use self::zipslices::ZipSlices;
-mod zipslices;
diff --git a/benches/extra/zipslices.rs b/benches/extra/zipslices.rs
deleted file mode 100644
index 2ec7bc0..0000000
--- a/benches/extra/zipslices.rs
+++ /dev/null
@@ -1,194 +0,0 @@
-use std::cmp;
-
-// Note: There are different ways to implement ZipSlices.
-// This version performed the best in benchmarks.
-//
-// I also implemented a version with three pointers (tptr, tend, uptr),
-// that mimiced slice::Iter and only checked bounds by using tptr == tend,
-// but that was inferior to this solution.
-
-/// An iterator which iterates two slices simultaneously.
-///
-/// `ZipSlices` acts like a double-ended `.zip()` iterator.
-///
-/// It was intended to be more efficient than `.zip()`, and it was, then
-/// rustc changed how it optimizes so it can not promise improved performance
-/// at this time.
-///
-/// Note that elements past the end of the shortest of the two slices are ignored.
-///
-/// Iterator element type for `ZipSlices<T, U>` is `(T::Item, U::Item)`. For example,
-/// for a `ZipSlices<&'a [A], &'b mut [B]>`, the element type is `(&'a A, &'b mut B)`.
-#[derive(Clone)]
-pub struct ZipSlices<T, U> {
-    t: T,
-    u: U,
-    len: usize,
-    index: usize,
-}
-
-impl<'a, 'b, A, B> ZipSlices<&'a [A], &'b [B]> {
-    /// Create a new `ZipSlices` from slices `a` and `b`.
-    ///
-    /// Act like a double-ended `.zip()` iterator, but more efficiently.
-    ///
-    /// Note that elements past the end of the shortest of the two slices are ignored.
-    #[inline(always)]
-    pub fn new(a: &'a [A], b: &'b [B]) -> Self {
-        let minl = cmp::min(a.len(), b.len());
-        ZipSlices {
-            t: a,
-            u: b,
-            len: minl,
-            index: 0,
-        }
-    }
-}
-
-impl<T, U> ZipSlices<T, U>
-where
-    T: Slice,
-    U: Slice,
-{
-    /// Create a new `ZipSlices` from slices `a` and `b`.
-    ///
-    /// Act like a double-ended `.zip()` iterator, but more efficiently.
-    ///
-    /// Note that elements past the end of the shortest of the two slices are ignored.
-    #[inline(always)]
-    pub fn from_slices(a: T, b: U) -> Self {
-        let minl = cmp::min(a.len(), b.len());
-        Self {
-            t: a,
-            u: b,
-            len: minl,
-            index: 0,
-        }
-    }
-}
-
-impl<T, U> Iterator for ZipSlices<T, U>
-where
-    T: Slice,
-    U: Slice,
-{
-    type Item = (T::Item, U::Item);
-
-    #[inline(always)]
-    fn next(&mut self) -> Option<Self::Item> {
-        unsafe {
-            if self.index >= self.len {
-                None
-            } else {
-                let i = self.index;
-                self.index += 1;
-                Some((self.t.get_unchecked(i), self.u.get_unchecked(i)))
-            }
-        }
-    }
-
-    #[inline]
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let len = self.len - self.index;
-        (len, Some(len))
-    }
-}
-
-impl<T, U> DoubleEndedIterator for ZipSlices<T, U>
-where
-    T: Slice,
-    U: Slice,
-{
-    #[inline(always)]
-    fn next_back(&mut self) -> Option<Self::Item> {
-        unsafe {
-            if self.index >= self.len {
-                None
-            } else {
-                self.len -= 1;
-                let i = self.len;
-                Some((self.t.get_unchecked(i), self.u.get_unchecked(i)))
-            }
-        }
-    }
-}
-
-impl<T, U> ExactSizeIterator for ZipSlices<T, U>
-where
-    T: Slice,
-    U: Slice,
-{
-}
-
-unsafe impl<T, U> Slice for ZipSlices<T, U>
-where
-    T: Slice,
-    U: Slice,
-{
-    type Item = (T::Item, U::Item);
-
-    fn len(&self) -> usize {
-        self.len - self.index
-    }
-
-    unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
-        (self.t.get_unchecked(i), self.u.get_unchecked(i))
-    }
-}
-
-/// A helper trait to let `ZipSlices` accept both `&[T]` and `&mut [T]`.
-///
-/// # Safety
-///
-/// Unsafe trait because:
-///
-/// - Implementors must guarantee that `get_unchecked` is valid for all indices `0..len()`.
-pub unsafe trait Slice {
-    /// The type of a reference to the slice's elements
-    type Item;
-    #[doc(hidden)]
-    fn len(&self) -> usize;
-    #[doc(hidden)]
-    unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item;
-}
-
-unsafe impl<'a, T> Slice for &'a [T] {
-    type Item = &'a T;
-    #[inline(always)]
-    fn len(&self) -> usize {
-        (**self).len()
-    }
-    #[inline(always)]
-    unsafe fn get_unchecked(&mut self, i: usize) -> &'a T {
-        debug_assert!(i < self.len());
-        (**self).get_unchecked(i)
-    }
-}
-
-unsafe impl<'a, T> Slice for &'a mut [T] {
-    type Item = &'a mut T;
-    #[inline(always)]
-    fn len(&self) -> usize {
-        (**self).len()
-    }
-    #[inline(always)]
-    unsafe fn get_unchecked(&mut self, i: usize) -> &'a mut T {
-        debug_assert!(i < self.len());
-        // override the lifetime constraints of &mut &'a mut [T]
-        (*(*self as *mut [T])).get_unchecked_mut(i)
-    }
-}
-
-#[test]
-fn zipslices() {
-    let xs = [1, 2, 3, 4, 5, 6];
-    let ys = [1, 2, 3, 7];
-    ::itertools::assert_equal(ZipSlices::new(&xs, &ys), xs.iter().zip(&ys));
-
-    let xs = [1, 2, 3, 4, 5, 6];
-    let mut ys = [0; 6];
-    for (x, y) in ZipSlices::from_slices(&xs[..], &mut ys[..]) {
-        *y = *x;
-    }
-    ::itertools::assert_equal(&xs, &ys);
-}
diff --git a/src/process_results_impl.rs b/src/process_results_impl.rs
index 7b40cc0..ad6c60d 100644
--- a/src/process_results_impl.rs
+++ b/src/process_results_impl.rs
@@ -13,14 +13,10 @@ pub struct ProcessResults<'a, I, E: 'a> {
     iter: I,
 }
 
-impl<'a, I, T, E> Iterator for ProcessResults<'a, I, E>
-where
-    I: Iterator<Item = Result<T, E>>,
-{
-    type Item = T;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        match self.iter.next() {
+impl<'a, I, E> ProcessResults<'a, I, E> {
+    #[inline(always)]
+    fn next_body<T>(&mut self, item: Option<Result<T, E>>) -> Option<T> {
+        match item {
             Some(Ok(x)) => Some(x),
             Some(Err(e)) => {
                 *self.error = Err(e);
@@ -29,6 +25,18 @@ where
             None => None,
         }
     }
+}
+
+impl<'a, I, T, E> Iterator for ProcessResults<'a, I, E>
+where
+    I: Iterator<Item = Result<T, E>>,
+{
+    type Item = T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let item = self.iter.next();
+        self.next_body(item)
+    }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
         (0, self.iter.size_hint().1)
@@ -52,6 +60,33 @@ where
     }
 }
 
+impl<'a, I, T, E> DoubleEndedIterator for ProcessResults<'a, I, E>
+where
+    I: Iterator<Item = Result<T, E>>,
+    I: DoubleEndedIterator,
+{
+    fn next_back(&mut self) -> Option<Self::Item> {
+        let item = self.iter.next_back();
+        self.next_body(item)
+    }
+
+    fn rfold<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let error = self.error;
+        self.iter
+            .try_rfold(init, |acc, opt| match opt {
+                Ok(x) => Ok(f(acc, x)),
+                Err(e) => {
+                    *error = Err(e);
+                    Err(acc)
+                }
+            })
+            .unwrap_or_else(|e| e)
+    }
+}
+
 /// “Lift” a function of the values of an iterator so that it can process
 /// an iterator of `Result` values instead.
 ///


Test patch:
diff --git a/tests/specializations.rs b/tests/specializations.rs
index fd8801e..949b2a7 100644
--- a/tests/specializations.rs
+++ b/tests/specializations.rs
@@ -466,7 +466,7 @@ quickcheck! {
         helper(v.iter().copied());
         helper(v.iter().copied().filter(Result::is_ok));
 
-        fn helper(it: impl Iterator<Item = Result<u8, u8>> + Clone) {
+        fn helper(it: impl Iterator<Item = Result<u8, u8>> + DoubleEndedIterator + Clone) {
             macro_rules! check_results_specialized {
                 ($src:expr, |$it:pat| $closure:expr) => {
                     assert_eq!(
@@ -482,6 +482,7 @@ quickcheck! {
             check_results_specialized!(it, |i| i.count());
             check_results_specialized!(it, |i| i.last());
             check_results_specialized!(it, |i| i.collect::<Vec<_>>());
+            check_results_specialized!(it, |i| i.rev().collect::<Vec<_>>());
             check_results_specialized!(it, |i| {
                 let mut parameters_from_fold = vec![];
                 let fold_result = i.fold(vec![], |mut acc, v| {
@@ -491,6 +492,15 @@ quickcheck! {
                 });
                 (parameters_from_fold, fold_result)
             });
+            check_results_specialized!(it, |i| {
+                let mut parameters_from_rfold = vec![];
+                let rfold_result = i.rfold(vec![], |mut acc, v| {
+                    parameters_from_rfold.push((acc.clone(), v));
+                    acc.push(v);
+                    acc
+                });
+                (parameters_from_rfold, rfold_result)
+            });
             check_results_specialized!(it, |mut i| {
                 let mut parameters_from_all = vec![];
                 let first = i.next();
@@ -504,6 +514,9 @@ quickcheck! {
             for n in 0..size + 2 {
                 check_results_specialized!(it, |mut i| i.nth(n));
             }
+            for n in 0..size + 2 {
+                check_results_specialized!(it, |mut i| i.nth_back(n));
+            }
         }
     }
 }