-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
BUGFIX avoid silent overflow in lcm
least common multiple function
#33926
Conversation
The |
So it would be ok to change |
It's all about context: checking is too expensive when you're doing |
As you can see, I retired the new functions and added the |
widelcm
and checked_lcm
for least common multiplelcm
least common multiple function
test/checked.jl
Outdated
@@ -166,6 +166,7 @@ import Base: checked_abs, checked_neg, checked_add, checked_sub, checked_mul, | |||
@test checked_cld(typemin(T), T(1)) === typemin(T) | |||
@test_throws DivideError checked_cld(typemin(T), T(0)) | |||
@test_throws DivideError checked_cld(typemin(T), T(-1)) | |||
|
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 think you can revert the changes to this file now.
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.
done
Well, that's certainly a much simpler fix now 😁 👍 💯 |
The last change fixes a gotcha in |
is the being forgotten? |
Thanks for the reminder! |
Solves #33911 .
Define new functions
checked_lcm
andwidelcm
analogously tochecked_mul
andwidemul
.The behaviour of
lcm
is not changed, and the user can switch to the appropriate variant at his own discretion.