Fix invalid casts in is/2 #2777
Open
+711
−27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes mthom#2772. The current implementation of `rnd_i` incorrectly casts `f` (an `f64`) into an `i64`, before casting it into an `Integer`. This fixes that issue by using `Integer::try_from(f)` instead.
|
(might as well fix |
adri326
commented
Jan 17, 2025
adri326
commented
Jan 17, 2025
The original issue can be reproduced with `X is round(2 ^ 54 + 1) - 2 ^ 54, X = 1.`
adri326
force-pushed
the
fix-2772-rnd_i-clipping
branch
from
508e901 to
a739390
Compare
|
Thank you a lot, this is ideal: With this I can address #2771 entirely in Prolog, based on solid underlying functionality in the core! |
The implementation for gcd/2 would cast the second argument to an isize.
adri326
changed the title
It now behaves the same way as SWI-Prolog.
This extensively tests the behavior of is/2, both when compiled and in metacalls.
adri326
force-pushed
the
fix-2772-rnd_i-clipping
branch
from
1afa051 to
3b6c209
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #2772 and a few of other issues I found:
rnd_iwould first convert big floats into an i64, before converting them into anIBiground/1would add0.5to big integers, losing precision beyond±2 ^ 53gcd/2would attempt to cast its second argument to anisizebefore computing the gcd usingIBig::gcd, triggering apanic!()min/2andmax/2would return the float version of their arguments if their types didn't matchThis PR is accompanied by a test suite (that @UWN might be interested in) for the behavior of
is/2, both after being compiled and being metacalled. Implementation-specific behavior (like bitshifting negative numbers) is excluded from this test suite.I ran the integration tests with
cargo-tarpaulinto get a report and help me find uncovered edge cases. I chose not to test special cases likepow(integer, integer), since it's unclear to me whether or not smallNumber::Integershould be returned from computations (likeX is 2 ^ 64 - 2 ^ 64).Each of these changes (including the addition of the new test suite) were placed in their own commits.