On Thu, 17 Jul 2025 13:58:32 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Replace casting with `as_Region()` in `generate_string_range_check`
>
> src/java.base/share/classes/java/lang/StringCoding.java line 130:
>
>> 128: * <p>
>> 129: * This method assumes that {@code sa} is encoded in UTF-16, and
>> hence,
>> 130: * each {@code char} maps to 2 bytes.
>
> Since `sa` is assumed to be encoded in UTF-16, what's the point of the
> previous paragraph?
It might be obvious to some, but some (incl. myself) got really confused on
what input arguments actually denote, and how they are operated. I'm inclined
to keep the for-dummies explanation, unless there is a strong objection.
> src/java.base/share/classes/java/lang/StringCoding.java line 150:
>
>> 148: Objects.requireNonNull(sa, "sa");
>> 149: Objects.requireNonNull(da, "da");
>> 150: Preconditions.checkFromIndexSize(sp, len << 1, sa.length,
>> Preconditions.AIOOBE_FORMATTER);
>
> Since `sp` is a character index and `len` counts characters but `sa.length`
> is in bytes, this should look like so
> Suggestion:
>
> Preconditions.checkFromIndexSize(sp, len, sa.length >>> 1,
> Preconditions.AIOOBE_FORMATTER);
>
> or something similar.
Very sharp of you @rgiulietti! Thanks so much! 🙇 Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25998#discussion_r2219097580
PR Review Comment: https://git.openjdk.org/jdk/pull/25998#discussion_r2219099452
