Fix infinite loop in to_integer/1 for zero coefficient#229
Merged
Conversation
When the coefficient is zero and the exponent is negative (e.g.
Decimal.new("0.0")), strip_trailing_zeros_one/2 recursed indefinitely
because rem(0, 10) == 0 always matched and div(0, 10) == 0. Add a base
case so such values normalize to {0, 0} and to_integer/1 returns the
integer 0, matching the existing convention where to_integer(~d"1.00")
returns 1.
Closes #228
do_normalize_one/2 has the same recursive shape as the previously fixed strip_trailing_zeros_one/2 and would loop forever if reached with a zero coefficient. All current callers (normalize/1, canonical_xsd/1, xsd_digit_count/2) short-circuit coef == 0 before recursing, so the path is unreachable today, but a future caller missing that guard would hang. Add a base case so the helper terminates with the canonical zero. Add a regression test exercising normalize/1 with a manually constructed zero-coefficient decimal at non-zero exponents.
Generates random integers paired with random non-negative shifts to exercise the strip_trailing_zeros code path. Covers the previously buggy zero-coefficient case (n=0 with k>0 produces coef=0, exp=-k) alongside general regression coverage for the trailing-zero stripping logic across the chunk and one-by-one paths.
Introduce a DecimalGenerators helper in test_helper.exs that yields
finite, in-bounds decimals (and non-zero / non-negative variants) so
properties can stay clear of NaN/inf and decimal128 overflow signals
without each test rebuilding its own generator.
Add 21 new properties covering:
* algebraic identities for add/2, sub/2, mult/2, negate/1, abs/1
(commutativity, identity, inverse, sign cancellation)
* comparison-predicate agreement with compare/2 for gt?/2, lt?/2,
gte?/2, lte?/2, eq?/2, equal?/2
* min/2 and max/2 ordering invariants
* normalize/1 idempotence and value preservation
* positive?/1 and negative?/1 agreement with compare against zero
* to_string(:scientific) <-> parse round-trip
* Decimal.new(integer) <-> to_integer round-trip
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
When the coefficient is zero and the exponent is negative (e.g. Decimal.new("0.0")), strip_trailing_zeros_one/2 recursed indefinitely because rem(0, 10) == 0 always matched and div(0, 10) == 0. Add a base case so such values normalize to {0, 0} and to_integer/1 returns the integer 0, matching the existing convention where to_integer(~d"1.00") returns 1.
Closes #228