On Tue, 7 Oct 2025 23:19:28 GMT, Justin Lu <[email protected]> wrote: > Please review this PR which corrects a rounding error for DecimalFormat on > tie cases when the maximum fraction digits allowed is one less than the > position of the first significant digit in the double rounded. For example, > rounding a value such as 0.0005 to a maximum of 3 fractional digits. In order > to get the correct `count` value, we must strip the trailing zeros prior to > the check to ensure we have the correct value of `count` = 1 so that we can > go down the _Rounding position is the last index_ case which properly checks > if the String double was already rounded or is exact from its true decimal > form.
For some history, this bug is long-standing, but has taken different forms after recent changes to DigitList. DigitList, when handling the specific case as mentioned in the summary statement returns a faulty value due to an incorrect value of `count`. DigitList used to parse its own `count` value, but was recently changed in [JDK-8367324](https://bugs.openjdk.org/browse/JDK-8367324) to retrieve `count`from `FloatingDecimal`. Consider the double `0.00005` rounded to 4 fractional digits, `count` prior to JDK-8367324 would be calculated as 2 while it is now calculated as 17 due to the implementation differences. Thus, when handling the following case, } else if (-decimalAt == maximumDigits) { // If we round 0.0009 to 3 fractional digits, then we have to // create a new one digit in the least significant location. if (shouldRoundUp(0, roundedUp, valueExactAsDecimal)) { count = 1; ++decimalAt; digits[0] = '1'; } else { For both `count` values, `if (maximumDigits == (count - 1))` is erroneously evaluated in `shouldRoundUp` under the HALF_* cases and instead falls under the _Rounding position is not the last index_ case. Notice that before JDK-8367324, `count` was still being read **incorrectly** as 2 (for fractional doubles in exponential form). However, the behavioral differences had not been observed, since the HALF_UP/DOWN cases prior to https://bugs.openjdk.org/browse/JDK-8314169 could return the right values by chance when going down the wrong path. test/jdk/java/text/Format/DecimalFormat/RoundingToLSDTieTest.java line 1: > 1: /* I considered adding the tests in this file under _TieRoundingTest_, but did not do so to make review easier since JUnit could be used for the test in this PR. (_TieRoundingTest_ is old and does not use JUnit.) Will consider updating _TieRoundingTest_ to use JUnit and merging the tests in this PR under it in a separate issue. test/jdk/java/text/Format/NumberFormat/NumberRegression.java line 1779: > 1777: "0%", "0%", "1%", "1%", "1%", > 1778: "0", "2", "0.2", "0.6", "0.04", > 1779: "0.04", "0.001", "0.002", This test is checking that `.0005` rounded to 3 digits is "0.000" under HALF_EVEN. This is wrong. See, `new BigDecimal(0.0005) ==> 0.0005000000000000000104083408558608425664715468883514404296875`. Resolving the bug in this PR requires fixing this incorrect test case. From the JDK-4241880 issue, it seems this case may have been put in to "document the incorrect but actual behavior". ------------- PR Comment: https://git.openjdk.org/jdk/pull/27681#issuecomment-3379027299 PR Review Comment: https://git.openjdk.org/jdk/pull/27681#discussion_r2412168905 PR Review Comment: https://git.openjdk.org/jdk/pull/27681#discussion_r2412167812
