On Mon, 24 Apr 2023 16:26:02 GMT, Adam Sotona <asot...@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:
>
> Doc fixes + added null and empty tests
src/java.base/share/classes/java/lang/constant/ModuleDesc.java line 30:
> 28:
> 29: /**
> 30: * A nominal descriptor for a {@code Module} constant {@jvms 4.4.11}.
This inlined link looks a bit awkward in the output.
I suggest to move `{@jvms 4.4.11}` as see also section before `@since 21`. See
the suggestion below
src/java.base/share/classes/java/lang/constant/ModuleDesc.java line 32:
> 30: * A nominal descriptor for a {@code Module} constant {@jvms 4.4.11}.
> 31: *
> 32: * <p>To create a {@linkplain ModuleDesc} for a module, use {@link #of}.
Suggestion:
* <p>To create a {@linkplain ModuleDesc} for a module, use the {@link #of}
method.
src/java.base/share/classes/java/lang/constant/ModuleDesc.java line 34:
> 32: * <p>To create a {@linkplain ModuleDesc} for a module, use {@link #of}.
> 33: *
> 34: * @since 21
Suggestion:
* @jvms 4.4.11 The CONSTANT_Module_info Structure
* @since 21
src/java.base/share/classes/java/lang/constant/ModuleDesc.java line 43:
> 41: * given the name of the module.
> 42: *
> 43: * <p>{@jvms 4.2.3} Module names are not encoded in "internal form"
> like
I would avoid copying JVMS 4.2.3 into the javadoc as it might become
out-of-sync. In addition, adding `@jvms 4.2.3 Module and Package Names` in the
see also section should be adequate (like the javadoc of
`ClassDesc::ofInternalName`).
src/java.base/share/classes/java/lang/constant/PackageDesc.java line 30:
> 28:
> 29: /**
> 30: * A nominal descriptor for a {@code Package} constant {@jvms 4.4.12}.
I suggest to move `{@jvms 4.4.12}` as see also section before `@since 21`. See
the suggestion below
src/java.base/share/classes/java/lang/constant/PackageDesc.java line 35:
> 33: * {@link #ofInternalName(String)}.
> 34: *
> 35: * @since 21
Suggestion:
* * @jvms 4.4.11 The CONSTANT_Module_info Structure
* @since 21
The output javadoc will show `See Java Virtual Machine Specification:`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175818650
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175830777
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175819238
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175822059
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175774209
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1175774600
