Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8335111

Fix instruction name and API spec inconsistencies in CodeBuilder

XMLWordPrintable

    • source
    • minimal
    • Changes to preview APIs involve minimal risk.
    • Java API
    • SE

      Summary

      1. Specify what instruction CodeBuilder instruction factories can emit, and the mnemonics of instructions that are emitted (if there's a name mismatch); fix two meaningless name mismatches.
      2. Fix the IAE clause for CodeBuilder::receiverSlot
      3. Specify IAE behavior for CodeAttribute::labelToBci
      4. Change behavior of CodeBuilder::exceptionCatch (non-Optional version)

      Problem

      There are a few API problems with CodeBuilder and CodeAttribute: CodeBuilder has mnemonic-named methods but it's not fully complete, and there are name mismatches due to reasons; it has bad specification for receiverSlot() and bad implementation for exceptionCatch. Also, CodeAttribute.labelToBci has bad specification.

      Solution

      1. Add a section in CodeBuilder specification discussing the instruction factories. This section explains that the list is not complete, that there's merged factories for fixed opcodes or wide operands, renames due to clash with Java keywords, and missing legacy instructions. Similar notes are added to individual related factories. (xload/xstore, goto, 4 reserved names)
      2. Rename unnecessary mismatches for ifnonnull and ifnull
      3. Fix typo in CodeBuilder::receiverSlot spec
      4. Fix implementation in CodeBuilder::exceptionCatch to accept null ClassEntry
      5. Fix CodeAttribute::labelToBci spec to match implementation

      Specification

      diff --git a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
      index e954be079fa..cffa560bef3 100644
      --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
      +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
      @@ -97,6 +97,27 @@
        * #with(ClassFileElement)} or concretely by calling the various {@code withXxx}
        * methods.
        *
      + * <h2>Instruction Factories</h2>
      + * {@code CodeBuilder} provides convenience methods to create instructions (See
      + * JVMS {@jvms 6.5} Instructions) by their mnemonic, taking necessary operands.
      + * <ul>
      + * <li>Instructions that encode their operands in their opcode, such as {@code
      + * aload_<n>}, share their factories with their generic version like {@link
      + * #aload aload}. Note that some constant instructions, such as {@link #iconst_1
      + * iconst_1}, do not have generic versions, and thus have their own factories.
      + * <li>Instructions that accept wide operands, such as {@code ldc2_w} or {@code
      + * wide}, share their factories with their regular version like {@link #ldc}. Note
      + * that {@link #goto_w goto_w} has its own factory to avoid {@linkplain
      + * ClassFile.ShortJumpsOption short jumps}.
      + * <li>The {@code goto}, {@code instanceof}, {@code new}, and {@code return}
      + * instructions' factories are named {@link #goto_ goto_}, {@link #instanceOf
      + * instanceOf}, {@link #new_ new_}, and {@link #return_() return_} respectively,
      + * due to clashes with keywords in the Java programming language.
      + * <li>Factories are not provided for instructions {@code jsr}, {@code jsr_w},
      + * {@code ret}, and {@code wide ret}, which cannot appear in class files with
      + * major version {@value ClassFile#JAVA_7_VERSION} or higher. (JVMS {@jvms 4.9.1})
      + * </ul>
      + *
        * @see CodeTransform
        *
        * @since 22
      @@ -130,7 +151,7 @@ public sealed interface CodeBuilder
           /**
            * {@return the local variable slot associated with the receiver}.
            *
      -     * @throws IllegalStateException if this is not a static method
      +     * @throws IllegalStateException if this is a static method
            */
           int receiverSlot();
      
      @@ -837,6 +858,10 @@ default CodeBuilder aastore() {
      
           /**
            * Generate an instruction to load a reference from a local variable
      +     *
      +     * <p>This may also generate {@code aload_<N>} and
      +     * {@code wide aload} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -881,6 +906,10 @@ default CodeBuilder arraylength() {
      
           /**
            * Generate an instruction to store a reference into a local variable
      +     *
      +     * <p>This may also generate {@code astore_<N>} and
      +     * {@code wide astore} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1046,6 +1075,10 @@ default CodeBuilder ddiv() {
      
           /**
            * Generate an instruction to load a double from a local variable
      +     *
      +     * <p>This may also generate {@code dload_<N>} and
      +     * {@code wide dload} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1087,6 +1120,10 @@ default CodeBuilder dreturn() {
      
           /**
            * Generate an instruction to store a double into a local variable
      +     *
      +     * <p>This may also generate {@code dstore_<N>} and
      +     * {@code wide dstore} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1250,6 +1287,10 @@ default CodeBuilder fdiv() {
      
           /**
            * Generate an instruction to load a float from a local variable
      +     *
      +     * <p>This may also generate {@code fload_<N>} and
      +     * {@code wide fload} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1291,6 +1332,10 @@ default CodeBuilder freturn() {
      
           /**
            * Generate an instruction to store a float into a local variable
      +     *
      +     * <p>This may also generate {@code fstore_<N>} and
      +     * {@code wide fstore} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1350,6 +1395,15 @@ default CodeBuilder getstatic(ClassDesc owner, String name, ClassDesc type) {
      
           /**
            * Generate an instruction to branch always
      +     *
      +     * <p>This may also generate {@code goto_w} instructions if the {@link
      +     * ClassFile.ShortJumpsOption#FIX_SHORT_JUMPS FIX_SHORT_JUMPS} option
      +     * is set.
      +     *
      +     * @apiNote The instruction's name is {@code goto}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with an extra {@code _} suffix instead.
      +     *
            * @param target the branch target
            * @return this builder
            */
      @@ -1587,7 +1641,7 @@ default CodeBuilder if_icmpne(Label target) {
            * @param target the branch target
            * @return this builder
            */
      -    default CodeBuilder if_nonnull(Label target) {
      +    default CodeBuilder ifnonnull(Label target) {
               return branch(Opcode.IFNONNULL, target);
           }
      
      @@ -1596,7 +1650,7 @@ default CodeBuilder if_nonnull(Label target) {
            * @param target the branch target
            * @return this builder
            */
      -    default CodeBuilder if_null(Label target) {
      +    default CodeBuilder ifnull(Label target) {
               return branch(Opcode.IFNULL, target);
           }
      
      @@ -1666,6 +1720,10 @@ default CodeBuilder iinc(int slot, int val) {
      
           /**
            * Generate an instruction to load an int from a local variable
      +     *
      +     * <p>This may also generate {@code iload_<N>} and
      +     * {@code wide iload} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -1691,6 +1749,11 @@ default CodeBuilder ineg() {
      
           /**
            * Generate an instruction to determine if an object is of the given type
      +     *
      +     * @apiNote The instruction's name is {@code instanceof}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with camel case instead.
      +     *
            * @param target the target type
            * @return this builder
            * @since 23
      @@ -1701,6 +1764,11 @@ default CodeBuilder instanceOf(ClassEntry target) {
      
           /**
            * Generate an instruction to determine if an object is of the given type
      +     *
      +     * @apiNote The instruction's name is {@code instanceof}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with camel case instead.
      +     *
            * @param target the target type
            * @return this builder
            * @throws IllegalArgumentException if {@code target} represents a primitive type
      @@ -1910,6 +1978,10 @@ default CodeBuilder ishr() {
      
           /**
            * Generate an instruction to store an int into a local variable
      +     *
      +     * <p>This may also generate {@code istore_<N>} and
      +     * {@code wide istore} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -2033,6 +2105,12 @@ default CodeBuilder lconst_1() {
      
           /**
            * Generate an instruction pushing an item from the run-time constant pool onto the operand stack
      +     *
      +     * <p>This may also generate {@code ldc_w} and {@code ldc2_w} instructions.
      +     *
      +     * @apiNote {@link #loadConstant(ConstantDesc) loadConstant} generates more optimal instructions
      +     * and should be used for general constants if an {@code ldc} instruction is not strictly required.
      +     *
            * @param value the constant value
            * @return this builder
            */
      @@ -2042,6 +2120,9 @@ default CodeBuilder ldc(ConstantDesc value) {
      
           /**
            * Generate an instruction pushing an item from the run-time constant pool onto the operand stack
      +     *
      +     * <p>This may also generate {@code ldc_w} and {@code ldc2_w} instructions.
      +     *
            * @param entry the constant value
            * @return this builder
            */
      @@ -2062,6 +2143,10 @@ default CodeBuilder ldiv() {
      
           /**
            * Generate an instruction to load a long from a local variable
      +     *
      +     * <p>This may also generate {@code lload_<N>} and
      +     * {@code wide lload} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -2127,6 +2212,10 @@ default CodeBuilder lshr() {
      
           /**
            * Generate an instruction to store a long into a local variable
      +     *
      +     * <p>This may also generate {@code lstore_<N>} and
      +     * {@code wide lstore} instructions.
      +     *
            * @param slot the local variable slot
            * @return this builder
            */
      @@ -2197,6 +2286,11 @@ default CodeBuilder multianewarray(ClassDesc array, int dims) {
      
           /**
            * Generate an instruction to create a new object
      +     *
      +     * @apiNote The instruction's name is {@code new}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with an extra {@code _} suffix instead.
      +     *
            * @param clazz the new class type
            * @return this builder
            */
      @@ -2206,6 +2300,11 @@ default CodeBuilder new_(ClassEntry clazz) {
      
           /**
            * Generate an instruction to create a new object
      +     *
      +     * @apiNote The instruction's name is {@code new}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with an extra {@code _} suffix instead.
      +     *
            * @param clazz the new class type
            * @return this builder
            * @throws IllegalArgumentException if {@code clazz} represents a primitive type
      @@ -2283,6 +2382,11 @@ default CodeBuilder putstatic(ClassDesc owner, String name, ClassDesc type) {
      
           /**
            * Generate an instruction to return void from the method
      +     *
      +     * @apiNote The instruction's name is {@code return}, which coincides with a
      +     * reserved keyword of the Java programming language, thus this method is
      +     * named with an extra {@code _} suffix instead.
      +     *
            * @return this builder
            */
           default CodeBuilder return_() {
      diff --git a/src/java.base/share/classes/java/lang/classfile/attribute/CodeAttribute.java b/src/java.base/share/classes/java/lang/classfile/attribute/CodeAttribute.java
      index 1a57cf4f37f..02cbcee810f 100644
      --- a/src/java.base/share/classes/java/lang/classfile/attribute/CodeAttribute.java
      +++ b/src/java.base/share/classes/java/lang/classfile/attribute/CodeAttribute.java
      @@ -58,9 +58,9 @@ public sealed interface CodeAttribute extends Attribute<CodeAttribute>, CodeMode
           byte[] codeArray();
      
           /**
      -     * {@return the position of the {@code Label} in the {@code codeArray}
      -     * or -1 if the {@code Label} does not point to the {@code codeArray}}
      +     * {@return the position of the {@code label} in the {@link #codeArray codeArray}}
            * @param label a marker for a position within this {@code CodeAttribute}
      +     * @throws IllegalArgumentException if the {@code label} is not from this attribute
            */
           int labelToBci(Label label);
       }

            liach Chen Liang
            liach Chen Liang
            Adam Sotona
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: