factor in lossless representation in integer to float conversion check

closes #23
japaric · Sep 4, 2021 · c618d87 · c618d87
1 parent 67aa147
commit c618d87
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 4 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -281,10 +281,19 @@ macro_rules! from_float {
                             src == $src::NEG_INFINITY {
                             Error::Infinite
                         } else if {
-                            // we subtract 1 ULP (unit of least precision) here because some
-                            // lossy conversions like `u64::MAX as f64` round *up* and we want
-                            // to avoid this evaluating to false in that case
-                            let max = $src::from_bits(($dst::MAX as $src).to_bits() - 1);
+                            // this '$dst::BITS' works on 1.31.0 (MSRV)
+                            let dst_bits = core::mem::size_of::<$dst>() as u32 * 8;
+                            let lossless = dst_bits < core::$src::MANTISSA_DIGITS;
+
+                            let max = if lossless {
+                                $dst::MAX as $src
+                            } else {
+                                // we subtract 1 ULP (unit of least precision) here because some
+                                // lossy conversions like `u64::MAX as f64` round *up* and we want
+                                // to avoid the check below evaluating to false in that case
+                                $src::from_bits(($dst::MAX as $src).to_bits() - 1)
+                            };
+
                             src > max
                         } {
                             Error::Overflow

diff --git a/src/test.rs b/src/test.rs
@@ -193,3 +193,23 @@ fn gh15() {
     assert_eq!(super::u16(16_f32.exp2()), Err(super::Error::Overflow));
     assert_eq!(super::u16(16_f64.exp2()), Err(super::Error::Overflow));
 }
+
+#[test]
+fn gh23_lossless_integer_max_min_to_float() {
+    // f32::MANTISSA_DIGITS = 24
+    assert_eq!(Ok(u8::MAX), super::u8(255f32));
+    assert_eq!(Ok(u16::MAX), super::u16(65_535f32));
+
+    // f64::MANTISSA_DIGITS = 53
+    assert_eq!(Ok(u8::MAX), super::u8(255f64));
+    assert_eq!(Ok(u16::MAX), super::u16(65_535f64));
+    assert_eq!(Ok(u32::MAX), super::u32(4_294_967_295f64));
+
+    // also check negative values (not part of the original bug)
+    assert_eq!(Ok(i8::MIN), super::i8(-128f32));
+    assert_eq!(Ok(i16::MIN), super::i16(-32_768f32));
+
+    assert_eq!(Ok(i8::MIN), super::i8(-128f64));
+    assert_eq!(Ok(i16::MIN), super::i16(-32_768f64));
+    assert_eq!(Ok(i32::MIN), super::i32(-2_147_483_648f64));
+}