From 3562ce18074d7f7b98e94d0fd365ec21cdef87ec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 12:32:08 +0000 Subject: [PATCH] fix: do the formatting length arithmetic in long [patch] ToString and TryFormat sized their buffers with int arithmetic over the exponent, so a value whose exponent sat near either end of the int range threw rather than formatted: int.Abs(int.MinValue) overflows, and the sums near int.MaxValue wrapped negative and reached ArrayPool.Rent and Span.Slice as invalid lengths. Parse accepts any exponent an int holds, so a value could parse and then fail to format. Widen the lengths to long, the way #72 did for the conversion paths. TryFormat now answers false for a destination that cannot hold the rendering, whatever the exponent, and ToString throws an OverflowException naming the exponent when the text could not exist as a string at all. CountDecimalDigits returns long for the same reason: an exponent of int.MinValue implies one more decimal digit than an int can count. Round follows it, and stops raising ten to a power wider than the significand, since dropping one digit more than it holds always leaves zero. Formatting stays fixed point. Scientific notation for extreme exponents is a separate API decision and is left to the issue. Text benchmarks are unchanged within noise across all three digit counts, with allocations byte for byte identical. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XbHTU3pZh6egvtowTyuqNM --- PreciseNumber.Test/PreciseNumberTests.cs | 85 +++++++++++++++++++++++- PreciseNumber/PreciseNumber.cs | 73 +++++++++++++------- 2 files changed, 131 insertions(+), 27 deletions(-) diff --git a/PreciseNumber.Test/PreciseNumberTests.cs b/PreciseNumber.Test/PreciseNumberTests.cs index ef2a039..f5cf994 100644 --- a/PreciseNumber.Test/PreciseNumberTests.cs +++ b/PreciseNumber.Test/PreciseNumberTests.cs @@ -947,8 +947,8 @@ public void TestLowestDecimalDigits() { PreciseNumber number1 = PreciseNumber.CreateFromComponents(-2, 12345); PreciseNumber number2 = PreciseNumber.CreateFromComponents(-3, 678); - int result = PreciseNumber.LowestDecimalDigits(number1, number2); - Assert.AreEqual(2, result); + long result = PreciseNumber.LowestDecimalDigits(number1, number2); + Assert.AreEqual(2L, result); } [TestMethod] @@ -2300,4 +2300,85 @@ public void TestParseRoundTripsLongDecimals() Assert.AreEqual(text, parsed.ToString(CultureInfo.InvariantCulture)); } + + [TestMethod] + public void TestCountDecimalDigitsAtExtremeExponents() + { + PreciseNumber tiny = PreciseNumber.CreateFromComponents(int.MinValue, BigInteger.One); + Assert.AreEqual(-(long)int.MinValue, tiny.CountDecimalDigits()); + + PreciseNumber huge = PreciseNumber.CreateFromComponents(int.MaxValue, BigInteger.One); + Assert.AreEqual(0L, huge.CountDecimalDigits()); + } + + [TestMethod] + public void TestTryFormatAtExtremeExponentsReturnsFalse() + { + // The text these need is longer than any int, so the only correct answer for a + // destination this size is false, not an exception. + Span buffer = stackalloc char[64]; + foreach (int exponent in new[] { int.MinValue, int.MaxValue }) + { + PreciseNumber number = PreciseNumber.CreateFromComponents(exponent, BigInteger.One); + + bool result = number.TryFormat(buffer, out int charsWritten, "G".AsSpan(), CultureInfo.InvariantCulture); + + Assert.IsFalse(result, $"TryFormat should fail for an exponent of {exponent}"); + Assert.AreEqual(0, charsWritten); + } + } + + [TestMethod] + public void TestToStringAtExtremeExponentsThrowsOverflow() + { + // Neither value can exist as a string in fixed point notation, so the failure should say + // so rather than surface as an allocation or negation error. + foreach (int exponent in new[] { int.MinValue, int.MaxValue }) + { + PreciseNumber number = PreciseNumber.CreateFromComponents(exponent, BigInteger.One); + + Assert.ThrowsExactly( + () => number.ToString(CultureInfo.InvariantCulture), + $"ToString should overflow for an exponent of {exponent}"); + } + } + + [TestMethod] + public void TestParseThenFormatAtExtremeExponentsRoundTrips() + { + // Parse accepts any exponent that fits an int, so formatting has to answer for one. + PreciseNumber tiny = PreciseNumber.Parse("1E-2147483648", NumberStyles.Float, CultureInfo.InvariantCulture); + Assert.AreEqual(int.MinValue, tiny.Exponent); + Assert.ThrowsExactly(() => tiny.ToString(CultureInfo.InvariantCulture)); + + PreciseNumber huge = PreciseNumber.Parse("1E2147483647", NumberStyles.Float, CultureInfo.InvariantCulture); + Assert.AreEqual(int.MaxValue, huge.Exponent); + Assert.ThrowsExactly(() => huge.ToString(CultureInfo.InvariantCulture)); + } + + [TestMethod] + public void TestRoundAtExtremeNegativeExponentGivesZero() + { + // Every significant digit sits far below the requested place, so the value rounds away. + PreciseNumber tiny = PreciseNumber.CreateFromComponents(int.MinValue, BigInteger.One); + + Assert.AreEqual(PreciseNumber.Zero, tiny.Round(2)); + } + + [TestMethod] + public void TestFormatAtLargeButRepresentableExponents() + { + // Still fixed point, and still the full run of zeros, but a length a string can hold. + PreciseNumber tiny = PreciseNumber.CreateFromComponents(-1000, BigInteger.One); + string tinyText = tiny.ToString(CultureInfo.InvariantCulture); + Assert.AreEqual(1002, tinyText.Length); + Assert.AreEqual("0.", tinyText[..2]); + Assert.AreEqual('1', tinyText[^1]); + + PreciseNumber huge = PreciseNumber.CreateFromComponents(1000, BigInteger.One); + string hugeText = huge.ToString(CultureInfo.InvariantCulture); + Assert.AreEqual(1001, hugeText.Length); + Assert.AreEqual('1', hugeText[0]); + Assert.AreEqual(new string('0', 1000), hugeText[1..]); + } } diff --git a/PreciseNumber/PreciseNumber.cs b/PreciseNumber/PreciseNumber.cs index 5a925b8..78c9f9e 100644 --- a/PreciseNumber/PreciseNumber.cs +++ b/PreciseNumber/PreciseNumber.cs @@ -330,14 +330,21 @@ public static string ToString(PreciseNumber number, string? format, IFormatProvi NumberFormatInfo numberFormat = NumberFormatInfo.GetInstance(formatProvider ?? InvariantCulture); // Digits, plus the padding zeros implied by the exponent, plus the sign, the decimal - // separator and a possible leading "0". - int desiredAlloc = number.SignificantDigits - + int.Abs(number.Exponent) + // separator and a possible leading "0". Widened, because an exponent of int.MinValue has + // no negation that fits an int and one near int.MaxValue would wrap the sum. + long desiredAlloc = number.SignificantDigits + + long.Abs(number.Exponent) + numberFormat.NegativeSign.Length + numberFormat.NumberDecimalSeparator.Length + 1; - char[]? rentedBuffer = desiredAlloc > MaxStackAllocChars ? ArrayPool.Shared.Rent(desiredAlloc) : null; + if (desiredAlloc > Array.MaxLength) + { + throw new OverflowException( + $"An exponent of {number.Exponent.ToString(InvariantCulture)} needs more characters in fixed point notation than a string can hold."); + } + + char[]? rentedBuffer = desiredAlloc > MaxStackAllocChars ? ArrayPool.Shared.Rent((int)desiredAlloc) : null; Span stackBuffer = stackalloc char[MaxStackAllocChars]; Span buffer = rentedBuffer is null ? stackBuffer : rentedBuffer.AsSpan(); @@ -373,12 +380,18 @@ public static string ToString(PreciseNumber number, string? format, IFormatProvi /// Rounds half away from zero, so 1.235 becomes 1.24 and 1.2349 becomes 1.23. public PreciseNumber Round(int decimalDigits) { - int currentDecimalDigits = CountDecimalDigits(); - int decimalDifference = int.Abs(decimalDigits - currentDecimalDigits); + long currentDecimalDigits = CountDecimalDigits(); + long decimalDifference = long.Abs(decimalDigits - currentDecimalDigits); if (currentDecimalDigits > decimalDigits && decimalDifference > 0) { - BigInteger newSignificand = DropDigitsRoundingHalfAwayFromZero(Significand, decimalDifference); - int newExponent = Exponent - int.CopySign(decimalDifference, Exponent); + // Dropping one digit more than the significand holds always leaves zero, so there is + // never a reason to raise ten to a wider power than that, however far below the + // requested place the value sits. + int droppedDigits = (int)long.Min(decimalDifference, SignificantDigits + 1); + BigInteger newSignificand = DropDigitsRoundingHalfAwayFromZero(Significand, droppedDigits); + int newExponent = newSignificand.IsZero + ? 0 + : Exponent - int.CopySign(droppedDigits, Exponent); return new PreciseNumber(newExponent, newSignificand); } @@ -615,10 +628,10 @@ private static BigInteger DropDigitsRoundingHalfAwayFromZero(BigInteger signific /// The first number. /// The second number. /// The lower of the decimal digit counts of the two numbers. - internal static int LowestDecimalDigits(PreciseNumber left, PreciseNumber right) + internal static long LowestDecimalDigits(PreciseNumber left, PreciseNumber right) { - int leftDecimalDigits = left.CountDecimalDigits(); - int rightDecimalDigits = right.CountDecimalDigits(); + long leftDecimalDigits = left.CountDecimalDigits(); + long rightDecimalDigits = right.CountDecimalDigits(); leftDecimalDigits = left.HasInfinitePrecision ? rightDecimalDigits : leftDecimalDigits; rightDecimalDigits = right.HasInfinitePrecision ? leftDecimalDigits : rightDecimalDigits; @@ -651,10 +664,14 @@ internal static int LowestSignificantDigits(PreciseNumber left, PreciseNumber ri /// Counts the number of decimal digits in the current instance. /// /// The number of decimal digits in the current instance. - internal int CountDecimalDigits() => + /// + /// Widened, because an exponent of implies one more decimal digit + /// than an can count. + /// + internal long CountDecimalDigits() => Exponent > 0 ? 0 - : int.Abs(Exponent); + : -(long)Exponent; /// /// Reduces the significance of the current instance to a specified number of significant digits. @@ -1114,9 +1131,12 @@ private bool TryWriteDigits(Span destination, ReadOnlySpan digits, N sign = numberFormat.NegativeSign; } + // Every length below is widened, because the padding zeros an extreme exponent implies can + // outnumber anything an int holds. Past that point no destination is large enough, so the + // comparison against its length is what answers, rather than an overflow. if (Exponent >= 0) { - int wholeLength = sign.Length + digits.Length + Exponent; + long wholeLength = (long)sign.Length + digits.Length + Exponent; if (destination.Length < wholeLength) { return false; @@ -1125,47 +1145,50 @@ private bool TryWriteDigits(Span destination, ReadOnlySpan digits, N sign.CopyTo(destination); digits.CopyTo(destination[sign.Length..]); destination.Slice(sign.Length + digits.Length, Exponent).Fill('0'); - charsWritten = wholeLength; + charsWritten = (int)wholeLength; return true; } ReadOnlySpan separator = numberFormat.NumberDecimalSeparator; - int fractionalDigits = -Exponent; - int integralDigits = digits.Length - fractionalDigits; + long fractionalDigits = -(long)Exponent; + long integralDigits = digits.Length - fractionalDigits; // When the exponent consumes every digit the integral part is a single "0" and the // fractional part is padded out to the full width with leading zeros. - int integralLength = integralDigits > 0 ? integralDigits : 1; - int required = sign.Length + integralLength + separator.Length + fractionalDigits; + long integralLength = integralDigits > 0 ? integralDigits : 1; + long required = sign.Length + integralLength + separator.Length + fractionalDigits; if (destination.Length < required) { return false; } + // The destination holds the whole rendering, so every length below fits an int. int position = 0; sign.CopyTo(destination); position += sign.Length; if (integralDigits > 0) { - digits[..integralDigits].CopyTo(destination[position..]); - position += integralDigits; + int wholeDigits = (int)integralDigits; + digits[..wholeDigits].CopyTo(destination[position..]); + position += wholeDigits; separator.CopyTo(destination[position..]); position += separator.Length; - digits[integralDigits..].CopyTo(destination[position..]); + digits[wholeDigits..].CopyTo(destination[position..]); } else { + int padding = (int)(fractionalDigits - digits.Length); destination[position++] = '0'; separator.CopyTo(destination[position..]); position += separator.Length; - destination.Slice(position, fractionalDigits - digits.Length).Fill('0'); - position += fractionalDigits - digits.Length; + destination.Slice(position, padding).Fill('0'); + position += padding; digits.CopyTo(destination[position..]); } - charsWritten = required; + charsWritten = (int)required; return true; }