Expand .zip() specialization to .map() and .cloned()

Implement .zip() specialization for Map and Cloned.

The crucial thing for transparent specialization is that we want to
preserve the potential side effects.

The simplest example is that in this code snippet:

`(0..6).map(f).zip((0..4).map(g)).count()`

`f` will be called five times, and `g` four times. The last time for `f`
is when the other iterator is at its end, so this element is unused.
This side effect can be preserved without disturbing code generation for
simple uses of `.map()`.

The `Zip::next_back()` case is even more complicated, unfortunately.

diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs
index 035418570a019076033f171f1fa98feb3d218ff3..9eeb2608071c2fd4bca9f43479e64b36f861f756 100644 (file)
--- a/src/libcore/iter/mod.rs
+++ b/src/libcore/iter/mod.rs
@@ -420,6 +420,18 @@ impl<'a, I, T: 'a> FusedIterator for Cloned<I>
     where I: FusedIterator<Item=&'a T>, T: Clone
 {}
 
+#[doc(hidden)]
+unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Cloned<I>
+    where I: TrustedRandomAccess<Item=&'a T>, T: Clone
+{
+    unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
+        self.it.get_unchecked(i).clone()
+    }
+
+    #[inline]
+    fn may_have_side_effect() -> bool { true }
+}
+
 /// An iterator that repeats endlessly.
 ///
 /// This `struct` is created by the [`cycle()`] method on [`Iterator`]. See its
@@ -773,6 +785,13 @@ fn next(&mut self) -> Option<(A::Item, B::Item)> {
             unsafe {
                 Some((self.a.get_unchecked(i), self.b.get_unchecked(i)))
             }
+        } else if A::may_have_side_effect() && self.index < self.a.len() {
+            // match the base implementation's potential side effects
+            unsafe {
+                self.a.get_unchecked(self.index);
+            }
+            self.index += 1;
+            None
         } else {
             None
         }
@@ -789,6 +808,23 @@ fn next_back(&mut self) -> Option<(A::Item, B::Item)>
         where A: DoubleEndedIterator + ExactSizeIterator,
               B: DoubleEndedIterator + ExactSizeIterator
     {
+        // Adjust a, b to equal length
+        if A::may_have_side_effect() {
+            let sz = self.a.len();
+            if sz > self.len {
+                for _ in 0..sz - cmp::max(self.len, self.index) {
+                    self.a.next_back();
+                }
+            }
+        }
+        if B::may_have_side_effect() {
+            let sz = self.b.len();
+            if sz > self.len {
+                for _ in 0..sz - self.len {
+                    self.b.next_back();
+                }
+            }
+        }
         if self.index < self.len {
             self.len -= 1;
             let i = self.len;
@@ -814,6 +850,9 @@ unsafe fn get_unchecked(&mut self, i: usize) -> (A::Item, B::Item) {
         (self.a.get_unchecked(i), self.b.get_unchecked(i))
     }
 
+    fn may_have_side_effect() -> bool {
+        A::may_have_side_effect() || B::may_have_side_effect()
+    }
 }
 
 #[unstable(feature = "fused", issue = "35602")]
@@ -920,6 +959,18 @@ impl<B, I: ExactSizeIterator, F> ExactSizeIterator for Map<I, F>
 impl<B, I: FusedIterator, F> FusedIterator for Map<I, F>
     where F: FnMut(I::Item) -> B {}
 
+#[doc(hidden)]
+unsafe impl<B, I, F> TrustedRandomAccess for Map<I, F>
+    where I: TrustedRandomAccess,
+          F: FnMut(I::Item) -> B,
+{
+    unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
+        (self.f)(self.iter.get_unchecked(i))
+    }
+    #[inline]
+    fn may_have_side_effect() -> bool { true }
+}
+
 /// An iterator that filters the elements of `iter` with `predicate`.
 ///
 /// This `struct` is created by the [`filter()`] method on [`Iterator`]. See its
@@ -1135,6 +1186,10 @@ unsafe impl<I> TrustedRandomAccess for Enumerate<I>
     unsafe fn get_unchecked(&mut self, i: usize) -> (usize, I::Item) {
         (self.count + i, self.iter.get_unchecked(i))
     }
+
+    fn may_have_side_effect() -> bool {
+        I::may_have_side_effect()
+    }
 }
 
 #[unstable(feature = "fused", issue = "35602")]
@@ -1764,6 +1819,10 @@ unsafe impl<I> TrustedRandomAccess for Fuse<I>
     unsafe fn get_unchecked(&mut self, i: usize) -> I::Item {
         self.iter.get_unchecked(i)
     }
