RFR: 8306729: Add nominal descriptors of modules and packages to Constants API [v2]

[Adam Sotona]

On Mon, 24 Apr 2023 14:30:46 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Adam Sotona has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   added links to JVMS and utility methods moved to ConstantUtils
>
> src/java.base/share/classes/java/lang/constant/ConstantUtils.java line 80:
> 
>> 78:     /**
>> 79:      * Validates the correctness of a binary package name. In particular checks for the presence of
>> 80:      * invalid characters in the name.
> 
> It may be useful to note that the empty string are considered valid. 
> Also add @throws NPE; there's an implicit null check in checking the length.
> Also in validateBinaryPackageName and validateModuleName.

I'll add that notes, thanks.

> src/java.base/share/classes/java/lang/constant/ConstantUtils.java line 130:
> 
>> 128:         }
>> 129:         return name;
>> 130:     }
> 
> If these are performance sensitive or get used a lot, should there be an array of flags indexed by the byte/char to indicate the valid cases?

It is just a range check and three escaped characters check. The range check can be implemented bit-wise and escaped check may be by a switch case, however I don't think there would be any performance benefits.

> src/java.base/share/classes/java/lang/constant/ModuleDescImpl.java line 27:
> 
>> 25: package java.lang.constant;
>> 26: 
>> 27: record ModuleDescImpl(String moduleName) implements ModuleDesc {
> 
> Given the validation is elsewhere, it might be useful to add a comment saying the moduleName must have been validated.

I'll add it, thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175483446
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175482952
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175484294