Skip to content

core::iter: Refactor iterators #16225

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

Closed
wants to merge 1 commit into from
Closed
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
86 changes: 24 additions & 62 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,13 +481,11 @@ pub trait Iterator<A> {
/// ```
#[inline]
fn nth(&mut self, mut n: uint) -> Option<A> {
loop {
match self.next() {
Some(x) => if n == 0 { return Some(x) },
None => return None
}
for x in *self {
if n == 0 { return Some(x) }
n -= 1;
}
None
}

/// Loops through the entire iterator, returning the last element of the
Expand Down Expand Up @@ -518,11 +516,8 @@ pub trait Iterator<A> {
#[inline]
fn fold<B>(&mut self, init: B, f: |B, A| -> B) -> B {
let mut accum = init;
loop {
match self.next() {
Some(x) => { accum = f(accum, x); }
None => { break; }
}
for x in *self {
accum = f(accum, x);
}
accum
}
Expand Down Expand Up @@ -720,21 +715,10 @@ pub trait ExactSize<A> : DoubleEndedIterator<A> {
/// If no element matches, None is returned.
#[inline]
fn rposition(&mut self, predicate: |A| -> bool) -> Option<uint> {
let (lower, upper) = self.size_hint();
assert!(upper == Some(lower));
let mut i = lower;
loop {
match self.next_back() {
None => break,
Some(x) => {
i = match i.checked_sub(&1) {
Some(x) => x,
None => fail!("rposition: incorrect ExactSize")
};
if predicate(x) {
return Some(i)
}
}
let len = self.len();
for i in range(0, len).rev() {
if predicate(self.next_back().expect("rposition: incorrect ExactSize")) {
return Some(i);
}
}
None
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the removal of this is why you're reporting better numbers on the benchmark.

Right now clients of ExactSize typically include this test. I don't think all clients should be including the test, instead len() should do any assertions necessary, but I'm not sure if sneaking that into this PR is appropriate (especially since you're only updating one client and not all).

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't len() (as defined just below) already include this same assertion, though?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh it already does the assertions? Then why is everyone using size_hint() and asserting manually?

Expand All @@ -744,7 +728,7 @@ pub trait ExactSize<A> : DoubleEndedIterator<A> {
/// Return the exact length of the iterator.
fn len(&self) -> uint {
let (lower, upper) = self.size_hint();
assert!(upper == Some(lower));
assert_eq!(upper, Some(lower));
lower
}
}
Expand Down Expand Up @@ -1330,18 +1314,12 @@ impl<'a, A, T: Iterator<A>> Iterator<A> for Filter<'a, A, T> {
impl<'a, A, T: DoubleEndedIterator<A>> DoubleEndedIterator<A> for Filter<'a, A, T> {
#[inline]
fn next_back(&mut self) -> Option<A> {
loop {
match self.iter.next_back() {
None => return None,
Some(x) => {
if (self.predicate)(&x) {
return Some(x);
} else {
continue
}
}
for x in self.iter.by_ref().rev() {
if (self.predicate)(&x) {
return Some(x);
}
}
None
}
}

Expand Down Expand Up @@ -1375,17 +1353,13 @@ impl<'a, A, B, T: DoubleEndedIterator<A>> DoubleEndedIterator<B>
for FilterMap<'a, A, B, T> {
#[inline]
fn next_back(&mut self) -> Option<B> {
loop {
match self.iter.next_back() {
None => return None,
Some(x) => {
match (self.f)(x) {
Some(y) => return Some(y),
None => ()
}
}
for x in self.iter.by_ref().rev() {
match (self.f)(x) {
Some(y) => return Some(y),
None => ()
}
}
None
}
}

Expand Down Expand Up @@ -1507,25 +1481,13 @@ pub struct SkipWhile<'a, A, T> {
impl<'a, A, T: Iterator<A>> Iterator<A> for SkipWhile<'a, A, T> {
#[inline]
fn next(&mut self) -> Option<A> {
let mut next = self.iter.next();
if self.flag {
next
} else {
loop {
match next {
Some(x) => {
if (self.predicate)(&x) {
next = self.iter.next();
continue
} else {
self.flag = true;
return Some(x)
}
}
None => return None
}
for x in self.iter {
if self.flag || !(self.predicate)(&x) {
self.flag = true;
return Some(x);
}
}
None
}

#[inline]
Expand Down
31 changes: 31 additions & 0 deletions src/libcoretest/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use core::uint;
use core::cmp;
use core::num;

use test::Bencher;

#[test]
fn test_lt() {
let empty: [int, ..0] = [];
Expand Down Expand Up @@ -270,6 +272,7 @@ fn test_iterator_nth() {
for i in range(0u, v.len()) {
assert_eq!(v.iter().nth(i).unwrap(), &v[i]);
}
assert_eq!(v.iter().nth(v.len()), None);
}

#[test]
Expand Down Expand Up @@ -842,3 +845,31 @@ fn test_iterate() {
assert_eq!(it.next(), Some(4u));
assert_eq!(it.next(), Some(8u));
}

#[bench]
fn bench_rposition(b: &mut Bencher) {
let it: Vec<uint> = range(0u, 300).collect();
b.iter(|| {
it.iter().rposition(|&x| x <= 150);
});
}

#[bench]
fn bench_skip_while(b: &mut Bencher) {
b.iter(|| {
let it = range(0u, 100);
let mut sum = 0;
it.skip_while(|&x| { sum += x; sum < 4000 }).all(|_| true);
});
}

#[bench]
fn bench_multiple_take(b: &mut Bencher) {
let mut it = range(0u, 42).cycle();
b.iter(|| {
let n = it.next().unwrap();
for m in range(0u, n) {
it.take(it.next().unwrap()).all(|_| true);
}
});
}