+
+    fn may_have_side_effect() -> bool {
+        I::may_have_side_effect()
+    }
 }
 
 #[unstable(feature = "fused", issue = "35602")]

diff --git a/src/libcore/iter_private.rs b/src/libcore/iter_private.rs
index 83eeef31ab054cd074c3fa515a78f6ead1401594..bc1aaa09f3dbd35d3f566c667dc896564b591ce5 100644 (file)
--- a/src/libcore/iter_private.rs
+++ b/src/libcore/iter_private.rs
@@ -14,6 +14,7 @@
 /// # Safety
 ///
 /// The iterator's .len() and size_hint() must be exact.
+/// `.len()` must be cheap to call.
 ///
 /// .get_unchecked() must return distinct mutable references for distinct
 /// indices (if applicable), and must return a valid reference if index is in
@@ -21,5 +22,7 @@
 #[doc(hidden)]
 pub unsafe trait TrustedRandomAccess : ExactSizeIterator {
     unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item;
+    /// Return `true` if getting an iterator element may have
+    /// side effects. Remember to take inner iterators into account.
+    fn may_have_side_effect() -> bool;
 }
-

diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs
index d1df56905df2401cdb3d6d48923dc404f1fb59d4..31be404ba905a1fcb580d0e219aa4528c7b96972 100644 (file)
--- a/src/libcore/slice.rs
+++ b/src/libcore/slice.rs
@@ -1968,6 +1968,7 @@ unsafe impl<'a, T> TrustedRandomAccess for Iter<'a, T> {
     unsafe fn get_unchecked(&mut self, i: usize) -> &'a T {
         &*self.ptr.offset(i as isize)
     }
+    fn may_have_side_effect() -> bool { false }
 }
 
 #[doc(hidden)]
@@ -1975,4 +1976,5 @@ unsafe impl<'a, T> TrustedRandomAccess for IterMut<'a, T> {
     unsafe fn get_unchecked(&mut self, i: usize) -> &'a mut T {
         &mut *self.ptr.offset(i as isize)
     }
+    fn may_have_side_effect() -> bool { false }
 }

diff --git a/src/test/codegen/zip.rs b/src/test/codegen/zip.rs
index 6c956364bf80f060d92adc98378f19a7214cdf63..d0051c5165fe139253700b111cb7d8e64c506e8d 100644 (file)
--- a/src/test/codegen/zip.rs
+++ b/src/test/codegen/zip.rs
@@ -20,3 +20,12 @@ pub fn zip_copy(xs: &[u8], ys: &mut [u8]) {
         *y = *x;
     }
 }
+
+// CHECK-LABEL: @zip_copy_mapped
+#[no_mangle]
+pub fn zip_copy_mapped(xs: &[u8], ys: &mut [u8]) {
+// CHECK: memcpy
+    for (x, y) in xs.iter().map(|&x| x).zip(ys) {
+        *y = x;
+    }
+}

diff --git a/src/test/run-pass/iter-zip.rs b/src/test/run-pass/iter-zip.rs
new file mode 100644 (file)
index 0000000..b0503bc
--- /dev/null
+++ b/src/test/run-pass/iter-zip.rs
@@ -0,0 +1,112 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Test that .zip() specialization preserves side effects
+// in sideeffectful iterator adaptors.
+
+use std::cell::Cell;
+
+#[derive(Debug)]
+struct CountClone(Cell<i32>);
+
+fn count_clone() -> CountClone { CountClone(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
+    }
+}
+
+fn test_zip_cloned_sideffectful() {
+    let xs = [count_clone(), count_clone(), count_clone(), count_clone()];
+    let ys = [count_clone(), count_clone()];
+
+    for _ in xs.iter().cloned().zip(ys.iter().cloned()) { }
+
+    assert_eq!(&xs, &[1, 1, 1, 0][..]);
+    assert_eq!(&ys, &[1, 1][..]);
+
+    let xs = [count_clone(), count_clone()];
+    let ys = [count_clone(), count_clone(), count_clone(), count_clone()];
+
+    for _ in xs.iter().cloned().zip(ys.iter().cloned()) { }
+
+    assert_eq!(&xs, &[1, 1][..]);
+    assert_eq!(&ys, &[1, 1, 0, 0][..]);
+}
+
+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]);
+}
+
+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]);
+}
+
+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]);
+}
+
+fn main() {
+    test_zip_cloned_sideffectful();
+    test_zip_map_sideffectful();
+    test_zip_map_rev_sideffectful();
+    test_zip_nested_sideffectful();
+}