Skip to content

Commit

Permalink
Auto merge of #51622 - kennytm:three-field-range-inclusive, r=SimonSapin
Browse files Browse the repository at this point in the history 
Change RangeInclusive to a three-field struct.

Fix #45222.

This PR also reverts #48012 (i.e. removed the `try_fold`/`try_rfold` specialization for `RangeInclusive`) because LLVM no longer has trouble recognizing a RangeInclusive loop.
  • Loading branch information
@bors
bors committed Jul 13, 2018
2 parents 8b48b24 + 6093128 commit c0955a3
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 112 deletions.
8 changes: 5 additions & 3 deletions src/libcore/iter/mod.rs
View file
Expand Up @@ -787,17 +787,19 @@ where
#[inline]
fn spec_next(&mut self) -> Option<Self::Item> {
self.first_take = false;
if !(self.iter.start <= self.iter.end) {
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_empty = Some(!(n <= self.iter.end));
let next = mem::replace(&mut self.iter.start, n);
Some(next)
} else {
let last = self.iter.start.replace_one();
self.iter.end.replace_zero();
let last = self.iter.start.clone();
self.iter.is_empty = Some(true);
Some(last)
}
}
Expand Down
104 changes: 32 additions & 72 deletions src/libcore/iter/range.rs
View file
Expand Up @@ -10,7 +10,7 @@

use convert::TryFrom;
use mem;
use ops::{self, Add, Sub, Try};
use ops::{self, Add, Sub};
use usize;

use super::{FusedIterator, TrustedLen};
Expand Down Expand Up @@ -330,23 +330,23 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {

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

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
if !(self.start <= self.end) {
if self.is_empty() {
return (0, Some(0));
}

Expand All @@ -358,25 +358,29 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {

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

if let Some(plus_n) = self.start.add_usize(n) {
use cmp::Ordering::*;

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

self.start.replace_one();
self.end.replace_zero();
self.is_empty = Some(true);
None
}

Expand All @@ -394,68 +398,24 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
fn max(mut self) -> Option<A> {
self.next_back()
}

#[inline]
fn try_fold<B, F, R>(&mut self, init: B, mut f: F) -> R where
Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try<Ok=B>
{
let mut accum = init;
if self.start <= self.end {
loop {
let (x, done) =
if self.start < self.end {
let n = self.start.add_one();
(mem::replace(&mut self.start, n), false)
} else {
self.end.replace_zero();
(self.start.replace_one(), true)
};
accum = f(accum, x)?;
if done { break }
}
}
Try::from_ok(accum)
}
}

#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
#[inline]
fn next_back(&mut self) -> Option<A> {
if self.start <= self.end {
if self.start < self.end {
let n = self.end.sub_one();
Some(mem::replace(&mut self.end, n))
} else {
let last = self.end.replace_zero();
self.start.replace_one();
Some(last)
}
} else {
None
self.compute_is_empty();
if self.is_empty.unwrap_or_default() {
return None;
}
}

#[inline]
fn try_rfold<B, F, R>(&mut self, init: B, mut f: F) -> R where
Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try<Ok=B>
{
let mut accum = init;
if self.start <= self.end {
loop {
let (x, done) =
if self.start < self.end {
let n = self.end.sub_one();
(mem::replace(&mut self.end, n), false)
} else {
self.start.replace_one();
(self.end.replace_zero(), true)
};
accum = f(accum, x)?;
if done { break }
}
}
Try::from_ok(accum)
let is_iterating = self.start < self.end;
self.is_empty = Some(!is_iterating);
Some(if is_iterating {
let n = self.end.sub_one();
mem::replace(&mut self.end, n)
} else {
self.end.clone()
})
}
}

Expand Down
65 changes: 58 additions & 7 deletions src/libcore/ops/range.rs
View file
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

use fmt;
use hash::{Hash, Hasher};

/// An unbounded range (`..`).
///
Expand Down Expand Up @@ -326,15 +327,56 @@ impl<Idx: PartialOrd<Idx>> RangeTo<Idx> {
/// assert_eq!(arr[1..=2], [ 1,2 ]); // RangeInclusive
/// ```
#[doc(alias = "..=")]
#[derive(Clone, PartialEq, Eq, Hash)] // not Copy -- see #27186
#[derive(Clone)] // not Copy -- see #27186
#[stable(feature = "inclusive_range", since = "1.26.0")]
pub struct RangeInclusive<Idx> {
// FIXME: The current representation follows RFC 1980,
// but it is known that LLVM is not able to optimize loops following that RFC.
// Consider adding an extra `bool` field to indicate emptiness of the range.
// See #45222 for performance test cases.
pub(crate) start: Idx,
pub(crate) end: Idx,
pub(crate) is_empty: Option<bool>,
// This field is:
// - `None` when next() or next_back() was never called
// - `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.
}

trait RangeInclusiveEquality: Sized {
fn canonicalized_is_empty(range: &RangeInclusive<Self>) -> bool;
}
impl<T> RangeInclusiveEquality for T {
#[inline]
default fn canonicalized_is_empty(range: &RangeInclusive<Self>) -> bool {
range.is_empty.unwrap_or_default()
}
}
impl<T: PartialOrd> RangeInclusiveEquality for T {
#[inline]
fn canonicalized_is_empty(range: &RangeInclusive<Self>) -> bool {
range.is_empty()
}
}

#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: PartialEq> PartialEq for RangeInclusive<Idx> {
#[inline]
fn eq(&self, other: &Self) -> bool {
self.start == other.start && self.end == other.end
&& RangeInclusiveEquality::canonicalized_is_empty(self)
== RangeInclusiveEquality::canonicalized_is_empty(other)
}
}

#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: Eq> Eq for RangeInclusive<Idx> {}

#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: Hash> Hash for RangeInclusive<Idx> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.start.hash(state);
self.end.hash(state);
RangeInclusiveEquality::canonicalized_is_empty(self).hash(state);
}
}

