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

Change RangeInclusive to a three-field struct. #51622

Merged
merged 5 commits into from
Jul 13, 2018
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
8 changes: 5 additions & 3 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -2262,36 +2262,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
Original file line number Diff line number Diff line change
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
Loading