Skip to content

Ensure that Decimal32/64/128 ToString roundtrips and preserves the cohort - #131422

Open
tannergooding wants to merge 7 commits into
dotnet:mainfrom
tannergooding:tannergooding-decimal-tostring-round-trip
Open

Ensure that Decimal32/64/128 ToString roundtrips and preserves the cohort#131422
tannergooding wants to merge 7 commits into
dotnet:mainfrom
tannergooding:tannergooding-decimal-tostring-round-trip

Conversation

@tannergooding

@tannergooding tannergooding commented Jul 27, 2026

Copy link
Copy Markdown
Member

ToString for Decimal32, Decimal64, and Decimal128 was not producing roundtrippable results and did not preserve the cohort.

The G/R path went through FormatGeneral(..., suppressScientific: true), which unconditionally emits fixed-point. A positive quantum exponent has no fixed-point spelling, so the result reparsed as a different member of the cohort:

value old reparses as new
1e7 10000000 1000000e1 (doesn't fit in 7 digits) 1E+07
10e6 10000000 1000000e1 1.0E+07
0e2 0 0e0 0E+02
Decimal32.MaxValue 9999999 + 90 zeros truncated 9.999999E+96

Zeros were worse -- DecimalIeee754ToNumber clamped number.Scale to 0 for a zero coefficient, so a positive quantum was discarded outright and 0e2 was indistinguishable from 0.

FormatGeneralAndRoundTripDecimalIeee754 now emits scientific notation when it is required (quantum exponent > 0) or when it is more compact (adjusted exponent < -4). The latter is exactly the existing G heuristic for the binary floating-point types (digPos > nMaxDigits || digPos < -3), and the exponent goes through the shared FormatExponent with minDigits: 2 and an explicit sign, so the output is stylistically identical to double/float/Half. IEEE 754 §5.12.2 constrains the syntax and requires the roundtrip to recover the quantum, but says nothing about padding -- that part is purely a .NET consistency call.

Two smaller fixes ride along, both needed for the above to be correct:

  • DecimalIeee754ToNumber no longer clamps Scale for a zero coefficient. Every other consumer resets it -- NumberToString's F/N/E/P/C cases all call RoundNumber first (which sets Scale = 0 when i == 0), and NumberToStringFormat sets it explicitly in its dig[0] == 0 branch.
  • R/r now ignores a precision specifier and G{n} honors it via RoundNumber, matching double.

RoundNumber strips trailing coefficient digits without adjusting Scale, so once a precision specifier has rounded the value, Scale - DigitsCount no longer reports the quantum exponent. G2 on 10.00000 left the buffer as digits 1 / Scale 2, which the required-scientific test misread as a positive quantum:

value format before after double
10.00000 G2 1E+01 10 10
1000.400 G4 1E+03 1000 1000
100.0 G2 1E+02 1E+02 1E+02

The test now compares against the requested precision when rounding occurred, and the fixed-point path recovers the dropped digits as trailing zeros. Thanks @vcsjones for catching this.


The sign of zero was being dropped for every specifier other than G/R. These types shared NumberBufferKind.Decimal with System.Decimal, which has no concept of negative zero, so RoundNumber and NumberToStringFormat cleared IsNegative whenever the rounded result was zero. A signed zero is a distinct IEEE value -- Decimal64.IsNegative(Decimal64.NegativeZero) is true -- so the sign has to survive, exactly as it does for double:

value format before after double
-0 E2 0.00E+000 -0.00E+000 -0.00E+000
-0 F2 0.00 -0.00 -0.00
-0 C2 ¤0.00 (¤0.00) (¤0.00)
-0 0.00 0.00 -0.00 -0.00
-0.001 F2 0.00 -0.00 -0.00

G/R were only correct by accident, because the rewrite above bypasses RoundNumber for a zero coefficient.

This needs a new NumberBufferKind.DecimalIeee754; neither existing kind works, and I confirmed both by building them:

  • Reusing FloatingPoint for the format buffer flips isCorrectlyRounded to true, and ShouldRoundUp then returns false unconditionally, so E/F/N/C/P stop rounding altogether -- 438 divergences from System.Decimal, e.g. 0.999 F0 gives 0 instead of 1. double's buffer arrives pre-rounded from Dragon4, where this is a no-op; the IEEE decimal buffer holds the raw coefficient and genuinely needs to round.
  • Reusing FloatingPoint for the parse buffer discards the quantum of a zero, since only Decimal preserves Scale there -- 0e-101 parses back as 0.

The requirement is a mix of the two, so the new kind is added and the three existing kinds keep byte-identical behavior; every changed condition in shared code only narrows.


Ties were rounding half away from zero rather than to even. IEEE 754 §5.12.1 requires a conversion to a character sequence to be correctly rounded under the applicable rounding-direction attribute, which defaults to roundTiesToEven. This was inherited from System.Decimal, whose away-from-zero behavior is itself a back-compat concession -- the comment in ShouldRoundUp has said since .NET Core 3.0 that the spec dictates ties-to-even.

value format before after double
0.5 F0 1 0 0
2.5 F0 3 2 2
2.500 F0 3 2 2
-2.5 F0 -3 -2 -2
1.25 E1 1.3E+000 1.2E+000 1.2E+000
12.5 G2 13 12 12

It also made ToString disagree with the rest of the same type: Round, quantize, parse, and every arithmetic operation were already ties-to-even, so Decimal64.Round(0.25, 1) gave 0.2 while (0.25).ToString("F1") gave 0.3.

The double custom-format path rounds away from zero because it double-rounds the Dragon4 shortest digits, which is the hazard the existing comment describes. The DecimalIeee754 buffer holds the exact coefficient, so a 5 followed only by zeros is a genuine tie and there is a single correct rounding -- these types are therefore ties-to-even for custom format strings too. That is a deliberate divergence from double, taken because these types are new in .NET 11 and carry no back-compat constraint.


Validation beyond the added tests:

  • Swept 1,006,382 sampled finite Decimal32 encodings, 9,216 explicit (coefficient, quantum) pairs across the full exponent range, and 4,368 Decimal64/Decimal128 values, asserting EncodeDecimal(Parse(ToString(x))) == EncodeDecimal(x) for null/G/R. Zero failures.
  • Differential-tested Decimal64 against System.Decimal across 30 specifiers (E/F/N/P/C and custom formats) and against G{n} rounding: 7,980 plus 749 comparisons, zero divergences other than the intended tie cases above, which System.Decimal rounds away from zero.
  • Differential-tested tie behavior against double over values that are exact in both radices, across 18 specifiers. The only remaining differences are the deliberate custom-format divergence and cohort preservation ((2.500).ToString("G4") is 2.500, not 2.5).
  • 425 checks across 5 cultures covering cohort roundtrip, TryFormat for char and UTF-8 at exact size, and correct false on every truncated destination.
  • Confirmed arithmetic already honors the IEEE preferred exponent (2.00 + 1.000 is 3.000, 1.20 * 1.00 is 1.2000, Sqrt(1.00) is 1.0); 1e3 * 1e3 giving 1E+06 is only observable because of this fix.
  • Confirmed hexadecimal-significand sequences are correctly rejected: all Parse/TryParse overloads route through ValidateParseStyleDecimal, and IEEE 754 §5.12.3 defines that form for binary formats only.

Every regression test was confirmed to fail against the unfixed code. Full System.Runtime.Tests passes (76,591 tests) and System.Runtime.Numerics.Tests passes (8,422 tests) -- the latter because BigInteger compiles the same shared parsing and formatting sources.

CC. @dotnet/area-system-numerics

Note

This PR description was drafted by GitHub Copilot.

…hort

The general and roundtrip specifiers unconditionally suppressed scientific
notation, so a positive quantum exponent had no correct spelling and reparsed
as a different cohort member.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the formatting logic for IEEE-754 decimal floating-point types (Decimal32/Decimal64/Decimal128) so that ToString on the general/round-trip formats preserves the quantum exponent (cohort member) and reliably round-trips through Parse, including for zeros and positive quantum exponents.

Changes:

  • Adjust G/R formatting for decimal IEEE-754 values to emit scientific notation when required (and when more compact), and preserve quantum for zero coefficients.
  • Update DecimalIeee754ToNumber so zero coefficients retain the unbiased exponent in NumberBuffer.Scale.
  • Update Decimal32/Decimal64/Decimal128 tests to reflect the new output and add roundtrip/cohort-preservation coverage.
Show a summary per file
File Description
src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs Implements new general/round-trip formatting logic for IEEE-754 decimal types and preserves quantum for zero coefficients.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal32Tests.cs Updates expected ToString/parse-preservation outputs and adds roundtrip/cohort-preservation theory.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal64Tests.cs Updates expected ToString/parse-preservation outputs and adds roundtrip/cohort-preservation theory.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal128Tests.cs Updates expected ToString/parse-preservation outputs and adds roundtrip/cohort-preservation theory.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 27, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal32Tests.cs Outdated
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal64Tests.cs Outdated
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Decimal128Tests.cs Outdated
@vcsjones
vcsjones self-requested a review July 27, 2026 18:43
Comment thread src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs Outdated
tannergooding and others added 2 commits July 27, 2026 13:11
RoundNumber strips trailing coefficient digits without adjusting Scale, so
Scale - DigitsCount no longer reports the quantum exponent once a precision
specifier has rounded the value. Compare against the requested precision in
that case and recover the dropped digits as trailing zeros.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The IEEE decimal types shared NumberBufferKind.Decimal with System.Decimal,
which has no concept of negative zero, so RoundNumber and NumberToStringFormat
cleared the sign for every specifier other than G and R. A signed zero is a
distinct IEEE value, so give these types their own kind and keep the sign the
way the binary floating-point types do.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 27, 2026 20:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

IEEE 754 requires conversions to a decimal character sequence to be
correctly rounded under the applicable rounding-direction attribute,
which defaults to roundTiesToEven. These types were inheriting the
half-away-from-zero rounding that System.Decimal uses for back-compat,
which also made ToString disagree with Round, quantize, and every
arithmetic operation on the same type.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 27, 2026 21:19
vcsjones
vcsjones previously approved these changes Jul 27, 2026
@vcsjones
vcsjones dismissed their stale review July 27, 2026 21:22

Dismissing because a new commit came in right before I approved it.

@tannergooding

Copy link
Copy Markdown
Member Author

@vcsjones I think the last commit conflicted with your approval and so it instant dismissed. No more changes should be coming, should all be handled now

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 1

Comment thread src/libraries/Common/src/System/Number.NumberBuffer.cs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 27, 2026 21:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 28, 2026 00:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

@tannergooding

Copy link
Copy Markdown
Member Author

CC. @jeffhandley for sign-off to take into RC1. Correctness fix to ensure that ToString on the new Decimal32/64/128 types correctly roundtrips in all edge cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants