[Experiment] do the opposite of what zip side-effect documentation currently guarantees

Author: the8472

library/alloc/src/collections/linked_list.rs

@@ -1925,7 +1925,8 @@ impl<T: Clone> Clone for LinkedList<T> {
         if self.len() > other.len() {
             self.split_off(other.len());
         }
-        for (elem, elem_other) in self.iter_mut().zip(&mut iter_other) {
+        let len = self.len();
+        for (elem, elem_other) in self.iter_mut().zip((&mut iter_other).take(len)) {
             elem.clone_from(elem_other);
         }
         if !iter_other.is_empty() {

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

@@ -13,10 +13,6 @@ use crate::iter::{InPlaceIterable, SourceIter, TrustedLen};
 pub struct Zip<A, B> {
     a: A,
     b: B,
-    // index, len and a_len are only used by the specialized version of zip
-    index: usize,
-    len: usize,
-    a_len: usize,
 }
 impl<A: Iterator, B: Iterator> Zip<A, B> {
     pub(in crate::iter) fn new(a: A, b: B) -> Zip<A, B> {
@@ -88,14 +84,21 @@ where
     }
 
     #[inline]
-    #[doc(hidden)]
-    unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
-    where
-        Self: TrustedRandomAccessNoCoerce,
-    {
-        // SAFETY: `ZipImpl::__iterator_get_unchecked` has same safety
-        // requirements as `Iterator::__iterator_get_unchecked`.
-        unsafe { ZipImpl::get_unchecked(self, idx) }
+    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+        let ((lower_a, _), (lower_b, _)) = (self.a.size_hint(), self.b.size_hint());
+        let lower = n.min(lower_a).min(lower_b);
+        let batched = match (self.b.advance_by(lower), self.a.advance_by(lower)) {
+            (Ok(()), Ok(())) => lower,
+            _ => panic!("size_hint contract violation"),
+        };
+
+        for i in batched..n {
+            if let (_, None) | (None, _) = (self.b.next(), self.a.next()) {
+                return Err(i);
+            }
+        }
+
+        Ok(())
     }
 }
 
@@ -123,30 +126,20 @@ trait ZipImpl<A, B> {
     where
         A: DoubleEndedIterator + ExactSizeIterator,
         B: DoubleEndedIterator + ExactSizeIterator;
-    // This has the same safety requirements as `Iterator::__iterator_get_unchecked`
-    unsafe fn get_unchecked(&mut self, idx: usize) -> <Self as Iterator>::Item
-    where
-        Self: Iterator + TrustedRandomAccessNoCoerce;
 }
 
 // Work around limitations of specialization, requiring `default` impls to be repeated
 // in intermediary impls.
 macro_rules! zip_impl_general_defaults {
     () => {
         default fn new(a: A, b: B) -> Self {
-            Zip {
-                a,
-                b,
-                index: 0, // unused
-                len: 0,   // unused
-                a_len: 0, // unused
-            }
+            Zip { a, b }
         }
 
         #[inline]
         default fn next(&mut self) -> Option<(A::Item, B::Item)> {
-            let x = self.a.next()?;
             let y = self.b.next()?;
+            let x = self.a.next()?;
             Some((x, y))
         }
 
@@ -179,8 +172,8 @@ macro_rules! zip_impl_general_defaults {
                     }
                 }
             }
-            match (self.a.next_back(), self.b.next_back()) {
-                (Some(x), Some(y)) => Some((x, y)),
+            match (self.b.next_back(), self.a.next_back()) {
+                (Some(y), Some(x)) => Some((x, y)),
                 (None, None) => None,
                 _ => unreachable!(),
             }
@@ -215,157 +208,6 @@ where
 
         (lower, upper)
     }
-
-    default unsafe fn get_unchecked(&mut self, _idx: usize) -> <Self as Iterator>::Item
-    where
-        Self: TrustedRandomAccessNoCoerce,
-    {
-        unreachable!("Always specialized");
-    }
-}
-
-#[doc(hidden)]
-impl<A, B> ZipImpl<A, B> for Zip<A, B>
-where
-    A: TrustedRandomAccessNoCoerce + Iterator,
-    B: TrustedRandomAccessNoCoerce + Iterator,
-{
-    zip_impl_general_defaults! {}
-
-    #[inline]
-    default fn size_hint(&self) -> (usize, Option<usize>) {
-        let size = cmp::min(self.a.size(), self.b.size());
-        (size, Some(size))
-    }
-
-    #[inline]
-    unsafe fn get_unchecked(&mut self, idx: usize) -> <Self as Iterator>::Item {
-        let idx = self.index + idx;
-        // SAFETY: the caller must uphold the contract for
-        // `Iterator::__iterator_get_unchecked`.
-        unsafe { (self.a.__iterator_get_unchecked(idx), self.b.__iterator_get_unchecked(idx)) }
-    }
-}
-
-#[doc(hidden)]
-impl<A, B> ZipImpl<A, B> for Zip<A, B>
-where
-    A: TrustedRandomAccess + Iterator,
-    B: TrustedRandomAccess + Iterator,
-{
-    fn new(a: A, b: B) -> Self {
-        let a_len = a.size();
-        let len = cmp::min(a_len, b.size());
-        Zip { a, b, index: 0, len, a_len }
-    }
-
-    #[inline]
-    fn next(&mut self) -> Option<(A::Item, B::Item)> {
-        if self.index < self.len {
-            let i = self.index;
-            // since get_unchecked executes code which can panic we increment the counters beforehand
-            // so that the same index won't be accessed twice, as required by TrustedRandomAccess
-            self.index += 1;
-            // SAFETY: `i` is smaller than `self.len`, thus smaller than `self.a.len()` and `self.b.len()`
-            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;
-            // as above, increment before executing code that may panic
-            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
-        }
-    }
-
-    #[inline]
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let len = self.len - self.index;
-        (len, Some(len))
-    }
-
-    #[inline]
-    fn nth(&mut self, n: usize) -> Option<Self::Item> {
-        let delta = cmp::min(n, self.len - self.index);
-        let end = self.index + delta;
-        while self.index < end {
-            let i = self.index;
-            // since get_unchecked executes code which can panic we increment the counters beforehand
-            // so that the same index won't be accessed twice, as required by TrustedRandomAccess
-            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)
-    }
-
-    #[inline]
-    fn next_back(&mut self) -> Option<(A::Item, B::Item)>
-    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 {
-                        // since next_back() may panic we increment the counters beforehand
-                        // to keep Zip's state in sync with the underlying iterator source
-                        self.a_len -= 1;
-                        self.a.next_back();
-                    }
-                    debug_assert_eq!(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 {
-            // since get_unchecked executes code which can panic we increment the counters beforehand
-            // so that the same index won't be accessed twice, as required by TrustedRandomAccess
-            self.len -= 1;
-            self.a_len -= 1;
-            let i = self.len;
-            // SAFETY: `i` is smaller than the previous value of `self.len`,
-            // which is also smaller than or equal to `self.a.len()` and `self.b.len()`
-            unsafe {
-                Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
-            }
-        } else {
-            None
-        }
-    }
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -376,25 +218,6 @@ where
 {
 }
 
-#[doc(hidden)]
-#[unstable(feature = "trusted_random_access", issue = "none")]
-unsafe impl<A, B> TrustedRandomAccess for Zip<A, B>
-where
-    A: TrustedRandomAccess,
-    B: TrustedRandomAccess,
-{
-}
-
-#[doc(hidden)]
-#[unstable(feature = "trusted_random_access", issue = "none")]
-unsafe impl<A, B> TrustedRandomAccessNoCoerce for Zip<A, B>
-where
-    A: TrustedRandomAccessNoCoerce,
-    B: TrustedRandomAccessNoCoerce,
-{
-    const MAY_HAVE_SIDE_EFFECT: bool = A::MAY_HAVE_SIDE_EFFECT || B::MAY_HAVE_SIDE_EFFECT;
-}
-
 #[stable(feature = "fused", since = "1.26.0")]
 impl<A, B> FusedIterator for Zip<A, B>
 where
@@ -448,16 +271,6 @@ impl<A: Debug, B: Debug> ZipFmt<A, B> for Zip<A, B> {
     }
 }
 
-impl<A: Debug + TrustedRandomAccessNoCoerce, B: Debug + TrustedRandomAccessNoCoerce> ZipFmt<A, B>
-    for Zip<A, B>
-{
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // It's *not safe* to call fmt on the contained iterators, since once
-        // we start iterating they're in strange, potentially unsafe, states.
-        f.debug_struct("Zip").finish()
-    }
-}
-
 /// An iterator whose items are random-accessible efficiently
 ///
 /// # Safety

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

