On Sat, 22 Apr 2023 18:40:45 GMT, Chen Liang <li...@openjdk.org> wrote:

>> Add a method `internalName` to `ClassDesc`, and unifies handling of string 
>> representation of a class constant in CONSTANT_Class_info via 
>> `ofInternalName` and `internalName` APIs, documented in `ClassDesc` itself. 
>> In particular, `ofInternalName` now accepts arrays.
>> 
>> The motivation of this API is that avoiding frequent String creations via 
>> caching (enabled by this new API, will be in a separate patch) would speed 
>> up Classfile API's [writing of simple class 
>> files](https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/jdk/classfile/Write.java)
>>  by 1/3. See 
>> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-April/000296.html 
>> for more context.
>> 
>> This API is futureproof: for Valhalla's Q-types, it will return their string 
>> representation in CONSTANT_Class_info, which is most likely their full 
>> descriptor string.
>> 
>> Javadoc: 
>> https://cr.openjdk.org/~liach/8306697/java.base/java/lang/constant/ClassDesc.html
>
> Chen Liang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Unify ofInternalName and internalName, document about CONSTANT_Class_info, 
> remove misleading JVMS 4.4.1 links

src/java.base/share/classes/java/lang/constant/ClassDesc.java line 107:

> 105:      * @see ClassDesc#internalName()
> 106:      * @see <a href="#constant-class-info">The {@code 
> CONSTANT_Class_info} Structure</a>
> 107:      * @since 20

This should have `@revised`, as array descriptors are not allowed as input to 
this method in **JDK 20**.
Suggestion:

     * @since 20
     * @revised 21

src/java.base/share/classes/java/lang/constant/ClassDesc.java line 375:

> 373:      * {@code CONSTANT_Class_info}.
> 374:      *
> 375:      * @throws IllegalStateException if this {@linkplain ClassDesc} 
> describes a type

This should really throw `UnsupportedOperationException`, as the instances are 
immutable, same as [`Class::arrayType()`]. See [JDK‑8268250] and [GH‑4382] for 
discussion.

[`Class::arrayType()`]: 
https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/Class.html#arrayType()
[JDK‑8268250]: https://bugs.openjdk.org/browse/JDK-8268250
[GH‑4382]: https://github.com/openjdk/jdk/pull/4382

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13598#discussion_r1174579298
PR Review Comment: https://git.openjdk.org/jdk/pull/13598#discussion_r1174578847

Reply via email to