On Fri, 3 May 2024 08:47:03 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR which corrects an edge case bug for
> java.text.DecimalFormat that causes incorrect parsing results for strings
> with very large exponent values.
>
> When parsing values with large exponents, if the value of the exponent
> exceeds `Integer.MAX_VALUE`, the parsed value is equal to 0. If the value of
> the exponent exceeds `Long.MAX_VALUE`, the parsed value is equal to the
> mantissa. Both results are confusing and incorrect.
>
> For example,
>
>
> NumberFormat fmt = NumberFormat.getInstance(Locale.US);
> fmt.parse(".1E2147483648"); // returns 0.0
> fmt.parse(".1E9223372036854775808"); // returns 0.1
> // For comparison
> Double.parseDouble(".1E2147483648"); // returns Infinity
> Double.parseDouble(".1E9223372036854775808"); // returns Infinity
>
>
> After this change, both parse calls return `Double.POSITIVE_INFINITY` now.
Looks good. Left some minor comments.
src/java.base/share/classes/java/text/DecimalFormat.java line 2590:
> 2588:
> 2589: if (subparse(text, pos, "",
> symbols.getMinusSignText(), exponentDigits, true, stat)) {
> 2590: // We parse the exponent with isExponent true,
> thus fitsIntoLong
Nit: `==` or `being` between `isExponent` and `true` may read better.
src/java.base/share/classes/java/text/DecimalFormat.java line 2596:
> 2594: exponent = -exponent;
> 2595: }
> 2596: sawExponent = true;
I see you removed this assignment. I am wondering if we need this variable at
all.
test/jdk/java/text/Format/DecimalFormat/LargeExponentsTest.java line 128:
> 126: // Trailing zeroes
> 127: Arguments.of("1.23E0000123", 1.23E123),
> 128: // Trailing zeroes - Past Long.MAX_VALUE length
Leading zeroes?
-------------
PR Review: https://git.openjdk.org/jdk/pull/19075#pullrequestreview-2038744656
PR Review Comment: https://git.openjdk.org/jdk/pull/19075#discussion_r1589560835
PR Review Comment: https://git.openjdk.org/jdk/pull/19075#discussion_r1589558570
PR Review Comment: https://git.openjdk.org/jdk/pull/19075#discussion_r1589573332
