JDK-8339283 : Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc)
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.lang.classfile
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 24
  • Submitted: 2024-08-29
  • Updated: 2024-08-30
  • Resolved: 2024-08-30
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Remove redundant and error-prone `CodeBuilder::loadConstant(Opcode, ConstantDesc)`.

Problem
-------

`CodeBuilder::loadConstant(Opcode, ConstantDesc)` was added before addition of convenience instruction factories to `CodeBuilder`, named `constantInstruction` back then. Almost always users should use `loadConstant(ConstantDesc)` instead.

In addition, there were argument validation for `bipush` and `sipush`, but it only existed in this method but not on the actual instruction objects. These 2 instructions actually produce `int` on the stack instead of `byte` or `short` which do not exist as separate types in the JVM.

Solution
--------

Remove this misleading API. Users should use `loadConstant` without an argument, use the specific factories like `bipush` or `sipush` to create instructions, or just use `with(ConstantInstruction.of(xxx))` if other factories do not suffice.

Move the instruction argument check to the actual `ArgumentConstantInstruction` factory and specify the check will throw exceptions.

Specify that `bipush` and `sipush` actually produce `int`, but the `int` value is in `byte` range or `short` range. They will now be considered of `INT` type kind.

Specification
-------------

    --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
    @@ -612,23 +612,6 @@ default CodeBuilder conversion(TypeKind fromType, TypeKind toType) {
             };
         }
     
    -    /**
    -     * Generate an instruction pushing a constant onto the operand stack
    -     * @see Opcode.Kind#CONSTANT
    -     * @param opcode the constant instruction opcode
    -     * @param value the constant value
    -     * @return this builder
    -     * @since 23
    -     */
    -    default CodeBuilder loadConstant(Opcode opcode, ConstantDesc value) {
    
         /**
          * Generate an instruction pushing a constant onto the operand stack
          * @param value the constant value
    @@ -939,12 +922,12 @@ default CodeBuilder bastore() {
         }
     
         /**
    -     * Generate an instruction pushing a byte onto the operand stack
    -     * @param b the byte
    +     * Generate an instruction pushing an int in the range of byte onto the operand stack.
    +     * @param b the int in the range of byte
          * @return this builder
          */
         default CodeBuilder bipush(int b) {
    
         /**
    @@ -2404,12 +2387,12 @@ default CodeBuilder sastore() {
         }
     
         /**
    -     * Generate an instruction pushing a short onto the operand stack
    -     * @param s the short
    +     * Generate an instruction pushing an int in the range of short onto the operand stack.
    +     * @param s the int in the range of short
          * @return this builder
          */
         default CodeBuilder sipush(int s) {
    
         /**
    --- a/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java
    +++ b/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java
    @@ -148,12 +149,18 @@ static IntrinsicConstantInstruction ofIntrinsic(Opcode op) {
    -     * @param op the opcode for the specific type of intrinsic constant instruction,
    -     *           which must be of kind {@link Opcode.Kind#CONSTANT}
    +     * @param op the opcode for the specific type of argument constant instruction,
    +     *           which must be {@link Opcode#BIPUSH} or {@link Opcode#SIPUSH}
          * @param value the constant value
          * @throws IllegalArgumentException if the opcode is not {@link Opcode#BIPUSH}
    -     *                                  or {@link Opcode#SIPUSH}
    +     *         or {@link Opcode#SIPUSH}, or if the constant value is out of range
    +     *         for the opcode
          */
         static ArgumentConstantInstruction ofArgument(Opcode op, int value) {
    
Comments
Moving to Approved.
30-08-2024

Assuming this fix is intended for JDK 24.
30-08-2024