RFR: 8343829: Unify decimal and hexadecimal parsing in FloatingDecimal [v9]
Raffaello Giulietti
rgiulietti at openjdk.org
Sat May 10 10:02:59 UTC 2025
On Fri, 9 May 2025 22:58:19 GMT, Chen Liang <liach at openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision:
>>
>> - Removed useless comment.
>> - Merge branch 'master' into 8343829
>> - Added javadoc to refer to the grammar in j.l.Double.
>> - Merge branch 'master' into 8343829
>> - Make some static arrays @Stable.
>> - Remove unused BIG_DECIMAL_EXPONENT
>> - Merge branch 'master' into 8343829
>> - Merge branch 'master' into 8343829
>> - Redacted comments.
>> - Merge branch 'master' into 8343829
>> - ... and 4 more: https://git.openjdk.org/jdk/compare/8862cc91...88fe2c08
>
> src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 1964:
>
>> 1962:
>> 1963: /* Skip opt [FfDd]? suffix. */
>> 1964: if (i < len && (((ch = in.charAt(i) | 0b10_0000)) == 'f' || ch == 'd')) {
>
> Is it really right to ignore these suffix for all ix values?
Although this might seem surprising, yes, it is right.
This is what the `[Double|Float].valueOf(String)` specs say (these are the "main clients" of this class):
> Note that trailing format specifiers, [...] do not influence the results of this method.
> src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 2006:
>
>> 2004: *
>> 2005: * |lz |pt |tnz |stop
>> 2006: * 0000000012345600000023.4567000000000
>
> Suggestion:
>
> * |lz |pt |tnz |stop
> * 0000000012345600000023.4567000000000
Addressed.
> src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 2265:
>
>> 2263:
>> 2264: /* Arithmetically "appends the digit" ch to v >= 0, clamping at 10^10. */
>> 2265: private static long appendDigit(long v, int ch) {
>
> Since this appends only decimal digit, should we name it `appendDecDegit`? In the use site we already note all exponents are decimals.
Addressed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22737#discussion_r2083096389
PR Review Comment: https://git.openjdk.org/jdk/pull/22737#discussion_r2083096060
PR Review Comment: https://git.openjdk.org/jdk/pull/22737#discussion_r2083096070
More information about the core-libs-dev
mailing list