Skip to content
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.

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

Re-implement float min/max in rust #42430

Merged
merged 1 commit into from
Jun 16, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 28 additions & 0 deletions src/libcore/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,32 @@ impl Float for f32 {
let value: f32 = consts::PI;
self * (value / 180.0f32)
}

/// Returns the maximum of the two numbers.
#[inline]
fn max(self, other: f32) -> f32 {
// IEEE754 says: maxNum(x, y) is the canonicalized number y if x < y, x if y < x, the
// canonicalized number if one operand is a number and the other a quiet NaN. Otherwise it
// is either x or y, canonicalized (this means results might differ among implementations).
// When either x or y is a signalingNaN, then the result is according to 6.2.
//
// Since we do not support sNaN in Rust yet, we do not need to handle them.
// FIXME(nagisa): due to https://bugs.llvm.org/show_bug.cgi?id=33303 we canonicalize by
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || self.is_nan() { other } else { self }) * 1.0
Copy link
Member

Choose a reason for hiding this comment

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

FWIW the musl implementation is quite different, it's not clear to me that these are the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

The only difference between the implementations is handling of the signed zero. IEEE 754 allows to ignore the signed-ness of zero here and, if 0.0 is compared to -0.0, to return either one.

Even the C99 standard does not mention signed zeroes in the annex referenced by musl. It quite literally paraphrases the IEEE 754 and provides an example implementation which matches (with small differences) the one provided here.

Copy link
Member

Choose a reason for hiding this comment

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

Ok seems reasonable!

}

/// Returns the minimum of the two numbers.
#[inline]
fn min(self, other: f32) -> f32 {
// IEEE754 says: minNum(x, y) is the canonicalized number x if x < y, y if y < x, the
// canonicalized number if one operand is a number and the other a quiet NaN. Otherwise it
// is either x or y, canonicalized (this means results might differ among implementations).
// When either x or y is a signalingNaN, then the result is according to 6.2.
//
// Since we do not support sNaN in Rust yet, we do not need to handle them.
// FIXME(nagisa): due to https://bugs.llvm.org/show_bug.cgi?id=33303 we canonicalize by
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || other.is_nan() { self } else { other }) * 1.0
Copy link
Member

Choose a reason for hiding this comment

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

You mean self > other here?

Copy link
Contributor

Choose a reason for hiding this comment

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

self < other so self is that minimal one, seems reasonable.

Copy link
Member Author

Choose a reason for hiding this comment

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

The other one is/was wrong. I’ll just write down some tests.

Copy link
Member

Choose a reason for hiding this comment

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

@bombless 😂

I was commenting on 896a41a9, nagisa fixed it in the new commit b9ed1c6, GitHub somehow messed up and used the old comment referring to the latest change.

}
}
28 changes: 28 additions & 0 deletions src/libcore/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,32 @@ impl Float for f64 {
let value: f64 = consts::PI;
self * (value / 180.0)
}

/// Returns the maximum of the two numbers.
#[inline]
fn max(self, other: f64) -> f64 {
// IEEE754 says: maxNum(x, y) is the canonicalized number y if x < y, x if y < x, the
// canonicalized number if one operand is a number and the other a quiet NaN. Otherwise it
// is either x or y, canonicalized (this means results might differ among implementations).
// When either x or y is a signalingNaN, then the result is according to 6.2.
//
// Since we do not support sNaN in Rust yet, we do not need to handle them.
// FIXME(nagisa): due to https://bugs.llvm.org/show_bug.cgi?id=33303 we canonicalize by
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || self.is_nan() { other } else { self }) * 1.0
}

/// Returns the minimum of the two numbers.
#[inline]
fn min(self, other: f64) -> f64 {
// IEEE754 says: minNum(x, y) is the canonicalized number x if x < y, y if y < x, the
// canonicalized number if one operand is a number and the other a quiet NaN. Otherwise it
// is either x or y, canonicalized (this means results might differ among implementations).
// When either x or y is a signalingNaN, then the result is according to 6.2.
//
// Since we do not support sNaN in Rust yet, we do not need to handle them.
// FIXME(nagisa): due to https://bugs.llvm.org/show_bug.cgi?id=33303 we canonicalize by
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || other.is_nan() { self } else { other }) * 1.0
}
}
7 changes: 7 additions & 0 deletions src/libcore/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2428,6 +2428,13 @@ pub trait Float: Sized {
/// Convert degrees to radians.
#[stable(feature = "deg_rad_conversions", since="1.7.0")]
fn to_radians(self) -> Self;

/// Returns the maximum of the two numbers.
#[stable(feature = "core_float_min_max", since="1.20.0")]
fn max(self, other: Self) -> Self;
/// Returns the minimum of the two numbers.
#[stable(feature = "core_float_min_max", since="1.20.0")]
fn min(self, other: Self) -> Self;
}

