RFR: 8278863: Add method ClassDesc::ofInternalName [v3]

Maurizio Cimadamore mcimadamore at openjdk.org
Tue Jul 26 09:29:04 UTC 2022


On Mon, 20 Jun 2022 07:45:46 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> The symbolic constants API (`java.lang.constant`) was designed, in part, to provide bytecode parsing and generation APIs with a validated, common, symbolic form for constants in Java class files. 
>> 
>> There is an easy way to create a `ClassDesc` from a binary name (`ClassDesc::of`) or a field descriptor (`ClassDesc::ofDescriptor`) but not from an internal name. But, the internal name is common in low-level bytecode-manipulation code. 
>> 
>> This patch adds `ClassDesc::ofInternalName` static factory method that creates a `ClassDesc` from class internal name.
>> Class internal name validation and extended ClassDescTest are also parts of this patch.
>> 
>> CSR is linked with the issue.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional commit since the last revision:
> 
>   updated ClassDesc::ofInternal javadoc with JVMS link and fixed indentation

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

> 83:      * given the name of the class or interface in internal form,
> 84:      * such as {@code "java/lang/String"}.
> 85:      * (To create a descriptor for an array type, either use {@link #ofDescriptor(String)}

It feels like this section could be expanded a bit (and parenthesis removed). E.g. as you point out in the comment, this method only supports reference types, so we should state so clearly. And then present alternatives (like you do now).

test/jdk/java/lang/constant/ClassDescTest.java line 268:

> 266:         }
> 267: 
> 268:         List<String> badInternalNames = List.of("I;", "[]",

is `[]` a good test for arrays? Wouldn't something more realistic like `[Ljava/lang/String` be better? I note that `[]` is also used in another test. Problem with testing `[]` is that if the checking logic has a bug, or a regression is introduced so that it only detects `]` instead of `[` the test would not detect that (but I guess that's a remote possibility).

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

PR: https://git.openjdk.org/jdk/pull/9201


More information about the core-libs-dev mailing list