-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix float -> u128 conversion bug #14
Conversation
See also rust-lang/rust#41799 |
Converting floats to u128 failed because in the comparison "src > u128::MAX as f32" we invoked UB due to the u128::MAX as f32 conversion.
r? @japaric and of course, I'll need a new version |
@@ -302,6 +302,37 @@ macro_rules! from_float { | |||
} | |||
} | |||
|
|||
/// From a float `$src` to an integer `$dst`, where $dst is large enough to contain | |||
/// all values of `$src`. We can't ever overflow here |
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.
We can't ever overflow here
Overflow can happen when going from f32 to i128
f32::MAX = 340282350000000000000000000000000000000
i128::MAX = 170141183460469231731687303715884105727
} else if src == $src::INFINITY || | ||
src == $src::NEG_INFINITY { | ||
Error::Infinite | ||
} else if ($dst::MIN == 0) && src < 0.0 { |
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.
Underflow can happen when going from f32 to i128
f32::MIN = -340282350000000000000000000000000000000
i128::MIN = -170141183460469231731687303715884105728
re-r? @japaric |
@homunkulus r+ |
📌 Commit 868978e has been approved by |
💔 Test failed - status-travis |
@homunkulus r+ |
📌 Commit d492df1 has been approved by |
☀️ Test successful - status-travis |
Converting floats to u128 failed because in the comparison
"src > u128::MAX as f32" we invoked UB due to the
u128::MAX as f32 conversion.