Skip to content

Commit

Permalink
Work-around optimiser deficiencies in Range iterator.
Browse files Browse the repository at this point in the history
The conditional mutation of the previous implementation resulted in poor
code, making it unconditional makes `Range` less well behaved as an
Iterator (but still legal) but also makes it fast.

The intention is that this change will be reverted when rustc/LLVM
handle the best-behaved implementation better.

cc rust-lang#24660
  • Loading branch information
huonw committed Apr 22, 2015
1 parent 7397bdc commit 97b7c76
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2737,9 +2737,17 @@ impl<A: Step + One> Iterator for ops::Range<A> where

#[inline]
fn next(&mut self) -> Option<A> {
if self.start < self.end {
let mut n = &self.start + &A::one();
mem::swap(&mut n, &mut self.start);
// FIXME #24660: this may start returning Some after returning
// None if the + overflows. This is OK per Iterator's
// definition, but it would be really nice for a core iterator
// like `x..y` to be as well behaved as
// possible. Unfortunately, for types like `i32`, LLVM
// mishandles the version that places the mutation inside the
// `if`: it seems to optimise the `Option<i32>` in a way that
// confuses it.
let mut n = &self.start + &A::one();
mem::swap(&mut n, &mut self.start);
if n < self.end {
Some(n)
} else {
None
Expand Down
7 changes: 7 additions & 0 deletions src/libcoretest/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,3 +932,10 @@ fn bench_max(b: &mut Bencher) {
it.map(scatter).max()
})
}

#[bench]
fn bench_range_constant_fold(b: &mut Bencher) {
// this should be constant-folded to just '1000', and so this
// benchmark should run quickly...
b.iter(|| (0..1000).count())
}

0 comments on commit 97b7c76

Please sign in to comment.