Skip to content

Commit

Permalink
Changed implementation of the third field to make LLVM optimize it be…
Browse files Browse the repository at this point in the history
…tter.
  • Loading branch information
kennytm committed Jul 13, 2018
1 parent 6e0dd9e commit 6093128
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 31 deletions.
8 changes: 4 additions & 4 deletions src/libcore/iter/mod.rs
Expand Up @@ -787,19 +787,19 @@ where
#[inline]
fn spec_next(&mut self) -> Option<Self::Item> {
self.first_take = false;
if self.iter.is_empty() {
self.iter.is_iterating = Some(false);
self.iter.compute_is_empty();
if self.iter.is_empty.unwrap_or_default() {
return None;
}
// add 1 to self.step to get original step size back
// it was decremented for the general case on construction
if let Some(n) = self.iter.start.add_usize(self.step+1) {
self.iter.is_iterating = Some(n <= self.iter.end);
self.iter.is_empty = Some(!(n <= self.iter.end));
let next = mem::replace(&mut self.iter.start, n);
Some(next)
} else {
let last = self.iter.start.clone();
self.iter.is_iterating = Some(false);
self.iter.is_empty = Some(true);
Some(last)
}
}
Expand Down
42 changes: 21 additions & 21 deletions src/libcore/iter/range.rs
Expand Up @@ -330,18 +330,18 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {

#[inline]
fn next(&mut self) -> Option<A> {
if self.is_empty() {
self.is_iterating = Some(false);
self.compute_is_empty();
if self.is_empty.unwrap_or_default() {
return None;
}
if self.start < self.end {
let is_iterating = self.start < self.end;
self.is_empty = Some(!is_iterating);
Some(if is_iterating {
let n = self.start.add_one();
self.is_iterating = Some(true);
Some(mem::replace(&mut self.start, n))
mem::replace(&mut self.start, n)
} else {
self.is_iterating = Some(false);
Some(self.start.clone())
}
self.start.clone()
})
}

#[inline]
Expand All @@ -358,8 +358,8 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {

#[inline]
fn nth(&mut self, n: usize) -> Option<A> {
if self.is_empty() {
self.is_iterating = Some(false);
self.compute_is_empty();
if self.is_empty.unwrap_or_default() {
return None;
}

Expand All @@ -368,19 +368,19 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {

match plus_n.partial_cmp(&self.end) {
Some(Less) => {
self.is_iterating = Some(true);
self.is_empty = Some(false);
self.start = plus_n.add_one();
return Some(plus_n)
}
Some(Equal) => {
self.is_iterating = Some(false);
self.is_empty = Some(true);
return Some(plus_n)
}
_ => {}
}
}

self.is_iterating = Some(false);
self.is_empty = Some(true);
None
}

Expand All @@ -404,18 +404,18 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
#[inline]
fn next_back(&mut self) -> Option<A> {
if self.is_empty() {
self.is_iterating = Some(false);
self.compute_is_empty();
if self.is_empty.unwrap_or_default() {
return None;
}
if self.start < self.end {
let is_iterating = self.start < self.end;
self.is_empty = Some(!is_iterating);
Some(if is_iterating {
let n = self.end.sub_one();
self.is_iterating = Some(true);
Some(mem::replace(&mut self.end, n))
mem::replace(&mut self.end, n)
} else {
self.is_iterating = Some(false);
Some(self.end.clone())
}
self.end.clone()
})
}
}

Expand Down
20 changes: 14 additions & 6 deletions src/libcore/ops/range.rs
Expand Up @@ -332,11 +332,11 @@ impl<Idx: PartialOrd<Idx>> RangeTo<Idx> {
pub struct RangeInclusive<Idx> {
pub(crate) start: Idx,
pub(crate) end: Idx,
pub(crate) is_iterating: Option<bool>,
pub(crate) is_empty: Option<bool>,
// This field is:
// - `None` when next() or next_back() was never called
// - `Some(true)` when `start <= end` assuming no overflow
// - `Some(false)` otherwise
// - `Some(false)` when `start <= end` assuming no overflow
// - `Some(true)` otherwise
// The field cannot be a simple `bool` because the `..=` constructor can
// accept non-PartialOrd types, also we want the constructor to be const.
}
Expand All @@ -347,7 +347,7 @@ trait RangeInclusiveEquality: Sized {
impl<T> RangeInclusiveEquality for T {
#[inline]
default fn canonicalized_is_empty(range: &RangeInclusive<Self>) -> bool {
!range.is_iterating.unwrap_or(false)
range.is_empty.unwrap_or_default()
}
}
impl<T: PartialOrd> RangeInclusiveEquality for T {
Expand Down Expand Up @@ -392,7 +392,7 @@ impl<Idx> RangeInclusive<Idx> {
#[stable(feature = "inclusive_range_methods", since = "1.27.0")]
#[inline]
pub const fn new(start: Idx, end: Idx) -> Self {
Self { start, end, is_iterating: None }
Self { start, end, is_empty: None }
}

/// Returns the lower bound of the range (inclusive).
Expand Down Expand Up @@ -536,7 +536,15 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
#[unstable(feature = "range_is_empty", reason = "recently added", issue = "48111")]
#[inline]
pub fn is_empty(&self) -> bool {
!self.is_iterating.unwrap_or_else(|| self.start <= self.end)
self.is_empty.unwrap_or_else(|| !(self.start <= self.end))
}

// If this range's `is_empty` is field is unknown (`None`), update it to be a concrete value.
#[inline]
pub(crate) fn compute_is_empty(&mut self) {
if self.is_empty.is_none() {
self.is_empty = Some(!(self.start <= self.end));
}
}
}

Expand Down

0 comments on commit 6093128

Please sign in to comment.