8339214: Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc)

Reviewed-by: asotona

src/java.base/share/classes/java/lang/classfile/CodeBuilder.java

@@ -604,23 +604,6 @@ public sealed interface CodeBuilder
         return this;
     }
 
-    /**
-     * 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) {
-        BytecodeHelpers.validateValue(opcode, value);
-        return with(switch (opcode) {
-            case SIPUSH, BIPUSH -> ConstantInstruction.ofArgument(opcode, ((Number)value).intValue());
-            case LDC, LDC_W, LDC2_W -> ConstantInstruction.ofLoad(opcode, BytecodeHelpers.constantEntry(constantPool(), value));
-            default -> ConstantInstruction.ofIntrinsic(opcode);
-        });
-    }
-
     /**
      * Generate an instruction pushing a constant onto the operand stack
      * @param value the constant value
@@ -931,12 +914,12 @@ public sealed interface CodeBuilder
     }
 
     /**
-     * Generate an instruction pushing a byte onto the operand stack
+     * Generate an instruction pushing an int in the range of byte onto the operand stack.
-     * @param b the byte
+     * @param b the int in the range of byte
      * @return this builder
      */
     default CodeBuilder bipush(int b) {
-        return loadConstant(Opcode.BIPUSH, b);
+        return with(ConstantInstruction.ofArgument(Opcode.BIPUSH, b));
     }
 
     /**
@@ -2396,12 +2379,12 @@ public sealed interface CodeBuilder
     }
 
     /**
-     * Generate an instruction pushing a short onto the operand stack
+     * Generate an instruction pushing an int in the range of short onto the operand stack.
-     * @param s the short
+     * @param s the int in the range of short
      * @return this builder
      */
     default CodeBuilder sipush(int s) {
-        return loadConstant(Opcode.SIPUSH, s);
+        return with(ConstantInstruction.ofArgument(Opcode.SIPUSH, s));
     }
 
     /**

src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@ import java.lang.classfile.Opcode;
 import java.lang.classfile.TypeKind;
 import java.lang.classfile.constantpool.LoadableConstantEntry;
 import jdk.internal.classfile.impl.AbstractInstruction;
+import jdk.internal.classfile.impl.BytecodeHelpers;
 import jdk.internal.classfile.impl.Util;
 import jdk.internal.javac.PreviewFeature;
 
@@ -144,16 +145,21 @@ public sealed interface ConstantInstruction extends Instruction {
     /**
      * {@return an argument constant instruction}
      *
-     * @param op the opcode for the specific type of intrinsic constant instruction,
+     * @param op the opcode for the specific type of argument constant instruction,
-     *           which must be of kind {@link Opcode.Kind#CONSTANT}
+     *           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) {
-        Util.checkKind(op, Opcode.Kind.CONSTANT);
+        if (op == Opcode.BIPUSH) {
-        if (op != Opcode.BIPUSH && op != Opcode.SIPUSH)
+            BytecodeHelpers.validateBipush(value);
+        } else if (op == Opcode.SIPUSH) {
+            BytecodeHelpers.validateSipush(value);
+        } else {
             throw new IllegalArgumentException(String.format("Wrong opcode specified; found %s, expected BIPUSH or SIPUSH", op));
+        }
         return new AbstractInstruction.UnboundArgumentConstantInstruction(op, value);
     }
 

src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java

@@ -277,40 +277,20 @@ public class BytecodeHelpers {
         };
     }
 
-    static void validateSipush(long value) {
+    public static void validateSipush(int value) {
-        if (value < Short.MIN_VALUE || Short.MAX_VALUE < value)
+        if (value != (short) value)
             throw new IllegalArgumentException(
                     "SIPUSH: value must be within: Short.MIN_VALUE <= value <= Short.MAX_VALUE, found: "
                             .concat(Long.toString(value)));
     }
 
-    static void validateBipush(long value) {
+    public static void validateBipush(int value) {
-        if (value < Byte.MIN_VALUE || Byte.MAX_VALUE < value)
+        if (value != (byte) value)
             throw new IllegalArgumentException(
                     "BIPUSH: value must be within: Byte.MIN_VALUE <= value <= Byte.MAX_VALUE, found: "
                             .concat(Long.toString(value)));
     }
 
-    static void validateSipush(ConstantDesc d) {
-        if (d instanceof Integer iVal) {
-            validateSipush(iVal.longValue());
-        } else if (d instanceof Long lVal) {
-            validateSipush(lVal.longValue());
-        } else {
-            throw new IllegalArgumentException("SIPUSH: not an integral number: ".concat(d.toString()));
-        }
-    }
-
-    static void validateBipush(ConstantDesc d) {
-        if (d instanceof Integer iVal) {
-            validateBipush(iVal.longValue());
-        } else if (d instanceof Long lVal) {
-            validateBipush(lVal.longValue());
-        } else {
-            throw new IllegalArgumentException("BIPUSH: not an integral number: ".concat(d.toString()));
-        }
-    }
-
     public static MethodHandleEntry handleDescToHandleInfo(ConstantPoolBuilder constantPool, DirectMethodHandleDesc bootstrapMethod) {
         ClassEntry bsOwner = constantPool.classEntry(bootstrapMethod.owner());
         NameAndTypeEntry bsNameAndType = constantPool.nameAndTypeEntry(constantPool.utf8Entry(bootstrapMethod.methodName()),
@@ -341,32 +321,6 @@ public class BytecodeHelpers {
                                                                        desc.constantType()));
     }
 
-    public static void validateValue(Opcode opcode, ConstantDesc v) {
-        switch (opcode) {
-            case ACONST_NULL -> {
-                if (v != null && v != ConstantDescs.NULL)
-                    throw new IllegalArgumentException("value must be null or ConstantDescs.NULL with opcode ACONST_NULL");
-            }
-            case SIPUSH ->
-                    validateSipush(v);
-            case BIPUSH ->
-                    validateBipush(v);
-            case LDC, LDC_W, LDC2_W -> {
-                if (v == null)
-                    throw new IllegalArgumentException("`null` must use ACONST_NULL");
-            }
-            default -> {
-                var exp = opcode.constantValue();
-                if (exp == null)
-                    throw new IllegalArgumentException("Can not use Opcode: " + opcode + " with constant()");
-                if (v == null || !(v.equals(exp) || (exp instanceof Long l && v.equals(l.intValue())))) {
-                    var t = (exp instanceof Long) ? "L" : (exp instanceof Float) ? "f" : (exp instanceof Double) ? "d" : "";
-                    throw new IllegalArgumentException("value must be " + exp + t + " with opcode " + opcode.name());
-                }
-            }
-        }
-    }
-
     public static Opcode ldcOpcode(LoadableConstantEntry entry) {
         return entry.typeKind().slotSize() == 2 ? Opcode.LDC2_W
                 : entry.index() > 0xff ? Opcode.LDC_W

src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java

@@ -258,8 +258,7 @@ public record ClassRemapperImpl(Function<ClassDesc, ClassDesc> mapFunction) impl
                     cob.localVariableType(c.slot(), c.name().stringValue(),
                             mapSignature(c.signatureSymbol()), c.startScope(), c.endScope());
                 case LoadConstantInstruction ldc ->
-                    cob.loadConstant(ldc.opcode(),
+                    cob.ldc(mapConstantValue(ldc.constantValue()));
-                            mapConstantValue(ldc.constantValue()));
                 case RuntimeVisibleTypeAnnotationsAttribute aa ->
                     cob.with(RuntimeVisibleTypeAnnotationsAttribute.of(
                             mapTypeAnnotations(aa.annotations())));

src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java

@@ -831,21 +831,6 @@ public final class DirectCodeBuilder
         return this;
     }
 
-    @Override
-    public CodeBuilder loadConstant(Opcode opcode, ConstantDesc value) {
-        BytecodeHelpers.validateValue(opcode, value);
-        // avoid non-local enum switch for bootstrap
-        if (opcode == BIPUSH || opcode == SIPUSH) {
-            writeArgumentConstant(opcode, ((Number) value).intValue());
-        } else if (opcode == LDC || opcode == LDC_W || opcode == LDC2_W) {
-            writeLoadConstant(opcode, BytecodeHelpers.constantEntry(constantPool(), value));
-        } else {
-            // intrinsics
-            writeBytecode(opcode);
-        }
-        return this;
-    }
-
     @Override
     public CodeBuilder nop() {
         writeBytecode(NOP);

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java

@@ -562,7 +562,7 @@ final class EventInstrumentation {
         // write begin event
         getEventConfiguration(blockCodeBuilder);
         // stack: [EW], [EW], [EventConfiguration]
-        blockCodeBuilder.loadConstant(Opcode.LDC2_W, eventTypeId);
+        blockCodeBuilder.loadConstant(eventTypeId);
         // stack: [EW], [EW], [EventConfiguration] [long]
         invokevirtual(blockCodeBuilder, TYPE_EVENT_WRITER, EventWriterMethod.BEGIN_EVENT.method());
         // stack: [EW], [integer]
@@ -676,7 +676,7 @@ final class EventInstrumentation {
         // stack: [EW] [EW]
         getEventConfiguration(blockCodeBuilder);
         // stack: [EW] [EW] [EC]
-        blockCodeBuilder.loadConstant(Opcode.LDC2_W, eventTypeId);
+        blockCodeBuilder.loadConstant(eventTypeId);
         invokevirtual(blockCodeBuilder, TYPE_EVENT_WRITER, EventWriterMethod.BEGIN_EVENT.method());
         // stack: [EW] [int]
         blockCodeBuilder.ifeq(excluded);

test/jdk/jdk/classfile/AdaptCodeTest.java

@@ -95,7 +95,7 @@ class AdaptCodeTest {
                     if ((val instanceof Integer) && ((Integer) val) == 13) {
                         val = 7;
                     }
-                    codeB.loadConstant(i.opcode(), val);
+                    codeB.loadConstant(val);
                 }
                 default -> codeB.with(codeE);
             }

test/jdk/jdk/classfile/LDCTest.java

@@ -64,9 +64,9 @@ class LDCTest {
                                   for (int i = 0; i <= 256/2 + 2; i++) { // two entries per String
                                       StringEntry s = cpb.stringEntry("string" + i);
                                   }
-                                  c0.loadConstant(LDC, "string0")
+                                  c0.ldc("string0")
-                                    .loadConstant(LDC, "string131")
+                                    .ldc("string131")
-                                    .loadConstant(LDC, "string50")
+                                    .ldc("string50")
                                     .loadConstant(-0.0f)
                                     .loadConstant(-0.0d)
                                     //non-LDC test cases

test/jdk/jdk/classfile/OpcodesValidationTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,107 +23,29 @@
 
 /*
  * @test
- * @summary Testing ClassFile constant instruction opcodes.
+ * @summary Testing ClassFile constant instruction argument validation.
  * @run junit OpcodesValidationTest
  */
