RFR: 8294982: Implementation of Classfile API [v13]

[Maurizio Cimadamore]

On Mon, 6 Feb 2023 13:29:10 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> This is root pull request with Classfile API implementation, tests and benchmarks initial drop into JDK.
>> 
>> Following pull requests consolidating JDK class files parsing, generating, and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) will chain to this one.
>> 
>> Classfile API development is tracked at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
>> 
>> Development branch of consolidated JDK class files parsing, generating, and transforming is at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
>> 
>> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online API documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/classfile/package-summary.html) is also available.
>> 
>> Please take you time to review this non-trivial JDK addition.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - javadoc fixes
>  - obsolete identifiers and unused imports cleanup
>  - TypeAnnotation.TypePathComponent cleanup

src/java.base/share/classes/jdk/internal/classfile/attribute/CharacterRangeInfo.java line 80:

> 78:      * well as increment and decrement expressions (both prefix and postfix).
> 79:      * <li>{@link jdk.internal.classfile.Classfile#CRT_FLOW_CONTROLLER} An expression
> 80:      * whose value will effect control flow. Flowcon in the following:

This sentence seems hard to parse. If `Flowcon` is the name of an expression in pseudocode, perhaps using `{@code}` would help.

src/java.base/share/classes/jdk/internal/classfile/attribute/CharacterRangeInfo.java line 146:

> 144:      * @param flags the range flags
> 145:      */
> 146:     static CharacterRangeInfo of(int startPc,

I get that the encoding is as per JaCoCo specification. However, I also wonder if the API should provide better accessors and or factories to let clients just reason about line/columns numbers. Or, maybe, provide static helpers to do the encoding/decoding.

src/java.base/share/classes/jdk/internal/classfile/attribute/CodeAttribute.java line 52:

> 50:     byte[] codeArray();
> 51: 
> 52:     int labelToBci(Label label);

Missing javadoc here. (also, this method looks a bit odd in here - but I see uses e.g. in ClassPrinterImpl)

src/java.base/share/classes/jdk/internal/classfile/attribute/ModuleAttribute.java line 132:

> 130:     }
> 131: 
> 132:     static ModuleAttribute of(ModuleDesc moduleName,

Some missing javadoc in this class

src/java.base/share/classes/jdk/internal/classfile/attribute/ModuleExportInfo.java line 107:

> 105:      * @param exportsTo the modules to which this package is exported
> 106:      */
> 107:     static ModuleExportInfo of(PackageEntry exports,

(Here and elsewhere) - should there be factories also taking a PackageDesc (which seems to be an extension of the ClassDesc API, but for packages) ?

src/java.base/share/classes/jdk/internal/classfile/attribute/NestMembersAttribute.java line 72:

> 70:      * @param nestMembers the member classes of the nest
> 71:      */
> 72:     static NestMembersAttribute ofSymbols(List<ClassDesc> nestMembers) {

I see that in some classes we use the `Symbols` prefix, in others we just use `of` - we should try to make the more consistent (in the future).

src/java.base/share/classes/jdk/internal/classfile/attribute/StackMapTableAttribute.java line 68:

> 66:      * A simple stack value.
> 67:      */
> 68:     public enum SimpleVerificationTypeInfo implements VerificationTypeInfo {

I note that in this class we have made the decision to model all the innards (XYZInfo) as nested classes - while in all the other cases XYZInfo are toplevel types. Moving forward, we should pick something consistent.

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

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