Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -773,6 +785,13 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
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
}
Expand All @@ -789,6 +808,23 @@ impl<A, B> ZipImpl<A, B> for Zip<A, B>
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;
Expand All @@ -814,6 +850,9 @@ unsafe impl<A, B> TrustedRandomAccess for Zip<A, B>
(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")]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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")]
Expand Down
5 changes: 4 additions & 1 deletion src/libcore/iter_private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@
/// # 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
/// 0..self.len().
#[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;
}

2 changes: 2 additions & 0 deletions src/libcore/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1968,11 +1968,13 @@ 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)]
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 }
}
9 changes: 9 additions & 0 deletions src/test/codegen/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR, this loop copies one byte at a time. So it's a nice improvement.

*y = x;
}
}
112 changes: 112 additions & 0 deletions src/test/run-pass/iter-zip.rs
Original file line number Diff line number Diff line change
@@ -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();
}