Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/libcore/iter/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ pub trait ExactSizeIterator: Iterator {
/// ```
/// #![feature(exact_size_is_empty)]
///
/// let mut one_element = 0..1;
/// let mut one_element = std::iter::once(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this related to this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because using 0..1 means the code wasn't actually using ExactSizeIterator; the inherent method was getting picked up instead. For normal code that's not a problem--they do exactly the same thing--but I figured the doctest should actually use the method it's documenting.

/// assert!(!one_element.is_empty());
///
/// assert_eq!(one_element.next(), Some(0));
Expand Down
36 changes: 34 additions & 2 deletions src/libcore/ops/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ impl<Idx: fmt::Debug> fmt::Debug for Range<Idx> {
}
}

#[unstable(feature = "range_contains", reason = "recently added as per RFC", issue = "32311")]
impl<Idx: PartialOrd<Idx>> Range<Idx> {
/// Returns `true` if `item` is contained in the range.
///
Expand All @@ -109,9 +108,26 @@ impl<Idx: PartialOrd<Idx>> Range<Idx> {
/// assert!(!(3..3).contains(3));
/// assert!(!(3..2).contains(3));
/// ```
#[unstable(feature = "range_contains", reason = "recently added as per RFC", issue = "32311")]
pub fn contains(&self, item: Idx) -> bool {
(self.start <= item) && (item < self.end)
}

/// Returns `true` if the range contains no items.
///
/// # Examples
///
/// ```
/// #![feature(range_is_empty)]
///
/// assert!(!(3..5).is_empty());
/// assert!( (3..3).is_empty());
/// assert!( (3..2).is_empty());
/// ```
#[unstable(feature = "range_is_empty", reason = "recently added", issue = "123456789")]
pub fn is_empty(&self) -> bool {
!(self.start < self.end)
}
}

/// A range only bounded inclusively below (`start..`).
Expand Down Expand Up @@ -280,7 +296,6 @@ impl<Idx: fmt::Debug> fmt::Debug for RangeInclusive<Idx> {
}
}

#[unstable(feature = "range_contains", reason = "recently added as per RFC", issue = "32311")]
impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
/// Returns `true` if `item` is contained in the range.
///
Expand All @@ -298,9 +313,26 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
/// assert!( (3..=3).contains(3));
/// assert!(!(3..=2).contains(3));
/// ```
#[unstable(feature = "range_contains", reason = "recently added as per RFC", issue = "32311")]
pub fn contains(&self, item: Idx) -> bool {
self.start <= item && item <= self.end
}

/// Returns `true` if the range contains no items.
///
/// # Examples
///
/// ```
/// #![feature(range_is_empty,inclusive_range_syntax)]
///
/// assert!(!(3..=5).is_empty());
/// assert!(!(3..=3).is_empty());
/// assert!( (3..=2).is_empty());
/// ```
#[unstable(feature = "range_is_empty", reason = "recently added", issue = "123456789")]
pub fn is_empty(&self) -> bool {
!(self.start <= self.end)
}
}

/// A range only bounded inclusively above (`..=end`).
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/tests/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1427,9 +1427,9 @@ fn test_range_inclusive_nth() {
assert_eq!(r, 13..=20);
assert_eq!(r.nth(2), Some(15));
assert_eq!(r, 16..=20);
assert_eq!(r.is_empty(), false);
assert_eq!(ExactSizeIterator::is_empty(&r), false);
assert_eq!(r.nth(10), None);
assert_eq!(r.is_empty(), true);
assert_eq!(ExactSizeIterator::is_empty(&r), true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to be a concern for end users?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExactSizeIterator::is_empty (also unstable)

ah, nvm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Range::is_empty is strictly more general, so it's not a problem in a world where everything is stable. It may be a slight hiccup in nightly, but I would expect it to be solved in the wild by adding the new feature gate, rather than the annoying UFCS. Here I just changed it to the other method because I didn't want to change what the test was testing. (It would be a problem if we wanted to stabilize ExactSizeIterator::is_empty first, as mentioned in the OP, but I feel like this should be less controversial than the new trait method, especially since there are a bunch of iterators that aren't ExactSize that could provide an efficient is_empty.)

Copy link
Member Author

@scottmcm scottmcm Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed RangeInclusive exhaustion tests to check is_empty, updated the docs to unspecify the post-iteration value of a RangeInclusive, and added some more Range tests using its is_empty.

assert_eq!(r, 1..=0); // We may not want to document/promise this detail
}

Expand Down
1 change: 1 addition & 0 deletions src/libcore/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#![feature(iter_rfold)]
#![feature(nonzero)]
#![feature(pattern)]
#![feature(range_is_empty)]
#![feature(raw)]
#![feature(refcell_replace_swap)]
#![feature(sip_hash_13)]
Expand Down
24 changes: 24 additions & 0 deletions src/libcore/tests/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,27 @@ fn test_range_inclusive() {
assert_eq!(r.size_hint(), (0, Some(0)));
assert_eq!(r.next(), None);
}


#[test]
fn test_range_is_empty() {
use core::f32::*;

assert!(!(0.0 .. 10.0).is_empty());
assert!( (-0.0 .. 0.0).is_empty());
assert!( (10.0 .. 0.0).is_empty());

assert!(!(NEG_INFINITY .. INFINITY).is_empty());
assert!( (EPSILON .. NAN).is_empty());
assert!( (NAN .. EPSILON).is_empty());
assert!( (NAN .. NAN).is_empty());

assert!(!(0.0 ..= 10.0).is_empty());
assert!(!(-0.0 ..= 0.0).is_empty());
assert!( (10.0 ..= 0.0).is_empty());

assert!(!(NEG_INFINITY ..= INFINITY).is_empty());
assert!( (EPSILON ..= NAN).is_empty());
assert!( (NAN ..= EPSILON).is_empty());
assert!( (NAN ..= NAN).is_empty());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about a test that calls next on a Range{Inclusive} and makes sure it transitions from not empty to empty?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought; I'll change all of these to look for is_empty instead of 1..=0: https://github.com/rust-lang/rust/blob/master/src/libcore/tests/iter.rs#L1433