On Thu, 12 Feb 2026 16:11:25 GMT, Volkan Yazici <[email protected]> wrote:
>> Moves input validation checks to Java for `java.lang.StringLatin1`
>> intrinsics. While doing so, affected `java.lang.StringUTF16` methods needed
>> to be updated due to relaxed checks at intrinsics. The javadocs of the
>> touched methods are extensively overhauled.
>
> Volkan Yazici has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Apply review feedback
src/java.base/share/classes/java/lang/StringUTF16.java line 407:
> 405: * @param srcOff the index (inclusive) of the first character in
> {@code src}
> 406: * @param dst the target Latin-1 string byte array
> 407: * @param srcOff the index (inclusive) of the first character in
> {@code dst}
Suggestion:
* @param dstOff the index (inclusive) of the first character in {@code dst}
src/java.base/share/classes/java/lang/StringUTF16.java line 437:
> 435:
> 436: /**
> 437: * Copies the prefix of Latin-1 characters from a UTF-16 string byte
> array
Since there are offsets, strictly speaking this is not a prefix but a general
subrange.
src/java.base/share/classes/java/lang/StringUTF16.java line 499:
> 497:
> 498: @IntrinsicCandidate
> 499: static void getChars(byte[] value, int srcBegin, int srcEnd, char[]
> dst, int dstBegin) {
What about this intrinsic? It doesn't seem to follow the conventions about
`private` and a gatekeeper method.
src/java.base/share/classes/java/lang/StringUTF16.java line 623:
> 621: Objects.requireNonNull(other);
> 622: checkOffset(len1, value);
> 623: String.checkOffset(len2, StringLatin1.length(other));
FYI, all these checks are already performed in the called (non-intrinsic)
method.
src/java.base/share/classes/java/lang/StringUTF16.java line 726:
> 724: static int compareToCI_Latin1(byte[] value, byte[] other) {
> 725: Objects.requireNonNull(value);
> 726: Objects.requireNonNull(other);
FYI, checks are already performed in the called method.
src/java.base/share/classes/java/lang/StringUTF16.java line 892:
> 890: }
> 891:
> 892: private static int indexOfUnsafe(byte[] value, int valueToIndex,
> byte[] str, int strToIndex, int valueFromIndex) {
I think this method requires documentation about what it assumes about the
arguments.
While it can be somehow derived from its usages, it would help to have it
located here.
src/java.base/share/classes/java/lang/StringUTF16.java line 981:
> 979: }
> 980:
> 981: private static int indexOfLatin1Unsafe(byte[] src, int srcCount,
> byte[] tgt, int tgtCount, int fromIndex) {
Similarly to above, documenting the expectations about the arguments here would
help.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804366706
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804382508
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804368864
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804413083
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804428055
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804470315
PR Review Comment: https://git.openjdk.org/jdk/pull/28832#discussion_r2804488037