@@ -31,7 +31,7 @@ fn test_cloned_side_effects() {
             .zip(&[1]);
         for _ in iter {}
     }
-    assert_eq!(count, 2);
+    assert_eq!(count, 1);
 }
 
 #[test]

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

@@ -108,7 +108,7 @@ fn test_zip_next_back_side_effects_exhausted() {
     iter.next();
     iter.next();
     assert_eq!(iter.next_back(), None);
-    assert_eq!(a, vec![1, 2, 3, 4, 6, 5]);
+    assert_eq!(a, vec![1, 2, 3, 6, 5, 4]);
     assert_eq!(b, vec![200, 300, 400]);
 }
 
@@ -119,7 +119,7 @@ fn test_zip_cloned_sideffectful() {
 
     for _ in xs.iter().cloned().zip(ys.iter().cloned()) {}
 
-    assert_eq!(&xs, &[1, 1, 1, 0][..]);
+    assert_eq!(&xs, &[1, 1, 0, 0][..]);
     assert_eq!(&ys, &[1, 1][..]);
 
     let xs = [CountClone::new(), CountClone::new()];
@@ -128,7 +128,7 @@ fn test_zip_cloned_sideffectful() {
     for _ in xs.iter().cloned().zip(ys.iter().cloned()) {}
 
     assert_eq!(&xs, &[1, 1][..]);
-    assert_eq!(&ys, &[1, 1, 0, 0][..]);
+    assert_eq!(&ys, &[1, 1, 1, 0][..]);
 }
 
 #[test]
@@ -138,7 +138,7 @@ fn test_zip_map_sideffectful() {
 
     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!(&xs, &[1, 1, 1, 1, 0, 0]);
     assert_eq!(&ys, &[1, 1, 1, 1]);
 
     let mut xs = [0; 4];
@@ -147,7 +147,7 @@ fn test_zip_map_sideffectful() {
     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]);
+    assert_eq!(&ys, &[1, 1, 1, 1, 1, 0]);
 }
 
 #[test]
