On Wed, 11 Jun 2025 23:47:01 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Johannes Graham has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review comments
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2143060934
