RFR: 8294982: Implementation of Classfile API [v15]

Maurizio Cimadamore mcimadamore at openjdk.org
Thu Feb 9 15:10:37 UTC 2023


On Thu, 9 Feb 2023 08:44:41 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/internal/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 one additional commit since the last revision:
> 
>   AttributeElement.Kind removal (#48)

src/java.base/share/classes/jdk/internal/classfile/instruction/CharacterRange.java line 47:

> 45:      * {@return the start of the instruction range}
> 46:      */
> 47:     Label startScope();

I noticed that this pattern of start/endScope is here, but also in ExceptionCatch, LocalVariable and LocalVariableType. Consider using a common interface for "instructions that are associated with a range".

src/java.base/share/classes/jdk/internal/classfile/instruction/CharacterRange.java line 60:

> 58:      * viewed as an array of (possibly multi-byte) characters.
> 59:      */
> 60:     int characterRangeStart();

Not sure if this belongs here - e.g. it seems to me that `characterRangeStart` is less useful than the labels. But I'm not super expert on how this code element might be used.

src/java.base/share/classes/jdk/internal/classfile/instruction/ConstantInstruction.java line 63:

> 61:      * aload_0}).
> 62:      */
> 63:     sealed interface IntrinsicConstantInstruction extends ConstantInstruction

I'm not super sure of the fine-grained distinction here. The constant pool variant is interesting (as you can ask for the associated constant entry) - but the distinction between intrinsics vs. argument seems rather weak.

src/java.base/share/classes/jdk/internal/classfile/instruction/InvokeInstruction.java line 60:

> 58:      * {@return whether the class holding the method is an interface}
> 59:      */
> 60:     boolean isInterface();

This can also be tested with pattern matching on the MemberRefEntry?

src/java.base/share/classes/jdk/internal/classfile/instruction/MonitorInstruction.java line 48:

> 46:      *           which must be of kind {@link Opcode.Kind#MONITOR}
> 47:      */
> 48:     static MonitorInstruction of(Opcode op) {

There are only two cases here - perhaps also offer factories for monitor enter/exit? Or is creating instruction models a rare operation (e.g. because when adapting you always also have a CodeBuilder which has the user-friendly methods?)

src/java.base/share/classes/jdk/internal/classfile/instruction/TypeCheckInstruction.java line 39:

> 37: 
> 38: /**
> 39:  * Models an {@code instanceof} or {@code checkcast} instruction in the {@code

This seems to model both `instanceof` and `checkcast`. The latter seems to overlap partially with `ConvertInstruction`.

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

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



More information about the build-dev mailing list