@@ -176,15 +176,15 @@ fn test_zip_map_rev_sideffectful() {
 
 #[test]
 fn test_zip_nested_sideffectful() {
-    let mut xs = [0; 6];
-    let ys = [0; 4];
+    let xs = [0; 4];
+    let mut ys = [0; 6];
 
     {
         // test that it has the side effect nested inside enumerate
-        let it = xs.iter_mut().map(|x| *x = 1).enumerate().zip(&ys);
+        let it = xs.iter().enumerate().zip(ys.iter_mut().map(|x| *x = 1));
         it.count();
     }
-    assert_eq!(&xs, &[1, 1, 1, 1, 1, 0]);
+    assert_eq!(&ys, &[1, 1, 1, 1, 1, 0]);
 }
 
 #[test]
@@ -208,7 +208,7 @@ fn test_zip_nth_back_side_effects_exhausted() {
     iter.next();
     iter.next();
     assert_eq!(iter.nth_back(0), None);
-    assert_eq!(a, vec![1, 2, 3, 4, 6, 5]);
+    assert_eq!(a, vec![1, 2, 3, 6, 5, 4]);
     assert_eq!(b, vec![200, 300, 400]);
 }
 
@@ -227,8 +227,7 @@ fn test_zip_trusted_random_access_composition() {
     assert_eq!(z1.next().unwrap(), (0, 0));
 
     let mut z2 = z1.zip(c);
-    fn assert_trusted_random_access<T: TrustedRandomAccess>(_a: &T) {}
-    assert_trusted_random_access(&z2);
+
     assert_eq!(z2.next().unwrap(), ((1, 1), 1));
 }