Skip to content

Commit

Permalink
cmp: switch min and max to TotalOrd
Browse files Browse the repository at this point in the history
The `Float` trait provides correct `min` and `max` methods on floating
point types, providing a consistent result regardless of the order the
parameters are passed.

These generic functions do not take the necessary performance hit to
correctly support a partial order, so the true requirement should be
given as a type bound.

Closes #12712
  • Loading branch information
thestinger committed Mar 14, 2014
1 parent 3fbee34 commit 4e1c215
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 9 deletions.
2 changes: 0 additions & 2 deletions src/doc/guide-tasks.md
Expand Up @@ -353,8 +353,6 @@ fn pnorm(nums: &~[f64], p: uint) -> f64 {
fn main() {
let numbers = vec::from_fn(1000000, |_| rand::random::<f64>());
println!("Inf-norm = {}", *numbers.iter().max().unwrap());
let numbers_arc = Arc::new(numbers);
for num in range(1u, 10) {
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/cmp.rs
Expand Up @@ -184,12 +184,12 @@ pub trait Equiv<T> {
}

#[inline]
pub fn min<T:Ord>(v1: T, v2: T) -> T {
pub fn min<T: TotalOrd>(v1: T, v2: T) -> T {
if v1 < v2 { v1 } else { v2 }
}

#[inline]
pub fn max<T:Ord>(v1: T, v2: T) -> T {
pub fn max<T: TotalOrd>(v1: T, v2: T) -> T {
if v1 > v2 { v1 } else { v2 }
}

Expand Down
8 changes: 4 additions & 4 deletions src/libstd/iter.rs
Expand Up @@ -68,7 +68,7 @@ use cmp;
use num::{Zero, One, CheckedAdd, CheckedSub, Saturating, ToPrimitive, Int};
use option::{Option, Some, None};
use ops::{Add, Mul, Sub};
use cmp::{Eq, Ord};
use cmp::{Eq, Ord, TotalOrd};
use clone::Clone;
use uint;
use mem;
Expand Down Expand Up @@ -626,7 +626,7 @@ pub trait Iterator<A> {
/// assert_eq!(*xs.iter().max_by(|x| x.abs()).unwrap(), -10);
/// ```
#[inline]
fn max_by<B: Ord>(&mut self, f: |&A| -> B) -> Option<A> {
fn max_by<B: TotalOrd>(&mut self, f: |&A| -> B) -> Option<A> {
self.fold(None, |max: Option<(A, B)>, x| {
let x_val = f(&x);
match max {
Expand All @@ -650,7 +650,7 @@ pub trait Iterator<A> {
/// assert_eq!(*xs.iter().min_by(|x| x.abs()).unwrap(), 0);
/// ```
#[inline]
fn min_by<B: Ord>(&mut self, f: |&A| -> B) -> Option<A> {
fn min_by<B: TotalOrd>(&mut self, f: |&A| -> B) -> Option<A> {
self.fold(None, |min: Option<(A, B)>, x| {
let x_val = f(&x);
match min {
Expand Down Expand Up @@ -917,7 +917,7 @@ pub trait OrdIterator<A> {
fn min_max(&mut self) -> MinMaxResult<A>;
}

impl<A: Ord, T: Iterator<A>> OrdIterator<A> for T {
impl<A: TotalOrd, T: Iterator<A>> OrdIterator<A> for T {
#[inline]
fn max(&mut self) -> Option<A> {
self.fold(None, |max, x| {
Expand Down
2 changes: 1 addition & 1 deletion src/libtest/lib.rs
Expand Up @@ -1069,7 +1069,7 @@ impl MetricMap {
if delta.abs() <= noise {
LikelyNoise
} else {
let pct = delta.abs() / cmp::max(vold.value, f64::EPSILON) * 100.0;
let pct = delta.abs() / vold.value.max(f64::EPSILON) * 100.0;
if vold.noise < 0.0 {
// When 'noise' is negative, it means we want
// to see deltas that go up over time, and can
Expand Down

5 comments on commit 4e1c215

@bors
Copy link
Contributor

@bors bors commented on 4e1c215 Mar 14, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 4e1c215 Mar 14, 2014

Choose a reason for hiding this comment

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

merging thestinger/rust/cmp = 4e1c215 into auto

@bors
Copy link
Contributor

@bors bors commented on 4e1c215 Mar 14, 2014

Choose a reason for hiding this comment

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

thestinger/rust/cmp = 4e1c215 merged ok, testing candidate = 42fc32f

@bors
Copy link
Contributor

@bors bors commented on 4e1c215 Mar 14, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = 42fc32f

Please sign in to comment.