-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement conversion traits for primitive float types #29129
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1473,14 +1473,14 @@ impl fmt::Display for ParseIntError { | |
|
||
pub use num::dec2flt::ParseFloatError; | ||
|
||
// Conversion traits for primitive integer types | ||
// Conversion traits for primitive integer and float types | ||
// Conversions T -> T are covered by a blanket impl and therefore excluded | ||
// Some conversions from and to usize/isize are not implemented due to portability concerns | ||
macro_rules! impl_from { | ||
($Small: ty, $Large: ty) => { | ||
#[stable(feature = "lossless_int_conv", since = "1.5.0")] | ||
#[stable(feature = "lossless_prim_conv", since = "1.5.0")] | ||
impl From<$Small> for $Large { | ||
#[stable(feature = "lossless_int_conv", since = "1.5.0")] | ||
#[stable(feature = "lossless_prim_conv", since = "1.5.0")] | ||
#[inline] | ||
fn from(small: $Small) -> $Large { | ||
small as $Large | ||
|
@@ -1514,3 +1514,24 @@ impl_from! { u8, i64 } | |
impl_from! { u16, i32 } | ||
impl_from! { u16, i64 } | ||
impl_from! { u32, i64 } | ||
|
||
// Note: integers can only be represented with full precision in a float if | ||
// they fit in the significand, which is 24 bits in f32 and 53 bits in f64. | ||
// Lossy float conversions are not implemented at this time. | ||
|
||
// Signed -> Float | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment about the selection of types here, for future reference? (This is totally something I could imagine someone looking at and wondering about, especially since the details of floating point aren't in the front of everyone's head all the time.) I.e. mention something about the precision (24 and 53 bits respectively) meaning these types being the only integers which can be represented losslessly in the float types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I just pushed a new comment for this. |
||
impl_from! { i8, f32 } | ||
impl_from! { i8, f64 } | ||
impl_from! { i16, f32 } | ||
impl_from! { i16, f64 } | ||
impl_from! { i32, f64 } | ||
|
||
// Unsigned -> Float | ||
impl_from! { u8, f32 } | ||
impl_from! { u8, f64 } | ||
impl_from! { u16, f32 } | ||
impl_from! { u16, f64 } | ||
impl_from! { u32, f64 } | ||
|
||
// Float -> Float | ||
impl_from! { f32, f64 } |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,4 +177,61 @@ mod tests { | |
test_impl_from! { test_u16i32, u16, i32 } | ||
test_impl_from! { test_u16i64, u16, i64 } | ||
test_impl_from! { test_u32i64, u32, i64 } | ||
|
||
// Signed -> Float | ||
test_impl_from! { test_i8f32, i8, f32 } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say conversions integer -> float feel... too suspicious for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll elaborate. I have a criterion for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for elaborating. I don't have a specific example already where this would be useful, but it's not hard to concoct one. Say But I realize this is a pretty subjective thing to judge. Is there anyone else we should ping for an opinion about int->float? Do you at least agree with f32->f64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@rust-lang/libs ?
Yes! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My opinion carries no more weight than yours. :) I appreciate your input! |
||
test_impl_from! { test_i8f64, i8, f64 } | ||
test_impl_from! { test_i16f32, i16, f32 } | ||
test_impl_from! { test_i16f64, i16, f64 } | ||
test_impl_from! { test_i32f64, i32, f64 } | ||
|
||
// Unsigned -> Float | ||
test_impl_from! { test_u8f32, u8, f32 } | ||
test_impl_from! { test_u8f64, u8, f64 } | ||
test_impl_from! { test_u16f32, u16, f32 } | ||
test_impl_from! { test_u16f64, u16, f64 } | ||
test_impl_from! { test_u32f64, u32, f64 } | ||
|
||
// Float -> Float | ||
#[test] | ||
fn test_f32f64() { | ||
use core::f32; | ||
|
||
let max: f64 = f32::MAX.into(); | ||
assert_eq!(max as f32, f32::MAX); | ||
assert!(max.is_normal()); | ||
|
||
let min: f64 = f32::MIN.into(); | ||
assert_eq!(min as f32, f32::MIN); | ||
assert!(min.is_normal()); | ||
|
||
let min_positive: f64 = f32::MIN_POSITIVE.into(); | ||
assert_eq!(min_positive as f32, f32::MIN_POSITIVE); | ||
assert!(min_positive.is_normal()); | ||
|
||
let epsilon: f64 = f32::EPSILON.into(); | ||
assert_eq!(epsilon as f32, f32::EPSILON); | ||
assert!(epsilon.is_normal()); | ||
|
||
let zero: f64 = (0.0f32).into(); | ||
assert_eq!(zero as f32, 0.0f32); | ||
assert!(zero.is_sign_positive()); | ||
|
||
let neg_zero: f64 = (-0.0f32).into(); | ||
assert_eq!(neg_zero as f32, -0.0f32); | ||
assert!(neg_zero.is_sign_negative()); | ||
|
||
let infinity: f64 = f32::INFINITY.into(); | ||
assert_eq!(infinity as f32, f32::INFINITY); | ||
assert!(infinity.is_infinite()); | ||
assert!(infinity.is_sign_positive()); | ||
|
||
let neg_infinity: f64 = f32::NEG_INFINITY.into(); | ||
assert_eq!(neg_infinity as f32, f32::NEG_INFINITY); | ||
assert!(neg_infinity.is_infinite()); | ||
assert!(neg_infinity.is_sign_negative()); | ||
|
||
let nan: f64 = f32::NAN.into(); | ||
assert!(nan.is_nan()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if this isn't merged before 1.5 branches, I'll need a distinct macro and stable tag, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this will technically land during 1.6, I think these are fine here. We don't actually read these
since
versions anywhere, and it'd just be a pain to separate out this macro for 1.5 stable and 1.6 stable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this PR is incorrectly listed in RELEASES.md under 1.5, possibly due to this confusion between the feature tag and the time of the actual merge. (I was wondering why my PR was listed but I wasn't in the contributor list...)
@brson -- need to tag this somehow so you can remember it for 1.6? It seems like "relnotes" needs to be version-specific.