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

[Roger Riggs]

On Mon, 24 Apr 2023 13:18:09 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Constants API already provides models for all loadable constants to help programs manipulating class files and modelling bytecode instructions. However no models of module and package constants are provided by Constants API. Every program manipulating class files must implement own models and validation of modules and packages constants.
>> 
>> This pul request adds `java.lang.constant.ModuleDesc` and `java.lang.constant.PackageDesc` to the Constants API. 
>> 
>> Classfile API will follow up and remove its internal implementations of `PackageDesc` and `ModuleDesc`. 
>> 
>> Please review this pull request and attached CSR.
>> 
>> Thank you,
>> Adam
>
> 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.

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?

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.

src/java.base/share/classes/java/lang/constant/package-info.java line 92:

> 90:  * reading and writing APIs.
> 91:  *
> 92:  * <p>Another members of this package are {@link java.lang.constant.ModuleDesc}

"Another" -> "Other"

test/jdk/java/lang/constant/ModuleDescTest.java line 37:

> 35: 
> 36: class ModuleDescTest {
> 37: 

Add tests for empty and null arguments.

test/jdk/java/lang/constant/PackageDescTest.java line 39:

> 37: 
> 38: class PackageDescTest {
> 39:     @ParameterizedTest

Add tests for empty and null arguments.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175375915
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175366692
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175371637
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175382854
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175383972
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175384822