RFR: 8372353: API to compute the byte length of a String encoded in a given Charset [v2]

Liam Miller-Cushon cushon at openjdk.org
Thu Jan 15 09:13:18 UTC 2026


On Wed, 14 Jan 2026 22:49:06 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Liam Miller-Cushon has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review feedback
>
> src/java.base/share/classes/java/lang/String.java line 1080:
> 
>> 1078:             return value.length;
>> 1079:         }
>> 1080:         int len = value.length >> 1;
> 
> I don't think I understand what's being done and what Charset encoder it is mimicking.
> It probably needs to document the assumptions about unmappable characters and malformed surrogates.
> (Likely it is correct since the test of US_ASCII passes, but could use an explanation).

I added some `//` comments documenting which methods the `encodedLength*` methods are mimicking. The logic here should be identical to `encodeASCII` (except that it isn't allocating and writing to a destination array).

The handling of unmappable characters and malformed surrogates should match `encodeASCII`.

> src/java.base/share/classes/java/lang/String.java line 2130:
> 
>> 2128: 
>> 2129:     /**
>> 2130:      * Returns the length in bytes of the given String encoded with the given {@link Charset}.
> 
> You can use the javadoc tag `@return` and skip the duplication.  This first sentence reads better then the @return below since it emphasies the "encoded string" aspect.

Sorry I'm not sure I understand, can you clarify how that would work?

The javadoc can't start with `@return`, it needs to be a non-tag sentence fragment (the build enables doclint to enforce this).

> src/java.base/share/classes/java/lang/String.java line 2136:
> 
>> 2134:      * @implNote This method may allocate memory to compute the length for some charsets.
>> 2135:      *
>> 2136:      * @param cs the charset used to the compute the length
> 
> Capitalize and perhaps link "Charset".

Done

> src/java.base/share/classes/java/lang/String.java line 2155:
> 
>> 2153:     }
>> 2154: 
>> 2155:     private int byteFor(int length, int coder) {
> 
> Add a //comment please.
> The method name should be plural.  `bytesForCoder`.

Done

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693532105
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693479863
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693480259
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693532859


More information about the core-libs-dev mailing list