-import java.lang.constant.ClassDesc;
+import java.lang.classfile.instruction.ConstantInstruction;
-import java.lang.constant.ConstantDesc;
+import org.junit.jupiter.api.Test;
-import static java.lang.constant.ConstantDescs.CD_void;
-import java.lang.constant.MethodTypeDesc;
 
-import java.lang.reflect.AccessFlag;
-import java.lang.classfile.ClassFile;
-import java.lang.classfile.Opcode;
-import org.junit.jupiter.api.*;
-import static org.junit.jupiter.api.DynamicTest.dynamicTest;
 import static org.junit.jupiter.api.Assertions.*;
 import static java.lang.classfile.Opcode.*;
-import java.util.stream.Stream;
 
-public class OpcodesValidationTest {
+class OpcodesValidationTest {
 
-    record Case(Opcode opcode, Object constant) {}
+    @Test
+    void testArgumentConstant() {
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, 0));
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, Short.MIN_VALUE));
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(SIPUSH, Short.MAX_VALUE));
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, 0));
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, Byte.MIN_VALUE));
+        assertDoesNotThrow(() -> ConstantInstruction.ofArgument(BIPUSH, Byte.MAX_VALUE));
 
-    static Stream<Case> positiveCases() {
+        assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(SIPUSH, (int)Short.MIN_VALUE - 1));
-        return Stream.of(
+        assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(SIPUSH, (int)Short.MAX_VALUE + 1));
-            new Case(ACONST_NULL, null),
+        assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(BIPUSH, (int)Byte.MIN_VALUE - 1));
-            new Case(SIPUSH, (int)Short.MIN_VALUE),
+        assertThrows(IllegalArgumentException.class, () -> ConstantInstruction.ofArgument(BIPUSH, (int)Byte.MAX_VALUE + 1));
-            new Case(SIPUSH, (int)Short.MAX_VALUE),
-            new Case(BIPUSH, (int)Byte.MIN_VALUE),
-            new Case(BIPUSH, (int)Byte.MAX_VALUE),
-            new Case(ICONST_M1, -1),
-            new Case(ICONST_0, 0),
-            new Case(ICONST_1, 1),
-            new Case(ICONST_2, 2),
-            new Case(ICONST_3, 3),
-            new Case(ICONST_4, 4),
-            new Case(ICONST_5, 5),
-            new Case(LCONST_0, 0l),
-            new Case(LCONST_0, 0),
-            new Case(LCONST_1, 1l),
-            new Case(LCONST_1, 1),
-            new Case(FCONST_0, 0.0f),
-            new Case(FCONST_1, 1.0f),
-            new Case(FCONST_2, 2.0f),
-            new Case(DCONST_0, 0.0d),
-            new Case(DCONST_1, 1.0d)
-        );
-    }
-
-    static Stream<Case> negativeCases() {
-        return Stream.of(
-            new Case(ACONST_NULL, 0),
-            new Case(SIPUSH, (int)Short.MIN_VALUE - 1),
-            new Case(SIPUSH, (int)Short.MAX_VALUE + 1),
-            new Case(BIPUSH, (int)Byte.MIN_VALUE - 1),
-            new Case(BIPUSH, (int)Byte.MAX_VALUE + 1),
-            new Case(ICONST_M1, -1l),
-            new Case(ICONST_0, 0l),
-            new Case(ICONST_1, 1l),
-            new Case(ICONST_2, 2l),
-            new Case(ICONST_3, 3l),
-            new Case(ICONST_4, 4l),
-            new Case(ICONST_5, 5l),
-            new Case(LCONST_0, null),
-            new Case(LCONST_0, 1l),
-            new Case(LCONST_1, 1.0d),
-            new Case(LCONST_1, 0),
-            new Case(FCONST_0, 0.0d),
-            new Case(FCONST_1, 1.01f),
-            new Case(FCONST_2, 2),
-            new Case(DCONST_0, 0.0f),
-            new Case(DCONST_1, 1.0f),
-            new Case(DCONST_1, 1)
-        );
-    }
-
-    @TestFactory
-    Stream<DynamicTest> testPositiveCases() {
-        return positiveCases().map(c -> dynamicTest(c.toString(), () -> testPositiveCase(c.opcode, c.constant)));
-    }
-
-    private void testPositiveCase(Opcode opcode, Object constant) {
-        ClassFile.of().build(ClassDesc.of("MyClass"),
-                        cb -> cb.withFlags(AccessFlag.PUBLIC)
-                                .withMethod("<init>", MethodTypeDesc.of(CD_void), 0,
-                                      mb -> mb.withCode(
-                                              codeb -> codeb.loadConstant(opcode, (ConstantDesc) constant))));
-    }
-
-
-    @TestFactory
-    Stream<DynamicTest> testNegativeCases() {
-        return negativeCases().map(c -> dynamicTest(
-            c.toString(),
-            () -> assertThrows(IllegalArgumentException.class, () -> testNegativeCase(c.opcode, c.constant))
-        ));
-    }
-
-    private void testNegativeCase(Opcode opcode, Object constant) {
-        ClassFile.of().build(ClassDesc.of("MyClass"),
-                        cb -> cb.withFlags(AccessFlag.PUBLIC)
-                                .withMethod("<init>", MethodTypeDesc.of(CD_void), 0,
-                        mb -> mb .withCode(
-                                codeb -> codeb.loadConstant(opcode, (ConstantDesc)constant))));
     }
 }

test/jdk/jdk/classfile/helpers/InstructionModelToCodeBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -77,7 +77,7 @@ public class InstructionModelToCodeBuilder {
             case OperatorInstruction im ->
                 cb.with(OperatorInstruction.of(im.opcode()));
             case ConstantInstruction im ->
-                cb.loadConstant(im.opcode(), im.constantValue());
+                cb.loadConstant(im.constantValue());
             case MonitorInstruction im ->
                 cb.with(MonitorInstruction.of(im.opcode()));
             case NopInstruction im ->