impl<Idx> RangeInclusive<Idx> {
Expand All @@ -350,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 }
Self { start, end, is_empty: None }
}

/// Returns the lower bound of the range (inclusive).
Expand Down Expand Up @@ -492,8 +534,17 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
/// assert!(r.is_empty());
/// ```
#[unstable(feature = "range_is_empty", reason = "recently added", issue = "48111")]
#[inline]
pub fn is_empty(&self) -> bool {
!(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
20 changes: 10 additions & 10 deletions src/libcore/slice/mod.rs
View file
Expand Up @@ -2265,36 +2265,36 @@ impl<T> SliceIndex<[T]> for ops::RangeInclusive<usize> {

#[inline]
fn get(self, slice: &[T]) -> Option<&[T]> {
if self.end == usize::max_value() { None }
else { (self.start..self.end + 1).get(slice) }
if *self.end() == usize::max_value() { None }
else { (*self.start()..self.end() + 1).get(slice) }
}

#[inline]
fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> {
if self.end == usize::max_value() { None }
else { (self.start..self.end + 1).get_mut(slice) }
if *self.end() == usize::max_value() { None }
else { (*self.start()..self.end() + 1).get_mut(slice) }
}

#[inline]
unsafe fn get_unchecked(self, slice: &[T]) -> &[T] {
(self.start..self.end + 1).get_unchecked(slice)
(*self.start()..self.end() + 1).get_unchecked(slice)
}

#[inline]
unsafe fn get_unchecked_mut(self, slice: &mut [T]) -> &mut [T] {
(self.start..self.end + 1).get_unchecked_mut(slice)
(*self.start()..self.end() + 1).get_unchecked_mut(slice)
}

#[inline]
fn index(self, slice: &[T]) -> &[T] {
if self.end == usize::max_value() { slice_index_overflow_fail(); }
(self.start..self.end + 1).index(slice)
if *self.end() == usize::max_value() { slice_index_overflow_fail(); }
(*self.start()..self.end() + 1).index(slice)
}

#[inline]
fn index_mut(self, slice: &mut [T]) -> &mut [T] {
if self.end == usize::max_value() { slice_index_overflow_fail(); }
(self.start..self.end + 1).index_mut(slice)
if *self.end() == usize::max_value() { slice_index_overflow_fail(); }
(*self.start()..self.end() + 1).index_mut(slice)
}
}

Expand Down
20 changes: 10 additions & 10 deletions src/libcore/str/mod.rs
View file
Expand Up @@ -2004,31 +2004,31 @@ mod traits {
type Output = str;
#[inline]
fn get(self, slice: &str) -> Option<&Self::Output> {
if self.end == usize::max_value() { None }
else { (self.start..self.end+1).get(slice) }
if *self.end() == usize::max_value() { None }
else { (*self.start()..self.end()+1).get(slice) }
}
#[inline]
fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {
if self.end == usize::max_value() { None }
else { (self.start..self.end+1).get_mut(slice) }
if *self.end() == usize::max_value() { None }
else { (*self.start()..self.end()+1).get_mut(slice) }
}
#[inline]
unsafe fn get_unchecked(self, slice: &str) -> &Self::Output {
(self.start..self.end+1).get_unchecked(slice)
(*self.start()..self.end()+1).get_unchecked(slice)
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: &mut str) -> &mut Self::Output {
(self.start..self.end+1).get_unchecked_mut(slice)
(*self.start()..self.end()+1).get_unchecked_mut(slice)
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
if self.end == usize::max_value() { str_index_overflow_fail(); }
(self.start..self.end+1).index(slice)
if *self.end() == usize::max_value() { str_index_overflow_fail(); }
(*self.start()..self.end()+1).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut str) -> &mut Self::Output {
if self.end == usize::max_value() { str_index_overflow_fail(); }
(self.start..self.end+1).index_mut(slice)
if *self.end() == usize::max_value() { str_index_overflow_fail(); }
(*self.start()..self.end()+1).index_mut(slice)
}
}

Expand Down

0 comments on commit c0955a3

Please sign in to comment.