diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs index 9ab8ab8672dfa..6954438e93a0a 100644 --- a/src/libcore/cmp.rs +++ b/src/libcore/cmp.rs @@ -362,6 +362,8 @@ pub trait PartialOrd: PartialEq { /// Compare and return the minimum of two values. /// +/// Returns the first argument if the comparison determines them to be equal. +/// /// # Examples /// /// ``` @@ -373,11 +375,13 @@ pub trait PartialOrd: PartialEq { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn min(v1: T, v2: T) -> T { - if v1 < v2 { v1 } else { v2 } + if v1 <= v2 { v1 } else { v2 } } /// Compare and return the maximum of two values. /// +/// Returns the second argument if the comparison determines them to be equal. +/// /// # Examples /// /// ``` @@ -389,7 +393,7 @@ pub fn min(v1: T, v2: T) -> T { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn max(v1: T, v2: T) -> T { - if v1 > v2 { v1 } else { v2 } + if v2 >= v1 { v2 } else { v1 } } /// Compare and return the minimum of two values if there is one. @@ -427,7 +431,7 @@ pub fn partial_min(v1: T, v2: T) -> Option { /// Compare and return the maximum of two values if there is one. /// -/// Returns the first argument if the comparison determines them to be equal. +/// Returns the second argument if the comparison determines them to be equal. /// /// # Examples /// @@ -452,8 +456,8 @@ pub fn partial_min(v1: T, v2: T) -> Option { #[unstable(feature = "core")] pub fn partial_max(v1: T, v2: T) -> Option { match v1.partial_cmp(&v2) { - Some(Less) => Some(v2), - Some(Equal) | Some(Greater) => Some(v1), + Some(Equal) | Some(Less) => Some(v2), + Some(Greater) => Some(v1), None => None } } diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index 39bf97ae1ce1c..ca83a128c994c 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -722,6 +722,9 @@ pub trait Iterator { /// Consumes the entire iterator to return the maximum element. /// + /// Returns the rightmost element if the comparison determines two elements + /// to be equally maximum. + /// /// # Examples /// /// ``` @@ -732,16 +735,19 @@ pub trait Iterator { #[stable(feature = "rust1", since = "1.0.0")] fn max(self) -> Option where Self: Sized, Self::Item: Ord { - self.fold(None, |max, x| { + self.fold(None, |max, y| { match max { - None => Some(x), - Some(y) => Some(cmp::max(x, y)) + None => Some(y), + Some(x) => Some(cmp::max(x, y)) } }) } /// Consumes the entire iterator to return the minimum element. /// + /// Returns the leftmost element if the comparison determines two elements + /// to be equally minimum. + /// /// # Examples /// /// ``` @@ -752,10 +758,10 @@ pub trait Iterator { #[stable(feature = "rust1", since = "1.0.0")] fn min(self) -> Option where Self: Sized, Self::Item: Ord { - self.fold(None, |min, x| { + self.fold(None, |min, y| { match min { - None => Some(x), - Some(y) => Some(cmp::min(x, y)) + None => Some(y), + Some(x) => Some(cmp::min(x, y)) } }) } @@ -799,7 +805,7 @@ pub trait Iterator { Some(x) => { match self.next() { None => return OneElement(x), - Some(y) => if x < y {(x, y)} else {(y,x)} + Some(y) => if x <= y {(x, y)} else {(y, x)} } } }; @@ -817,19 +823,19 @@ pub trait Iterator { None => { if first < min { min = first; - } else if first > max { + } else if first >= max { max = first; } break; } Some(x) => x }; - if first < second { - if first < min {min = first;} - if max < second {max = second;} + if first <= second { + if first < min { min = first } + if second >= max { max = second } } else { - if second < min {min = second;} - if max < first {max = first;} + if second < min { min = second } + if first >= max { max = first } } } @@ -839,6 +845,9 @@ pub trait Iterator { /// Return the element that gives the maximum value from the /// specified function. /// + /// Returns the rightmost element if the comparison determines two elements + /// to be equally maximum. + /// /// # Examples /// /// ``` @@ -855,14 +864,14 @@ pub trait Iterator { Self: Sized, F: FnMut(&Self::Item) -> B, { - self.fold(None, |max: Option<(Self::Item, B)>, x| { - let x_val = f(&x); + self.fold(None, |max: Option<(Self::Item, B)>, y| { + let y_val = f(&y); match max { - None => Some((x, x_val)), - Some((y, y_val)) => if x_val > y_val { - Some((x, x_val)) - } else { + None => Some((y, y_val)), + Some((x, x_val)) => if y_val >= x_val { Some((y, y_val)) + } else { + Some((x, x_val)) } } }).map(|(x, _)| x) @@ -871,6 +880,9 @@ pub trait Iterator { /// Return the element that gives the minimum value from the /// specified function. /// + /// Returns the leftmost element if the comparison determines two elements + /// to be equally minimum. + /// /// # Examples /// /// ``` @@ -887,11 +899,11 @@ pub trait Iterator { Self: Sized, F: FnMut(&Self::Item) -> B, { - self.fold(None, |min: Option<(Self::Item, B)>, x| { - let x_val = f(&x); + self.fold(None, |min: Option<(Self::Item, B)>, y| { + let y_val = f(&y); match min { - None => Some((x, x_val)), - Some((y, y_val)) => if x_val < y_val { + None => Some((y, y_val)), + Some((x, x_val)) => if x_val <= y_val { Some((x, x_val)) } else { Some((y, y_val)) diff --git a/src/test/run-pass/minmax-stability-issue-23687.rs b/src/test/run-pass/minmax-stability-issue-23687.rs new file mode 100644 index 0000000000000..86dd1a04532b4 --- /dev/null +++ b/src/test/run-pass/minmax-stability-issue-23687.rs @@ -0,0 +1,83 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(core)] +use std::fmt::Debug; +use std::cmp::{self, PartialOrd, Ordering}; +use std::iter::MinMaxResult::MinMax; + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +struct Foo { + n: u8, + name: &'static str +} + +impl PartialOrd for Foo { + fn partial_cmp(&self, other: &Foo) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Foo { + fn cmp(&self, other: &Foo) -> Ordering { + self.n.cmp(&other.n) + } +} + +fn main() { + let a = Foo { n: 4, name: "a" }; + let b = Foo { n: 4, name: "b" }; + let c = Foo { n: 8, name: "c" }; + let d = Foo { n: 8, name: "d" }; + let e = Foo { n: 22, name: "e" }; + let f = Foo { n: 22, name: "f" }; + + let data = [a, b, c, d, e, f]; + + // `min` should return the left when the values are equal + assert_eq!(data.iter().min(), Some(&a)); + assert_eq!(data.iter().min_by(|a| a.n), Some(&a)); + assert_eq!(cmp::min(a, b), a); + assert_eq!(cmp::min(b, a), b); + assert_eq!(cmp::partial_min(a, b), Some(a)); + assert_eq!(cmp::partial_min(b, a), Some(b)); + + // `max` should return the right when the values are equal + assert_eq!(data.iter().max(), Some(&f)); + assert_eq!(data.iter().max_by(|a| a.n), Some(&f)); + assert_eq!(cmp::max(e, f), f); + assert_eq!(cmp::max(f, e), e); + assert_eq!(cmp::partial_max(e, f), Some(f)); + assert_eq!(cmp::partial_max(f, e), Some(e)); + + // Similar for `min_max` + assert_eq!(data.iter().min_max(), MinMax(&a, &f)); + assert_eq!(data[1..5].iter().min_max(), MinMax(&b, &e)); + assert_eq!(data[2..4].iter().min_max(), MinMax(&c, &d)); + + let mut presorted = data.to_vec(); + presorted.sort(); + assert_stable(&presorted); + + let mut presorted = data.to_vec(); + presorted.sort_by(|a, b| a.cmp(b)); + assert_stable(&presorted); + + // Assert that sorted and min/max are the same + fn assert_stable(presorted: &[T]) { + for slice in presorted.windows(2) { + let a = &slice[0]; + let b = &slice[1]; + + assert_eq!(a, cmp::min(a, b)); + assert_eq!(b, cmp::max(a, b)); + } + } +}