break as many Zip side-effects as we can

Author: the8472

library/core/src/iter/adapters/zip.rs

@@ -231,16 +231,6 @@ where
             unsafe {
                 Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
             }
-        } else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a_len {
-            let i = self.index;
-            self.index += 1;
-            self.len += 1;
-            // match the base implementation's potential side effects
-            // SAFETY: we just checked that `i` < `self.a.len()`
-            unsafe {
-                self.a.__iterator_get_unchecked(i);
-            }
-            None
         } else {
             None
         }
@@ -257,22 +247,7 @@ where
         let delta = cmp::min(n, self.len - self.index);
         let end = self.index + delta;
         while self.index < end {
-            let i = self.index;
             self.index += 1;
-            if A::MAY_HAVE_SIDE_EFFECT {
-                // SAFETY: the usage of `cmp::min` to calculate `delta`
-                // ensures that `end` is smaller than or equal to `self.len`,
-                // so `i` is also smaller than `self.len`.
-                unsafe {
-                    self.a.__iterator_get_unchecked(i);
-                }
-            }
-            if B::MAY_HAVE_SIDE_EFFECT {
-                // SAFETY: same as above.
-                unsafe {
-                    self.b.__iterator_get_unchecked(i);
-                }
-            }
         }
 
         self.super_nth(n - delta)
