Skip to content

Commit 3504027

Browse files
committed
auto merge of #8400 : blake2-ppc/rust/seq-ord, r=cmr
Use Eq + Ord for lexicographical ordering of sequences. For each of <, <=, >= or > as R, use:: [x, ..xs] R [y, ..ys] = if x != y { x R y } else { xs R ys } Previous code using `a < b` and then `!(b < a)` for short-circuiting fails on cases such as [1.0, 2.0] < [0.0/0.0, 3.0], where the first element was effectively considered equal. Containers like &[T] did also implement only one comparison operator `<`, and derived the comparison results from this. This isn't correct either for Ord. Implement functions in `std::iterator::order::{lt,le,gt,ge,equal,cmp}` that all iterable containers can use for lexical order. We also visit tuple ordering, having the same problem and same solution (but differing implementation).
2 parents 59434a1 + 854e219 commit 3504027

File tree

6 files changed

+308
-73
lines changed

6 files changed

+308
-73
lines changed

src/libextra/dlist.rs

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use std::cast;
2626
use std::ptr;
2727
use std::util;
2828
use std::iterator::{FromIterator, Extendable, Invert};
29+
use std::iterator;
2930

3031
use container::Deque;
3132

@@ -589,12 +590,27 @@ impl<A, T: Iterator<A>> Extendable<A, T> for DList<A> {
589590
impl<A: Eq> Eq for DList<A> {
590591
fn eq(&self, other: &DList<A>) -> bool {
591592
self.len() == other.len() &&
592-
self.iter().zip(other.iter()).all(|(a, b)| a.eq(b))
593+
iterator::order::eq(self.iter(), other.iter())
593594
}
594595

595-
#[inline]
596596
fn ne(&self, other: &DList<A>) -> bool {
597-
!self.eq(other)
597+
self.len() != other.len() &&
598+
iterator::order::ne(self.iter(), other.iter())
599+
}
600+
}
601+
602+
impl<A: Eq + Ord> Ord for DList<A> {
603+
fn lt(&self, other: &DList<A>) -> bool {
604+
iterator::order::lt(self.iter(), other.iter())
605+
}
606+
fn le(&self, other: &DList<A>) -> bool {
607+
iterator::order::le(self.iter(), other.iter())
608+
}
609+
fn gt(&self, other: &DList<A>) -> bool {
610+
iterator::order::gt(self.iter(), other.iter())
611+
}
612+
fn ge(&self, other: &DList<A>) -> bool {
613+
iterator::order::ge(self.iter(), other.iter())
598614
}
599615
}
600616

@@ -964,6 +980,48 @@ mod tests {
964980
assert_eq!(&n, &m);
965981
}
966982

983+
#[test]
984+
fn test_ord() {
985+
let n: DList<int> = list_from([]);
986+
let m = list_from([1,2,3]);
987+
assert!(n < m);
988+
assert!(m > n);
989+
assert!(n <= n);
990+
assert!(n >= n);
991+
}
992+
993+
#[test]
994+
fn test_ord_nan() {
995+
let nan = 0.0/0.0;
996+
let n = list_from([nan]);
997+
let m = list_from([nan]);
998+
assert!(!(n < m));
999+
assert!(!(n > m));
1000+
assert!(!(n <= m));
1001+
assert!(!(n >= m));
1002+
1003+
let n = list_from([nan]);
1004+
let one = list_from([1.0]);
1005+
assert!(!(n < one));
1006+
assert!(!(n > one));
1007+
assert!(!(n <= one));
1008+
assert!(!(n >= one));
1009+
1010+
let u = list_from([1.0,2.0,nan]);
1011+
let v = list_from([1.0,2.0,3.0]);
1012+
assert!(!(u < v));
1013+
assert!(!(u > v));
1014+
assert!(!(u <= v));
1015+
assert!(!(u >= v));
1016+
1017+
let s = list_from([1.0,2.0,4.0,2.0]);
1018+
let t = list_from([1.0,2.0,3.0,2.0]);
1019+
assert!(!(s < t));
1020+
assert!(s > one);
1021+
assert!(!(s <= one));
1022+
assert!(s >= one);
1023+
}
1024+
9671025
#[test]
9681026
fn test_fuzz() {
9691027
do 25.times {

src/libstd/iterator.rs

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,6 +1568,163 @@ impl<A: Clone> RandomAccessIterator<A> for Repeat<A> {
15681568
fn idx(&self, _: uint) -> Option<A> { Some(self.element.clone()) }
15691569
}
15701570

1571+
/// Functions for lexicographical ordering of sequences.
1572+
///
1573+
/// Lexicographical ordering through `<`, `<=`, `>=`, `>` requires
1574+
/// that the elements implement both `Eq` and `Ord`.
1575+
///
1576+
/// If two sequences are equal up until the point where one ends,
1577+
/// the shorter sequence compares less.
1578+
pub mod order {
1579+
use cmp;
1580+
use cmp::{TotalEq, TotalOrd, Ord, Eq};
1581+
use option::{Some, None};
1582+
use super::Iterator;
1583+
1584+
/// Compare `a` and `b` for equality using `TotalOrd`
1585+
pub fn equals<A: TotalEq, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1586+
loop {
1587+
match (a.next(), b.next()) {
1588+
(None, None) => return true,
1589+
(None, _) | (_, None) => return false,
1590+
(Some(x), Some(y)) => if !x.equals(&y) { return false },
1591+
}
1592+
}
1593+
}
1594+
1595+
/// Order `a` and `b` lexicographically using `TotalOrd`
1596+
pub fn cmp<A: TotalOrd, T: Iterator<A>>(mut a: T, mut b: T) -> cmp::Ordering {
1597+
loop {
1598+
match (a.next(), b.next()) {
1599+
(None, None) => return cmp::Equal,
1600+
(None, _ ) => return cmp::Less,
1601+
(_ , None) => return cmp::Greater,
1602+
(Some(x), Some(y)) => match x.cmp(&y) {
1603+
cmp::Equal => (),
1604+
non_eq => return non_eq,
1605+
},
1606+
}
1607+
}
1608+
}
1609+
1610+
/// Compare `a` and `b` for equality (Using partial equality, `Eq`)
1611+
pub fn eq<A: Eq, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1612+
loop {
1613+
match (a.next(), b.next()) {
1614+
(None, None) => return true,
1615+
(None, _) | (_, None) => return false,
1616+
(Some(x), Some(y)) => if !x.eq(&y) { return false },
1617+
}
1618+
}
1619+
}
1620+
1621+
/// Compare `a` and `b` for nonequality (Using partial equality, `Eq`)
1622+
pub fn ne<A: Eq, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1623+
loop {
1624+
match (a.next(), b.next()) {
1625+
(None, None) => return false,
1626+
(None, _) | (_, None) => return true,
1627+
(Some(x), Some(y)) => if x.ne(&y) { return true },
1628+
}
1629+
}
1630+
}
1631+
1632+
/// Return `a` < `b` lexicographically (Using partial order, `Ord`)
1633+
pub fn lt<A: Eq + Ord, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1634+
loop {
1635+
match (a.next(), b.next()) {
1636+
(None, None) => return false,
1637+
(None, _ ) => return true,
1638+
(_ , None) => return false,
1639+
(Some(x), Some(y)) => if x.ne(&y) { return x.lt(&y) },
1640+
}
1641+
}
1642+
}
1643+
1644+
/// Return `a` <= `b` lexicographically (Using partial order, `Ord`)
1645+
pub fn le<A: Eq + Ord, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1646+
loop {
1647+
match (a.next(), b.next()) {
1648+
(None, None) => return true,
1649+
(None, _ ) => return true,
1650+
(_ , None) => return false,
1651+
(Some(x), Some(y)) => if x.ne(&y) { return x.le(&y) },
1652+
}
1653+
}
1654+
}
1655+
1656+
/// Return `a` > `b` lexicographically (Using partial order, `Ord`)
1657+
pub fn gt<A: Eq + Ord, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1658+
loop {
1659+
match (a.next(), b.next()) {
1660+
(None, None) => return false,
1661+
(None, _ ) => return false,
1662+
(_ , None) => return true,
1663+
(Some(x), Some(y)) => if x.ne(&y) { return x.gt(&y) },
1664+
}
1665+
}
1666+
}
1667+
1668+
/// Return `a` >= `b` lexicographically (Using partial order, `Ord`)
1669+
pub fn ge<A: Eq + Ord, T: Iterator<A>>(mut a: T, mut b: T) -> bool {
1670+
loop {
1671+
match (a.next(), b.next()) {
1672+
(None, None) => return true,
1673+
(None, _ ) => return false,
1674+
(_ , None) => return true,
1675+
(Some(x), Some(y)) => if x.ne(&y) { return x.ge(&y) },
1676+
}
1677+
}
1678+
}
1679+
1680+
#[test]
1681+
fn test_lt() {
1682+
use vec::ImmutableVector;
1683+
1684+
let empty: [int, ..0] = [];
1685+
let xs = [1,2,3];
1686+
let ys = [1,2,0];
1687+
1688+
assert!(!lt(xs.iter(), ys.iter()));
1689+
assert!(!le(xs.iter(), ys.iter()));
1690+
assert!( gt(xs.iter(), ys.iter()));
1691+
assert!( ge(xs.iter(), ys.iter()));
1692+
1693+
assert!( lt(ys.iter(), xs.iter()));
1694+
assert!( le(ys.iter(), xs.iter()));
1695+
assert!(!gt(ys.iter(), xs.iter()));
1696+
assert!(!ge(ys.iter(), xs.iter()));
1697+
1698+
assert!( lt(empty.iter(), xs.iter()));
1699+
assert!( le(empty.iter(), xs.iter()));
1700+
assert!(!gt(empty.iter(), xs.iter()));
1701+
assert!(!ge(empty.iter(), xs.iter()));
1702+
1703+
// Sequence with NaN
1704+
let u = [1.0, 2.0];
1705+
let v = [0.0/0.0, 3.0];
1706+
1707+
assert!(!lt(u.iter(), v.iter()));
1708+
assert!(!le(u.iter(), v.iter()));
1709+
assert!(!gt(u.iter(), v.iter()));
1710+
assert!(!ge(u.iter(), v.iter()));
1711+
1712+
let a = [0.0/0.0];
1713+
let b = [1.0];
1714+
let c = [2.0];
1715+
1716+
assert!(lt(a.iter(), b.iter()) == (a[0] < b[0]));
1717+
assert!(le(a.iter(), b.iter()) == (a[0] <= b[0]));
1718+
assert!(gt(a.iter(), b.iter()) == (a[0] > b[0]));
1719+
assert!(ge(a.iter(), b.iter()) == (a[0] >= b[0]));
1720+
1721+
assert!(lt(c.iter(), b.iter()) == (c[0] < b[0]));
1722+
assert!(le(c.iter(), b.iter()) == (c[0] <= b[0]));
1723+
assert!(gt(c.iter(), b.iter()) == (c[0] > b[0]));
1724+
assert!(ge(c.iter(), b.iter()) == (c[0] >= b[0]));
1725+
}
1726+
}
1727+
15711728
#[cfg(test)]
15721729
mod tests {
15731730
use super::*;

src/libstd/option.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ use ops::Add;
4747
use util;
4848
use num::Zero;
4949
use iterator::Iterator;
50+
use iterator;
5051
use str::{StrSlice, OwnedStr};
5152
use to_str::ToStr;
5253
use clone::DeepClone;
@@ -58,31 +59,21 @@ pub enum Option<T> {
5859
Some(T),
5960
}
6061

61-
impl<T:Ord> Ord for Option<T> {
62+
impl<T: Eq + Ord> Ord for Option<T> {
6263
fn lt(&self, other: &Option<T>) -> bool {
63-
match (self, other) {
64-
(&None, &None) => false,
65-
(&None, &Some(_)) => true,
66-
(&Some(_), &None) => false,
67-
(&Some(ref a), &Some(ref b)) => *a < *b
68-
}
64+
iterator::order::lt(self.iter(), other.iter())
6965
}
7066

7167
fn le(&self, other: &Option<T>) -> bool {
72-
match (self, other) {
73-
(&None, &None) => true,
74-
(&None, &Some(_)) => true,
75-
(&Some(_), &None) => false,
76-
(&Some(ref a), &Some(ref b)) => *a <= *b
77-
}
68+
iterator::order::le(self.iter(), other.iter())
7869
}
7970

8071
fn ge(&self, other: &Option<T>) -> bool {
81-
!(self < other)
72+
iterator::order::ge(self.iter(), other.iter())
8273
}
8374

8475
fn gt(&self, other: &Option<T>) -> bool {
85-
!(self <= other)
76+
iterator::order::gt(self.iter(), other.iter())
8677
}
8778
}
8879

@@ -553,6 +544,18 @@ mod tests {
553544
assert!(it.next().is_none());
554545
}
555546

547+
#[test]
548+
fn test_ord() {
549+
let small = Some(1.0);
550+
let big = Some(5.0);
551+
let nan = Some(0.0/0.0);
552+
assert!(!(nan < big));
553+
assert!(!(nan > big));
554+
assert!(small < big);
555+
assert!(None < big);
556+
assert!(big > None);
557+
}
558+
556559
#[test]
557560
fn test_mutate() {
558561
let mut x = Some(3i);

src/libstd/prelude.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ pub use from_str::FromStr;
7070
pub use to_bytes::IterBytes;
7171
pub use to_str::{ToStr, ToStrConsume};
7272
pub use tuple::{CopyableTuple, ImmutableTuple, ExtendedTupleOps};
73+
pub use tuple::{CloneableTuple1, ImmutableTuple1};
7374
pub use tuple::{CloneableTuple2, CloneableTuple3, CloneableTuple4, CloneableTuple5};
7475
pub use tuple::{CloneableTuple6, CloneableTuple7, CloneableTuple8, CloneableTuple9};
7576
pub use tuple::{CloneableTuple10, CloneableTuple11, CloneableTuple12};

0 commit comments

Comments
 (0)