On Thu, 12 Jun 2025 15:26:55 GMT, Johannes Graham <[email protected]> wrote:
>> src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 1841:
>>
>>> 1839:
>>> 1840: static ASCIIToBinaryConverter readDoubleSignlessDigits(int
>>> decExp, char[] digits, int length) {
>>> 1841: if (decExp < MIN_DECIMAL_EXPONENT) {
>>
>> Is this check needed? I think `ASCIIToBinaryConverter` will return the
>> proper zero value when `doubleValue()` is invoked.
>>
>>
>> if (decExponent < MIN_DECIMAL_EXPONENT - 1) {
>> return (isNegative) ? -0.0 : 0.0;
>>
>>
>> And if this explicit check is a shortcut, I don't think we would need one
>> for an edge case.
>
> Unfortunately some check is required (a test fails), but I now realize what I
> had was wrong. The issue is that on line 1084
> (https://github.com/openjdk/jdk/pull/25644/files#diff-79e6fd549b5ec5e7f49658581beddcb07fcbb0c09ae8e1117c385b66514da6d2R1084))
> exp can overflow and become positive again. I've updated the check to avoid
> the overflow.
Ah got it, I see your point. We would have goten underflow in
`ASCIIToBinaryConverter.doubleValue()` for some extreme cases without a check.
Is there a specific example you have that requires the switch to the newer
check? Adding a comment along those lines might be helpful. Actually, I thought
DigitList caps `decimalAt` to Integer.MIN/MAX, so then the first check you had
would have been fine. (Maybe I am missing something?)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2143781479