@@ -284,28 +259,6 @@ where
         A: DoubleEndedIterator + ExactSizeIterator,
         B: DoubleEndedIterator + ExactSizeIterator,
     {
-        if A::MAY_HAVE_SIDE_EFFECT || B::MAY_HAVE_SIDE_EFFECT {
-            let sz_a = self.a.size();
-            let sz_b = self.b.size();
-            // Adjust a, b to equal length, make sure that only the first call
-            // of `next_back` does this, otherwise we will break the restriction
-            // on calls to `self.next_back()` after calling `get_unchecked()`.
-            if sz_a != sz_b {
-                let sz_a = self.a.size();
-                if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len {
-                    for _ in 0..sz_a - self.len {
-                        self.a.next_back();
-                    }
-                    self.a_len = self.len;
-                }
-                let sz_b = self.b.size();
-                if B::MAY_HAVE_SIDE_EFFECT && sz_b > self.len {
-                    for _ in 0..sz_b - self.len {
-                        self.b.next_back();
-                    }
-                }
-            }
-        }
         if self.index < self.len {
             self.len -= 1;
             self.a_len -= 1;

library/core/src/iter/traits/double_ended.rs

@@ -1,3 +1,4 @@
+use super::super::{TrustedLen, TrustedRandomAccess};
 use crate::ops::{ControlFlow, Try};
 
 /// An iterator able to yield elements from both ends.
@@ -278,16 +279,12 @@ pub trait DoubleEndedIterator: Iterator {
     /// ```
     #[inline]
     #[stable(feature = "iter_rfold", since = "1.27.0")]
-    fn rfold<B, F>(mut self, init: B, mut f: F) -> B
+    fn rfold<B, F>(self, init: B, f: F) -> B
     where
         Self: Sized,
         F: FnMut(B, Self::Item) -> B,
     {
-        let mut accum = init;
-        while let Some(x) = self.next_back() {
-            accum = f(accum, x);
-        }
-        accum
+        self.rfold_spec(init, f)
     }
 
     /// Searches for an element of an iterator from the back that satisfies a predicate.
@@ -361,3 +358,71 @@ impl<'a, I: DoubleEndedIterator + ?Sized> DoubleEndedIterator for &'a mut I {
         (**self).nth_back(n)
     }
 }
+
+trait SpecIteratorDefaultImpls: DoubleEndedIterator {
+    fn rfold_spec<B, F>(self, init: B, f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B;
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: DoubleEndedIterator,
+{
+    #[inline]
+    default fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        while let Some(x) = self.next_back() {
+            accum = f(accum, x);
+        }
+        accum
+    }
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: DoubleEndedIterator + Sized + TrustedLen,
+{
+    #[inline]
+    default fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        while self.size_hint().1.is_none() {
+            accum = f(accum, self.next_back().unwrap())
+        }
+        for _ in 0..self.size_hint().1.unwrap() {
+            accum = f(accum, self.next_back().unwrap());
+        }
+        accum
+    }
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: DoubleEndedIterator + Sized + TrustedLen + TrustedRandomAccess,
+{
+    #[inline]
+    fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        // SAFETY: every element is only read once, as required by the
+        // TrustedRandomAccess contract
+        unsafe {
+            for i in (0..self.size()).rev() {
+                accum = f(accum, self.__iterator_get_unchecked(i))
+            }
+        }
+        accum
+    }
+}

library/core/src/iter/traits/iterator.rs

@@ -5,13 +5,13 @@
 use crate::cmp::{self, Ordering};
 use crate::ops::{ControlFlow, Try};
 
-use super::super::TrustedRandomAccess;
 use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse};
 use super::super::{FlatMap, Flatten};
 use super::super::{FromIterator, Intersperse, IntersperseWith, Product, Sum, Zip};
 use super::super::{
     Inspect, Map, MapWhile, Peekable, Rev, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile,
 };
+use super::super::{TrustedLen, TrustedRandomAccess};
 
 fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
 
@@ -2125,16 +2125,12 @@ pub trait Iterator {
     #[doc(alias = "inject")]
     #[inline]
     #[stable(feature = "rust1", since = "1.0.0")]
-    fn fold<B, F>(mut self, init: B, mut f: F) -> B
+    fn fold<B, F>(self, init: B, f: F) -> B
     where
         Self: Sized,
         F: FnMut(B, Self::Item) -> B,
     {
-        let mut accum = init;
-        while let Some(x) = self.next() {
-            accum = f(accum, x);
-        }
-        accum
+        self.fold_spec(init, f)
     }
 
     /// Reduces the elements to a single one, by repeatedly applying a reducing
@@ -3420,3 +3416,71 @@ impl<I: Iterator + ?Sized> Iterator for &mut I {
         (**self).nth(n)
     }
 }
+
+trait SpecIteratorDefaultImpls: Iterator {
+    fn fold_spec<B, F>(self, init: B, f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B;
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: Iterator,
+{
+    #[inline]
+    default fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        while let Some(x) = self.next() {
+            accum = f(accum, x);
+        }
+        accum
+    }
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: Iterator + Sized + TrustedLen,
+{
+    #[inline]
+    default fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        while self.size_hint().1.is_none() {
+            accum = f(accum, self.next().unwrap());
+        }
+        for _ in 0..self.size_hint().1.unwrap() {
+            accum = f(accum, self.next().unwrap())
+        }
+        accum
+    }
+}
+
+impl<T> SpecIteratorDefaultImpls for T
+where
+    T: Iterator + Sized + TrustedLen + TrustedRandomAccess,
+{
+    #[inline]
+    fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
+    where
+        Self: Sized,
+        F: FnMut(B, Self::Item) -> B,
+    {
+        let mut accum = init;
+        // SAFETY: every element is only read once, as required by the
+        // TrustedRandomAccess contract
+        unsafe {
+            for i in 0..self.size() {
+                accum = f(accum, self.__iterator_get_unchecked(i))
+            }
+        }
+        accum
+    }
+}

library/core/tests/iter/adapters/cloned.rs

@@ -17,23 +17,6 @@ fn test_cloned() {
     assert_eq!(it.next_back(), None);
 }
 
-#[test]
-fn test_cloned_side_effects() {
-    let mut count = 0;
-    {
-        let iter = [1, 2, 3]
-            .iter()
-            .map(|x| {
-                count += 1;
-                x
-            })
-            .cloned()
-            .zip(&[1]);
-        for _ in iter {}
-    }
-    assert_eq!(count, 2);
-}
-
 #[test]
 fn test_cloned_try_folds() {
     let a = [1, 2, 3, 4, 5, 6, 7, 8, 9];

library/core/tests/iter/adapters/mod.rs

@@ -20,8 +20,6 @@ mod take;
 mod take_while;
 mod zip;
 
-use core::cell::Cell;
-
 /// An iterator that panics whenever `next` or next_back` is called
 /// after `None` has already been returned. This does not violate
 /// `Iterator`'s contract. Used to test that iterator adaptors don't
@@ -159,27 +157,3 @@ impl<'a, T> Iterator for CycleIter<'a, T> {
         elt
     }
 }
-
-#[derive(Debug)]
-struct CountClone(Cell<i32>);
-
-impl CountClone {
-    pub fn new() -> Self {
-        Self(Cell::new(0))
-    }
-}
-
-impl PartialEq<i32> for CountClone {
-    fn eq(&self, rhs: &i32) -> bool {
-        self.0.get() == *rhs
-    }
-}
-
-impl Clone for CountClone {
-    fn clone(&self) -> Self {
-        let ret = CountClone(self.0.clone());
-        let n = self.0.get();
-        self.0.set(n + 1);
-        ret
-    }
-}

library/core/tests/iter/adapters/zip.rs

@@ -1,4 +1,3 @@
-use super::*;
 use core::iter::*;
 
 #[test]
@@ -18,200 +17,6 @@ fn test_zip_nth() {
     assert_eq!(it.nth(3), None);
 }
 
-#[test]
-fn test_zip_nth_side_effects() {
-    let mut a = Vec::new();
-    let mut b = Vec::new();
-    let value = [1, 2, 3, 4, 5, 6]
-        .iter()
-        .cloned()
-        .map(|n| {
-            a.push(n);
-            n * 10
-        })
-        .zip([2, 3, 4, 5, 6, 7, 8].iter().cloned().map(|n| {
-            b.push(n * 100);
-            n * 1000
-        }))
-        .skip(1)
-        .nth(3);
-    assert_eq!(value, Some((50, 6000)));
-    assert_eq!(a, vec![1, 2, 3, 4, 5]);
-    assert_eq!(b, vec![200, 300, 400, 500, 600]);
-}
-
-#[test]
-fn test_zip_next_back_side_effects() {
-    let mut a = Vec::new();
-    let mut b = Vec::new();
-    let mut iter = [1, 2, 3, 4, 5, 6]
-        .iter()
-        .cloned()
-        .map(|n| {
-            a.push(n);
-            n * 10
-        })
-        .zip([2, 3, 4, 5, 6, 7, 8].iter().cloned().map(|n| {
-            b.push(n * 100);
-            n * 1000
-        }));
-
-    // The second iterator is one item longer, so `next_back` is called on it
-    // one more time.
-    assert_eq!(iter.next_back(), Some((60, 7000)));
-    assert_eq!(iter.next_back(), Some((50, 6000)));
-    assert_eq!(iter.next_back(), Some((40, 5000)));
-    assert_eq!(iter.next_back(), Some((30, 4000)));
-    assert_eq!(a, vec![6, 5, 4, 3]);
-    assert_eq!(b, vec![800, 700, 600, 500, 400]);
-}
-
-#[test]
-fn test_zip_nth_back_side_effects() {
-    let mut a = Vec::new();
-    let mut b = Vec::new();
-    let value = [1, 2, 3, 4, 5, 6]
-        .iter()
-        .cloned()
-        .map(|n| {
-            a.push(n);
-            n * 10
-        })
-        .zip([2, 3, 4, 5, 6, 7, 8].iter().cloned().map(|n| {
-            b.push(n * 100);
-            n * 1000
-        }))
-        .nth_back(3);
-    assert_eq!(value, Some((30, 4000)));
-    assert_eq!(a, vec![6, 5, 4, 3]);
-    assert_eq!(b, vec![800, 700, 600, 500, 400]);
-}
-
-#[test]
-fn test_zip_next_back_side_effects_exhausted() {
-    let mut a = Vec::new();
-    let mut b = Vec::new();
-    let mut iter = [1, 2, 3, 4, 5, 6]
-        .iter()
-        .cloned()
-        .map(|n| {
-            a.push(n);
-            n * 10
-        })
-        .zip([2, 3, 4].iter().cloned().map(|n| {
-            b.push(n * 100);
-            n * 1000
-        }));
-
-    iter.next();
-    iter.next();
-    iter.next();
-    iter.next();
-    assert_eq!(iter.next_back(), None);
-    assert_eq!(a, vec![1, 2, 3, 4, 6, 5]);
-    assert_eq!(b, vec![200, 300, 400]);
-}
-
-#[test]
-fn test_zip_cloned_sideffectful() {
-    let xs = [CountClone::new(), CountClone::new(), CountClone::new(), CountClone::new()];
-    let ys = [CountClone::new(), CountClone::new()];
-
-    for _ in xs.iter().cloned().zip(ys.iter().cloned()) {}
-
-    assert_eq!(&xs, &[1, 1, 1, 0][..]);
-    assert_eq!(&ys, &[1, 1][..]);
-
-    let xs = [CountClone::new(), CountClone::new()];
-    let ys = [CountClone::new(), CountClone::new(), CountClone::new(), CountClone::new()];
-
-    for _ in xs.iter().cloned().zip(ys.iter().cloned()) {}
-
-    assert_eq!(&xs, &[1, 1][..]);
-    assert_eq!(&ys, &[1, 1, 0, 0][..]);
-}
-
-#[test]
-fn test_zip_map_sideffectful() {
-    let mut xs = [0; 6];
-    let mut ys = [0; 4];
-
-    for _ in xs.iter_mut().map(|x| *x += 1).zip(ys.iter_mut().map(|y| *y += 1)) {}
-
-    assert_eq!(&xs, &[1, 1, 1, 1, 1, 0]);
-    assert_eq!(&ys, &[1, 1, 1, 1]);
-
-    let mut xs = [0; 4];
-    let mut ys = [0; 6];
-
-    for _ in xs.iter_mut().map(|x| *x += 1).zip(ys.iter_mut().map(|y| *y += 1)) {}
-
-    assert_eq!(&xs, &[1, 1, 1, 1]);
-    assert_eq!(&ys, &[1, 1, 1, 1, 0, 0]);
-}
-
-#[test]
-fn test_zip_map_rev_sideffectful() {
-    let mut xs = [0; 6];
-    let mut ys = [0; 4];
-
-    {
-        let mut it = xs.iter_mut().map(|x| *x += 1).zip(ys.iter_mut().map(|y| *y += 1));
-        it.next_back();
-    }
-    assert_eq!(&xs, &[0, 0, 0, 1, 1, 1]);
-    assert_eq!(&ys, &[0, 0, 0, 1]);
-
-    let mut xs = [0; 6];
-    let mut ys = [0; 4];
-
-    {
-        let mut it = xs.iter_mut().map(|x| *x += 1).zip(ys.iter_mut().map(|y| *y += 1));
-        (&mut it).take(5).count();
-        it.next_back();
-    }
-    assert_eq!(&xs, &[1, 1, 1, 1, 1, 1]);
-    assert_eq!(&ys, &[1, 1, 1, 1]);
-}
-
-#[test]
-fn test_zip_nested_sideffectful() {
-    let mut xs = [0; 6];
-    let ys = [0; 4];
-
-    {
-        // test that it has the side effect nested inside enumerate
-        let it = xs.iter_mut().map(|x| *x = 1).enumerate().zip(&ys);
-        it.count();
-    }
-    assert_eq!(&xs, &[1, 1, 1, 1, 1, 0]);
-}
-
-#[test]
-fn test_zip_nth_back_side_effects_exhausted() {
-    let mut a = Vec::new();
-    let mut b = Vec::new();
-    let mut iter = [1, 2, 3, 4, 5, 6]
-        .iter()
-        .cloned()
-        .map(|n| {
-            a.push(n);
-            n * 10
-        })
-        .zip([2, 3, 4].iter().cloned().map(|n| {
-            b.push(n * 100);
-            n * 1000
-        }));
-
-    iter.next();
-    iter.next();
-    iter.next();
-    iter.next();
-    assert_eq!(iter.nth_back(0), None);
-    assert_eq!(a, vec![1, 2, 3, 4, 6, 5]);
-    assert_eq!(b, vec![200, 300, 400]);
-}
-
 #[test]
 fn test_zip_trusted_random_access_composition() {
     let a = [0, 1, 2, 3, 4];

src/test/ui/issues/issue-3044.stderr

@@ -11,7 +11,7 @@ LL | |     });
 note: associated function defined here
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
    |
-LL |     fn fold<B, F>(mut self, init: B, mut f: F) -> B
+LL |     fn fold<B, F>(self, init: B, f: F) -> B
    |        ^^^^
 
 error: aborting due to previous error