Make RangeInclusive just a two-field struct

Not being an enum improves ergonomics, especially since NonEmpty could be Empty.  It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.

Implements RFC 1980
This commit is contained in:
Scott McMurray 2017-04-23 21:14:32 -07:00
parent 0bd9e1f5e6
commit f166bd9857
8 changed files with 131 additions and 246 deletions

View file

@ -106,16 +106,10 @@ impl<T> RangeArgument<T> for Range<T> {
#[unstable(feature = "inclusive_range", reason = "recently added, follows RFC", issue = "28237")]
impl<T> RangeArgument<T> for RangeInclusive<T> {
fn start(&self) -> Bound<&T> {
match *self {
RangeInclusive::Empty{ ref at } => Included(at),
RangeInclusive::NonEmpty { ref start, .. } => Included(start),
}
Included(&self.start)
}
fn end(&self) -> Bound<&T> {
match *self {
RangeInclusive::Empty{ ref at } => Excluded(at),
RangeInclusive::NonEmpty { ref end, .. } => Included(end),
}
Included(&self.end)
}
}

View file

@ -403,61 +403,35 @@ impl<A: Step + Clone> Iterator for StepBy<A, ops::RangeInclusive<A>> {
#[inline]
fn next(&mut self) -> Option<A> {
use ops::RangeInclusive::*;
let rev = self.step_by.is_negative();
// this function has a sort of odd structure due to borrowck issues
// we may need to replace self.range, so borrows of start and end need to end early
let (finishing, n) = match self.range {
Empty { .. } => return None, // empty iterators yield no values
NonEmpty { ref mut start, ref mut end } => {
let rev = self.step_by.is_negative();
// march start towards (maybe past!) end and yield the old value
if (rev && start >= end) ||
(!rev && start <= end)
{
match start.step(&self.step_by) {
Some(mut n) => {
mem::swap(start, &mut n);
(None, Some(n)) // yield old value, remain non-empty
},
None => {
let mut n = end.clone();
mem::swap(start, &mut n);
(None, Some(n)) // yield old value, remain non-empty
}
}
} else {
// found range in inconsistent state (start at or past end), so become empty
(Some(end.replace_zero()), None)
}
if (rev && self.range.start >= self.range.end) ||
(!rev && self.range.start <= self.range.end)
{
match self.range.start.step(&self.step_by) {
Some(n) => {
Some(mem::replace(&mut self.range.start, n))
},
None => {
let last = self.range.start.replace_one();
self.range.end.replace_zero();
self.step_by.replace_one();
Some(last)
},
}
};
// turn into an empty iterator if we've reached the end
if let Some(end) = finishing {
self.range = Empty { at: end };
}
n
else {
None
}
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
use ops::RangeInclusive::*;
match self.range {
Empty { .. } => (0, Some(0)),
NonEmpty { ref start, ref end } =>
match Step::steps_between(start,
end,
&self.step_by) {
Some(hint) => (hint.saturating_add(1), hint.checked_add(1)),
None => (0, None)
}
match Step::steps_between(&self.range.start,
&self.range.end,
&self.step_by) {
Some(hint) => (hint.saturating_add(1), hint.checked_add(1)),
None => (0, None)
}
}
}
@ -583,56 +557,27 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> where
#[inline]
fn next(&mut self) -> Option<A> {
use ops::RangeInclusive::*;
use cmp::Ordering::*;
// this function has a sort of odd structure due to borrowck issues
// we may need to replace self, so borrows of self.start and self.end need to end early
let (finishing, n) = match *self {
Empty { .. } => (None, None), // empty iterators yield no values
NonEmpty { ref mut start, ref mut end } => {
if start == end {
(Some(end.replace_one()), Some(start.replace_one()))
} else if start < end {
let mut n = start.add_one();
mem::swap(&mut n, start);
// if the iterator is done iterating, it will change from
// NonEmpty to Empty to avoid unnecessary drops or clones,
// we'll reuse either start or end (they are equal now, so
// it doesn't matter which) to pull out end, we need to swap
// something back in
(if n == *end { Some(end.replace_one()) } else { None },
// ^ are we done yet?
Some(n)) // < the value to output
} else {
(Some(start.replace_one()), None)
}
}
};
// turn into an empty iterator if this is the last value
if let Some(end) = finishing {
*self = Empty { at: end };
match self.start.partial_cmp(&self.end) {
Some(Less) => {
let n = self.start.add_one();
Some(mem::replace(&mut self.start, n))
},
Some(Equal) => {
let last = self.start.replace_one();
self.end.replace_zero();
Some(last)
},
_ => None,
}
n
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
use ops::RangeInclusive::*;
match *self {
Empty { .. } => (0, Some(0)),
NonEmpty { ref start, ref end } =>
match Step::steps_between_by_one(start, end) {
Some(hint) => (hint.saturating_add(1), hint.checked_add(1)),
None => (0, None),
}
match Step::steps_between_by_one(&self.start, &self.end) {
Some(hint) => (hint.saturating_add(1), hint.checked_add(1)),
None => (0, None),
}
}
}
@ -644,33 +589,20 @@ impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> where
{
#[inline]
fn next_back(&mut self) -> Option<A> {
use ops::RangeInclusive::*;
use cmp::Ordering::*;
// see Iterator::next for comments
let (finishing, n) = match *self {
Empty { .. } => return None,
NonEmpty { ref mut start, ref mut end } => {
if start == end {
(Some(start.replace_one()), Some(end.replace_one()))
} else if start < end {
let mut n = end.sub_one();
mem::swap(&mut n, end);
(if n == *start { Some(start.replace_one()) } else { None },
Some(n))
} else {
(Some(end.replace_one()), None)
}
}
};
if let Some(start) = finishing {
*self = Empty { at: start };
match self.start.partial_cmp(&self.end) {
Some(Less) => {
let n = self.end.sub_one();
Some(mem::replace(&mut self.end, n))
},
Some(Equal) => {
let last = self.end.replace_zero();
self.start.replace_one();
Some(last)
},
_ => None,
}
n
}
}

View file

@ -2271,7 +2271,7 @@ impl<Idx: PartialOrd<Idx>> RangeTo<Idx> {
/// ```
/// #![feature(inclusive_range,inclusive_range_syntax)]
/// fn main() {
/// assert_eq!((3...5), std::ops::RangeInclusive::NonEmpty{ start: 3, end: 5 });
/// assert_eq!((3...5), std::ops::RangeInclusive{ start: 3, end: 5 });
/// assert_eq!(3+4+5, (3...5).sum());
///
/// let arr = [0, 1, 2, 3];
@ -2281,45 +2281,23 @@ impl<Idx: PartialOrd<Idx>> RangeTo<Idx> {
/// ```
#[derive(Clone, PartialEq, Eq, Hash)] // not Copy -- see #27186
#[unstable(feature = "inclusive_range", reason = "recently added, follows RFC", issue = "28237")]
pub enum RangeInclusive<Idx> {
/// Empty range (iteration has finished)
pub struct RangeInclusive<Idx> {
/// The lower bound of the range (inclusive).
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
Empty {
/// The point at which iteration finished
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
at: Idx
},
/// Non-empty range (iteration will yield value(s))
pub start: Idx,
/// The upper bound of the range (inclusive).
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
NonEmpty {
/// The lower bound of the range (inclusive).
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
start: Idx,
/// The upper bound of the range (inclusive).
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
end: Idx,
},
pub end: Idx,
}
#[unstable(feature = "inclusive_range", reason = "recently added, follows RFC", issue = "28237")]
impl<Idx: fmt::Debug> fmt::Debug for RangeInclusive<Idx> {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
use self::RangeInclusive::*;
match *self {
Empty { ref at } => write!(fmt, "[empty range @ {:?}]", at),
NonEmpty { ref start, ref end } => write!(fmt, "{:?}...{:?}", start, end),
}
write!(fmt, "{:?}...{:?}", self.start, self.end)
}
}
@ -2341,9 +2319,7 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
/// }
/// ```
pub fn contains(&self, item: Idx) -> bool {
if let &RangeInclusive::NonEmpty{ref start, ref end} = self {
(*start <= item) && (item <= *end)
} else { false }
self.start <= item && item <= self.end
}
}

View file

@ -981,95 +981,82 @@ impl<T> SliceIndex<[T]> for ops::RangeInclusive<usize> {
#[inline]
fn get(self, slice: &[T]) -> Option<&[T]> {
match self {
ops::RangeInclusive::Empty { .. } => Some(&[]),
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() => None,
ops::RangeInclusive::NonEmpty { start, end } => (start..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]> {
match self {
ops::RangeInclusive::Empty { .. } => Some(&mut []),
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() => None,
ops::RangeInclusive::NonEmpty { start, end } => (start..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] {
match self {
ops::RangeInclusive::Empty { .. } => &[],
ops::RangeInclusive::NonEmpty { start, end } => (start..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] {
match self {
ops::RangeInclusive::Empty { .. } => &mut [],
ops::RangeInclusive::NonEmpty { start, end } => {
(start..end + 1).get_unchecked_mut(slice)
}
}
(self.start..self.end + 1).get_unchecked_mut(slice)
}
#[inline]
fn index(self, slice: &[T]) -> &[T] {
match self {
ops::RangeInclusive::Empty { .. } => &[],
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() => {
panic!("attempted to index slice up to maximum usize");
},
ops::RangeInclusive::NonEmpty { start, end } => (start..end + 1).index(slice),
}
assert!(self.end != usize::max_value(),
"attempted to index slice up to maximum usize");
(self.start..self.end + 1).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut [T]) -> &mut [T] {
match self {
ops::RangeInclusive::Empty { .. } => &mut [],
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() => {
panic!("attempted to index slice up to maximum usize");
},
ops::RangeInclusive::NonEmpty { start, end } => (start..end + 1).index_mut(slice),
}
assert!(self.end != usize::max_value(),
"attempted to index slice up to maximum usize");
(self.start..self.end + 1).index_mut(slice)
}
}
#[cfg(stage0)] // The bootstrap compiler has a different `...` desugar
fn inclusive(start: usize, end: usize) -> ops::RangeInclusive<usize> {
ops::RangeInclusive { start, end }
}
#[cfg(not(stage0))]
fn inclusive(start: usize, end: usize) -> ops::RangeInclusive<usize> {
start...end
}
#[unstable(feature = "inclusive_range", reason = "recently added, follows RFC", issue = "28237")]
impl<T> SliceIndex<[T]> for ops::RangeToInclusive<usize> {
type Output = [T];
#[inline]
fn get(self, slice: &[T]) -> Option<&[T]> {
(0...self.end).get(slice)
inclusive(0, self.end).get(slice)
}
#[inline]
fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> {
(0...self.end).get_mut(slice)
inclusive(0, self.end).get_mut(slice)
}
#[inline]
unsafe fn get_unchecked(self, slice: &[T]) -> &[T] {
(0...self.end).get_unchecked(slice)
inclusive(0, self.end).get_unchecked(slice)
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: &mut [T]) -> &mut [T] {
(0...self.end).get_unchecked_mut(slice)
inclusive(0, self.end).get_unchecked_mut(slice)
}
#[inline]
fn index(self, slice: &[T]) -> &[T] {
(0...self.end).index(slice)
inclusive(0, self.end).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut [T]) -> &mut [T] {
(0...self.end).index_mut(slice)
inclusive(0, self.end).index_mut(slice)
}
}

View file

@ -1724,15 +1724,12 @@ mod traits {
#[inline]
fn index(&self, index: ops::RangeInclusive<usize>) -> &str {
match index {
ops::RangeInclusive::Empty { .. } => "",
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() =>
panic!("attempted to index slice up to maximum usize"),
ops::RangeInclusive::NonEmpty { start, end } =>
self.index(start .. end+1)
}
assert!(index.end != usize::max_value(),
"attempted to index str up to maximum usize");
self.index(index.start .. index.end+1)
}
}
#[unstable(feature = "inclusive_range",
reason = "recently added, follows RFC",
issue = "28237")]
@ -1741,7 +1738,9 @@ mod traits {
#[inline]
fn index(&self, index: ops::RangeToInclusive<usize>) -> &str {
self.index(0...index.end)
assert!(index.end != usize::max_value(),
"attempted to index str up to maximum usize");
self.index(.. index.end+1)
}
}
@ -1751,13 +1750,9 @@ mod traits {
impl ops::IndexMut<ops::RangeInclusive<usize>> for str {
#[inline]
fn index_mut(&mut self, index: ops::RangeInclusive<usize>) -> &mut str {
match index {
ops::RangeInclusive::Empty { .. } => &mut self[0..0], // `&mut ""` doesn't work
ops::RangeInclusive::NonEmpty { end, .. } if end == usize::max_value() =>
panic!("attempted to index str up to maximum usize"),
ops::RangeInclusive::NonEmpty { start, end } =>
self.index_mut(start .. end+1)
}
assert!(index.end != usize::max_value(),
"attempted to index str up to maximum usize");
self.index_mut(index.start .. index.end+1)
}
}
#[unstable(feature = "inclusive_range",
@ -1766,7 +1761,9 @@ mod traits {
impl ops::IndexMut<ops::RangeToInclusive<usize>> for str {
#[inline]
fn index_mut(&mut self, index: ops::RangeToInclusive<usize>) -> &mut str {
self.index_mut(0...index.end)
assert!(index.end != usize::max_value(),
"attempted to index str up to maximum usize");
self.index_mut(.. index.end+1)
}
}
@ -1948,45 +1945,27 @@ mod traits {
type Output = str;
#[inline]
fn get(self, slice: &str) -> Option<&Self::Output> {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..end+1,
}.get(slice)
(self.start..self.end+1).get(slice)
}
#[inline]
fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..end+1,
}.get_mut(slice)
(self.start..self.end+1).get_mut(slice)
}
#[inline]
unsafe fn get_unchecked(self, slice: &str) -> &Self::Output {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..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 {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..end+1,
}.get_unchecked_mut(slice)
(self.start..self.end+1).get_unchecked_mut(slice)
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..end+1,
}.index(slice)
(self.start..self.end+1).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut str) -> &mut Self::Output {
match self {
ops::RangeInclusive::Empty { .. } => 0..0,
ops::RangeInclusive::NonEmpty { start, end } => start..end+1,
}.index_mut(slice)
(self.start..self.end+1).index_mut(slice)
}
}

View file

@ -22,6 +22,7 @@
#![feature(fmt_internals)]
#![feature(iterator_step_by)]
#![feature(i128_type)]
#![feature(inclusive_range)]
#![feature(iter_rfind)]
#![feature(libc)]
#![feature(nonzero)]

View file

@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use core::ops::{Range, RangeFull, RangeFrom, RangeTo};
use core::ops::{Range, RangeFull, RangeFrom, RangeTo, RangeInclusive};
// Test the Range structs without the syntactic sugar.
@ -47,3 +47,19 @@ fn test_full_range() {
// Not much to test.
let _ = RangeFull;
}
#[test]
fn test_range_inclusive() {
let mut r = RangeInclusive { start: 1i8, end: 2 };
assert_eq!(r.next(), Some(1));
assert_eq!(r.next(), Some(2));
assert_eq!(r.next(), None);
r = RangeInclusive { start: 127i8, end: 127 };
assert_eq!(r.next(), Some(127));
assert_eq!(r.next(), None);
r = RangeInclusive { start: -128i8, end: -128 };
assert_eq!(r.next_back(), Some(-128));
assert_eq!(r.next_back(), None);
}

View file

@ -1934,13 +1934,13 @@ impl<'a> LoweringContext<'a> {
ExprKind::Range(ref e1, ref e2, lims) => {
use syntax::ast::RangeLimits::*;
let (path, variant) = match (e1, e2, lims) {
(&None, &None, HalfOpen) => ("RangeFull", None),
(&Some(..), &None, HalfOpen) => ("RangeFrom", None),
(&None, &Some(..), HalfOpen) => ("RangeTo", None),
(&Some(..), &Some(..), HalfOpen) => ("Range", None),
(&None, &Some(..), Closed) => ("RangeToInclusive", None),
(&Some(..), &Some(..), Closed) => ("RangeInclusive", Some("NonEmpty")),
let path = match (e1, e2, lims) {
(&None, &None, HalfOpen) => "RangeFull",
(&Some(..), &None, HalfOpen) => "RangeFrom",
(&None, &Some(..), HalfOpen) => "RangeTo",
(&Some(..), &Some(..), HalfOpen) => "Range",
(&None, &Some(..), Closed) => "RangeToInclusive",
(&Some(..), &Some(..), Closed) => "RangeInclusive",
(_, &None, Closed) =>
panic!(self.diagnostic().span_fatal(
e.span, "inclusive range with no end")),
@ -1957,7 +1957,7 @@ impl<'a> LoweringContext<'a> {
let is_unit = fields.is_empty();
let unstable_span = self.allow_internal_unstable("...", e.span);
let struct_path =
iter::once("ops").chain(iter::once(path)).chain(variant)
iter::once("ops").chain(iter::once(path))
.collect::<Vec<_>>();
let struct_path = self.std_path(unstable_span, &struct_path, is_unit);
let struct_path = hir::QPath::Resolved(None, P(struct_path));