macro_rules! from_str_radix_int_impl {
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 @@ -13,6 +13,7 @@
#![feature(box_syntax)]
#![feature(char_escape_debug)]
#![feature(const_fn)]
#![feature(core_float)]
#![feature(core_private_bignum)]
#![feature(core_private_diy_float)]
#![feature(dec2flt)]
Expand Down
54 changes: 54 additions & 0 deletions src/libcore/tests/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,3 +398,57 @@ test_impl_try_from_signed_to_unsigned_err! { test_try_i32u16, i32, u16 }
test_impl_try_from_signed_to_unsigned_err! { test_try_i64u8, i64, u8 }
test_impl_try_from_signed_to_unsigned_err! { test_try_i64u16, i64, u16 }
test_impl_try_from_signed_to_unsigned_err! { test_try_i64u32, i64, u32 }

macro_rules! test_float {
($modname: ident, $fty: ty, $inf: expr, $neginf: expr, $nan: expr) => { mod $modname {
use core::num::Float;
// FIXME(nagisa): these tests should test for sign of -0.0
#[test]
fn min() {
assert_eq!(0.0.min(0.0), 0.0);
assert_eq!((-0.0).min(-0.0), -0.0);
assert_eq!(9.0.min(9.0), 9.0);
assert_eq!((-9.0).min(0.0), -9.0);
assert_eq!(0.0.min(9.0), 0.0);
assert_eq!((-0.0).min(-9.0), -9.0);
assert_eq!($inf.min(9.0), 9.0);
assert_eq!(9.0.min($inf), 9.0);
assert_eq!($inf.min(-9.0), -9.0);
assert_eq!((-9.0).min($inf), -9.0);
assert_eq!($neginf.min(9.0), $neginf);
assert_eq!(9.0.min($neginf), $neginf);
assert_eq!($neginf.min(-9.0), $neginf);
assert_eq!((-9.0).min($neginf), $neginf);
assert_eq!($nan.min(9.0), 9.0);
assert_eq!($nan.min(-9.0), -9.0);
assert_eq!(9.0.min($nan), 9.0);
assert_eq!((-9.0).min($nan), -9.0);
assert!($nan.min($nan).is_nan());
}
#[test]
fn max() {
assert_eq!(0.0.max(0.0), 0.0);
assert_eq!((-0.0).max(-0.0), -0.0);
assert_eq!(9.0.max(9.0), 9.0);
assert_eq!((-9.0).max(0.0), 0.0);
assert_eq!(0.0.max(9.0), 9.0);
assert_eq!((-0.0).max(-9.0), -0.0);
assert_eq!($inf.max(9.0), $inf);
assert_eq!(9.0.max($inf), $inf);
assert_eq!($inf.max(-9.0), $inf);
assert_eq!((-9.0).max($inf), $inf);
assert_eq!($neginf.max(9.0), 9.0);
assert_eq!(9.0.max($neginf), 9.0);
assert_eq!($neginf.max(-9.0), -9.0);
assert_eq!((-9.0).max($neginf), -9.0);
assert_eq!($nan.max(9.0), 9.0);
assert_eq!($nan.max(-9.0), -9.0);
assert_eq!(9.0.max($nan), 9.0);
assert_eq!((-9.0).max($nan), -9.0);
assert!($nan.max($nan).is_nan());
}
} }
}

test_float!(f32, f32, ::core::f32::INFINITY, ::core::f32::NEG_INFINITY, ::core::f32::NAN);
test_float!(f64, f64, ::core::f64::INFINITY, ::core::f64::NEG_INFINITY, ::core::f64::NAN);
6 changes: 2 additions & 4 deletions src/libstd/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ mod cmath {
pub fn erfcf(n: c_float) -> c_float;
pub fn expm1f(n: c_float) -> c_float;
pub fn fdimf(a: c_float, b: c_float) -> c_float;
pub fn fmaxf(a: c_float, b: c_float) -> c_float;
pub fn fminf(a: c_float, b: c_float) -> c_float;
pub fn fmodf(a: c_float, b: c_float) -> c_float;
pub fn ilogbf(n: c_float) -> c_int;
pub fn logbf(n: c_float) -> c_float;
Expand Down Expand Up @@ -673,7 +671,7 @@ impl f32 {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
pub fn max(self, other: f32) -> f32 {
unsafe { cmath::fmaxf(self, other) }
num::Float::max(self, other)
}

/// Returns the minimum of the two numbers.
Expand All @@ -689,7 +687,7 @@ impl f32 {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
pub fn min(self, other: f32) -> f32 {
unsafe { cmath::fminf(self, other) }
num::Float::min(self, other)
}

/// The positive difference of two numbers.
Expand Down
6 changes: 2 additions & 4 deletions src/libstd/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ mod cmath {
pub fn erfc(n: c_double) -> c_double;
pub fn expm1(n: c_double) -> c_double;
pub fn fdim(a: c_double, b: c_double) -> c_double;
pub fn fmax(a: c_double, b: c_double) -> c_double;
pub fn fmin(a: c_double, b: c_double) -> c_double;
pub fn fmod(a: c_double, b: c_double) -> c_double;
pub fn frexp(n: c_double, value: &mut c_int) -> c_double;
pub fn ilogb(n: c_double) -> c_int;
Expand Down Expand Up @@ -587,7 +585,7 @@ impl f64 {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
pub fn max(self, other: f64) -> f64 {
unsafe { cmath::fmax(self, other) }
num::Float::max(self, other)
}

/// Returns the minimum of the two numbers.
Expand All @@ -603,7 +601,7 @@ impl f64 {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
pub fn min(self, other: f64) -> f64 {
unsafe { cmath::fmin(self, other) }
num::Float::min(self, other)
}

/// The positive difference of two numbers.
Expand Down