On Tue, 20 May 2025 15:53:34 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> warning
>
> src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 348:
>
>> 346: */
>> 347: public static int getChars(long i, int index, char[] buf) {
>> 348: // Used by trusted callers. Assumes all necessary bounds
>> checks have been done by the caller.
>
> Hello Shaojin, I think this was a misplaced comment previously. Looking at
> the implementation of this method, there's no "unsafe" access happening in
> this method's implementation. It ends up calling `putChar` which does a Java
> style array access and thus is backed by the language's bounds checking.
>
> Removing this comment I believe is the right thing. Having said that, I am
> unsure the javadoc comment of this method should refer to
> `DecimalDigits#uncheckedGetCharsUTF16` because that is confusing and
> misleading.
>
> Should we change the javadoc text of this method to:
>
>> Places characters representing the long i into the character array buf. The
>> characters are placed into the buffer backwards starting with the least
>> significant digit at the specified index (exclusive), and working backwards
>> from there.
>
> Would that accurately describe what this method's implementation currently
> does?
This method has the same algorithm as uncheckedGetCharsUTF16, the only
difference is the safe array access of char[] and the unsafe access of byte[]
by uncheckedPutPairUTF16.
This method was also copied from uncheckedGetCharsUTF16 and then modified when
the code was written, so I think the reference here is OK.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2098387154
