Skip to content

Remove duplicated code from Iterator::{ne, lt, le, gt, ge} #59262

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Remove duplicated code from Iterator::{ne, lt, le, gt, ge} #59262

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
079d9b1
Forward Iterator::{ne, lt, le, gt, ge} to Iterator::{eq, partial_cmp}
timvermeulen Mar 17, 2019
6778396
Add iter::{bench_partial_cmp, bench_lt} benchmarks
timvermeulen Mar 17, 2019
075b269
Simplify Iterator::{lt, gt}
timvermeulen Mar 18, 2019
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
10 changes: 10 additions & 0 deletions src/libcore/benches/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,3 +334,13 @@ fn bench_filter_chain_ref_count(b: &mut Bencher) {
(0i64..1000000).chain(0..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count()
})
}

#[bench]
fn bench_partial_cmp(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).partial_cmp((0..100000).map(black_box)))
}

#[bench]
fn bench_lt(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).lt((0..100000).map(black_box)))
}
114 changes: 18 additions & 96 deletions src/libcore/iter/traits/iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2435,145 +2435,67 @@ pub trait Iterator {
/// Determines if the elements of this `Iterator` are unequal to those of
/// another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn ne<I>(mut self, other: I) -> bool where
fn ne<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialEq<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_some(),
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

if x != y { return true }
}
!self.eq(other)
}

/// Determines if the elements of this `Iterator` are lexicographically
/// less than those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn lt<I>(mut self, other: I) -> bool where
fn lt<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_some(),
Some(val) => val,
};

let y = match other.next() {
None => return false,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return true,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return false,
None => return false,
}
match self.partial_cmp(other) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not self.partial_cmp(other) == Some(Ordering::Less)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see, you probably wanted to keep it consistent across le/lt/ge/gt... I think that's okay then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was indeed the rationale, but looking at it again, it might actually be worth it. It's a lot simpler and it's still consistent between lt/gt.

Copy link
Contributor

Choose a reason for hiding this comment

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

So is it worth to also fix std::cmp::PartialOrd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean exactly?

Some(Ordering::Less) => true,
_ => false,
}
}

/// Determines if the elements of this `Iterator` are lexicographically
/// less or equal to those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn le<I>(mut self, other: I) -> bool where
fn le<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => { other.next(); return true; },
Some(val) => val,
};

let y = match other.next() {
None => return false,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return true,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return false,
None => return false,
}
match self.partial_cmp(other) {
Some(Ordering::Less) | Some(Ordering::Equal) => true,
_ => false,
}
}

/// Determines if the elements of this `Iterator` are lexicographically
/// greater than those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn gt<I>(mut self, other: I) -> bool where
fn gt<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => { other.next(); return false; },
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return false,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return true,
None => return false,
}
match self.partial_cmp(other) {
Some(Ordering::Greater) => true,
_ => false,
}
}

/// Determines if the elements of this `Iterator` are lexicographically
/// greater than or equal to those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn ge<I>(mut self, other: I) -> bool where
fn ge<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_none(),
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return false,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return true,
None => return false,
}
match self.partial_cmp(other) {
Some(Ordering::Greater) | Some(Ordering::Equal) => true,
_ => false,
}
}

Expand Down