JDK-8329501 : Undocumented exception thrown by Instruction factory methods accepting Opcode
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.lang.classfile
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 23
  • Submitted: 2024-04-02
  • Updated: 2024-05-06
  • Resolved: 2024-04-15
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Document exceptions thrown by instructions factory methods.

Problem
-------

`IllegalArgumentException` thrown by some static factory methods of the following `java.lang.classfile.instruction` interfaces are not documented:

- `ArrayLoadInstruction`
- `ArrayStoreInstruction`
- `BranchInstruction`
- `ConstantInstruction`
- `ConvertInstruction`
- `DiscontinuedInstruction`
- `FieldInstruction`
- `InvokeInstruction`
- `LoadInstruction`
- `MonitorInstruction`
- `NewPrimitiveArrayInstruction`
- `OperatorInstruction`
- `ReturnInstruction`
- `StackInstruction`
- `StoreInstruction`
- `TypeCheckInstruction`

`NewPrimitiveArrayInstruction::of` factory method also does not perform the check for invalid argument.

Solution
--------

Fix `NewPrimitiveArrayInstruction::of` factory method and add all missing `@throws` Javadoc tags.

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

```
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java
index 876cc403727e0..3502e87edda25 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java
@@ -54,6 +54,8 @@ public sealed interface ArrayLoadInstruction extends Instruction
      *
      * @param op the opcode for the specific type of array load instruction,
      *           which must be of kind {@link Opcode.Kind#ARRAY_LOAD}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#ARRAY_LOAD}.
      */
     static ArrayLoadInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.ARRAY_LOAD);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java
index 10430f8ec3ad6..a85b994181aca 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java
@@ -54,6 +54,8 @@ public sealed interface ArrayStoreInstruction extends Instruction
      *
      * @param op the opcode for the specific type of array store instruction,
      *           which must be of kind {@link Opcode.Kind#ARRAY_STORE}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#ARRAY_STORE}.
      */
     static ArrayStoreInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.ARRAY_STORE);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/BranchInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/BranchInstruction.java
index e391b24cefbac..6ee47b7fbc2c1 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/BranchInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/BranchInstruction.java
@@ -56,6 +56,8 @@ public sealed interface BranchInstruction extends Instruction
      * @param op the opcode for the specific type of branch instruction,
      *           which must be of kind {@link Opcode.Kind#BRANCH}
      * @param target the target of the branch
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#BRANCH}.
      */
     static BranchInstruction of(Opcode op, Label target) {
         Util.checkKind(op, Opcode.Kind.BRANCH);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java
index 6052946a752c4..a7ba9e2a6a10f 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java
@@ -131,6 +131,8 @@ default TypeKind typeKind() {
      *
      * @param op the opcode for the specific type of intrinsic constant instruction,
      *           which must be of kind {@link Opcode.Kind#CONSTANT}
+     * @throws IllegalArgumentException if the opcode does not represent a constant
+     *                                  with implicit value
      */
     static IntrinsicConstantInstruction ofIntrinsic(Opcode op) {
         Util.checkKind(op, Opcode.Kind.CONSTANT);
@@ -145,6 +147,8 @@ 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 value the constant value
+     * @throws IllegalArgumentException if the opcode is not {@link Opcode#BIPUSH}
+     *                                  or {@link Opcode#SIPUSH}
      */
     static ArgumentConstantInstruction ofArgument(Opcode op, int value) {
         Util.checkKind(op, Opcode.Kind.CONSTANT);
@@ -159,6 +163,8 @@ static ArgumentConstantInstruction ofArgument(Opcode op, int value) {
      * @param op the opcode for the specific type of load constant instruction,
      *           which must be of kind {@link Opcode.Kind#CONSTANT}
      * @param constant the constant value
+     * @throws IllegalArgumentException if the opcode is not {@link Opcode#LDC},
+     *                                  {@link Opcode#LDC_W}, or {@link Opcode#LDC2_W}
      */
     static LoadConstantInstruction ofLoad(Opcode op, LoadableConstantEntry constant) {
         Util.checkKind(op, Opcode.Kind.CONSTANT);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ConvertInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ConvertInstruction.java
index 5a21b91ea8cba..daa7e60248470 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/ConvertInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/ConvertInstruction.java
@@ -70,6 +70,8 @@ static ConvertInstruction of(TypeKind fromType, TypeKind toType) {
      *
      * @param op the opcode for the specific type of conversion instruction,
      *           which must be of kind {@link Opcode.Kind#CONVERT}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#CONVERT}.
      */
     static ConvertInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.CONVERT);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/DiscontinuedInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/DiscontinuedInstruction.java
index d6f2c50a8faa9..61ea14c459884 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/DiscontinuedInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/DiscontinuedInstruction.java
@@ -68,6 +68,8 @@ sealed interface JsrInstruction extends DiscontinuedInstruction
          * @param op the opcode for the specific type of JSR instruction,
          *           which must be of kind {@link Opcode.Kind#DISCONTINUED_JSR}
          * @param target target label of the subroutine
+         * @throws IllegalArgumentException if the opcode kind is not
+         *         {@link Opcode.Kind#DISCONTINUED_JSR}.
          */
         static JsrInstruction of(Opcode op, Label target) {
             Util.checkKind(op, Opcode.Kind.DISCONTINUED_JSR);
@@ -109,6 +111,8 @@ sealed interface RetInstruction extends DiscontinuedInstruction
          * @param op the opcode for the specific type of RET instruction,
          *           which must be of kind {@link Opcode.Kind#DISCONTINUED_RET}
          * @param slot the local variable slot to load return address from
+         * @throws IllegalArgumentException if the opcode kind is not
+         *         {@link Opcode.Kind#DISCONTINUED_RET}.
          */
         static RetInstruction of(Opcode op, int slot) {
             Util.checkKind(op, Opcode.Kind.DISCONTINUED_RET);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/FieldInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/FieldInstruction.java
index 3d40866e194a4..43dbcccf75eaa 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/FieldInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/FieldInstruction.java
@@ -89,6 +89,8 @@ default ClassDesc typeSymbol() {
      * @param op the opcode for the specific type of field access instruction,
      *           which must be of kind {@link Opcode.Kind#FIELD_ACCESS}
      * @param field a constant pool entry describing the field
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#FIELD_ACCESS}.
      */
     static FieldInstruction of(Opcode op, FieldRefEntry field) {
         Util.checkKind(op, Opcode.Kind.FIELD_ACCESS);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/InvokeInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/InvokeInstruction.java
index a1cd11d55e882..79a2142eb6592 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/InvokeInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/InvokeInstruction.java
@@ -104,6 +104,8 @@ default MethodTypeDesc typeSymbol() {
      * @param op the opcode for the specific type of invocation instruction,
      *           which must be of kind {@link Opcode.Kind#INVOKE}
      * @param method a constant pool entry describing the method
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#INVOKE}.
      */
     static InvokeInstruction of(Opcode op, MemberRefEntry method) {
         Util.checkKind(op, Opcode.Kind.INVOKE);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/LoadInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/LoadInstruction.java
index 2a8605e72b1e3..f4cc8bab794fb 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/LoadInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/LoadInstruction.java
@@ -73,6 +73,8 @@ static LoadInstruction of(TypeKind kind, int slot) {
      * @param op the opcode for the specific type of load instruction,
      *           which must be of kind {@link Opcode.Kind#LOAD}
      * @param slot the local variable slot to load from
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#LOAD}.
      */
     static LoadInstruction of(Opcode op, int slot) {
         Util.checkKind(op, Opcode.Kind.LOAD);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/MonitorInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/MonitorInstruction.java
index 72c9aa8c438bc..d0148b34aee65 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/MonitorInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/MonitorInstruction.java
@@ -48,6 +48,8 @@ public sealed interface MonitorInstruction extends Instruction
      *
      * @param op the opcode for the specific type of monitor instruction,
      *           which must be of kind {@link Opcode.Kind#MONITOR}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#MONITOR}.
      */
     static MonitorInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.MONITOR);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java
index 4ef7049edc8b0..a67a0f21c0883 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java
@@ -51,8 +51,14 @@ public sealed interface NewPrimitiveArrayInstruction extends Instruction
      * {@return a new primitive array instruction}
      *
      * @param typeKind the component type of the array
+     * @throws IllegalArgumentException when the {@code typeKind} is not a legal
+     *                                  primitive array component type
      */
     static NewPrimitiveArrayInstruction of(TypeKind typeKind) {
+        // Implicit null-check:
+        if (typeKind.newarraycode() < 0) {
+            throw new IllegalArgumentException("Illegal component type: " + typeKind.typeName());
+        }
         return new AbstractInstruction.UnboundNewPrimitiveArrayInstruction(typeKind);
     }
 }
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/OperatorInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/OperatorInstruction.java
index ce34e61a6951b..aeaa4109d7bd6 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/OperatorInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/OperatorInstruction.java
@@ -54,6 +54,8 @@ public sealed interface OperatorInstruction extends Instruction
      *
      * @param op the opcode for the specific type of array load instruction,
      *           which must be of kind {@link Opcode.Kind#OPERATOR}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#OPERATOR}.
      */
     static OperatorInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.OPERATOR);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/ReturnInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/ReturnInstruction.java
index 9dd7ddf6b425e..fbfd22210d3dd 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/ReturnInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/ReturnInstruction.java
@@ -65,6 +65,8 @@ static ReturnInstruction of(TypeKind typeKind) {
      *
      * @param op the opcode for the specific type of return instruction,
      *           which must be of kind {@link Opcode.Kind#RETURN}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#RETURN}.
      */
     static ReturnInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.RETURN);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/StackInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/StackInstruction.java
index e66f9c5a3dbf8..9b2e0b6904387 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/StackInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/StackInstruction.java
@@ -49,6 +49,8 @@ public sealed interface StackInstruction extends Instruction
      *
      * @param op the opcode for the specific type of stack instruction,
      *           which must be of kind {@link Opcode.Kind#STACK}
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#STACK}.
      */
     static StackInstruction of(Opcode op) {
         Util.checkKind(op, Opcode.Kind.STACK);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/StoreInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/StoreInstruction.java
index 9eb45c0b5d4ac..278bf8c0ec399 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/StoreInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/StoreInstruction.java
@@ -72,6 +72,8 @@ static StoreInstruction of(TypeKind kind, int slot) {
      * @param op the opcode for the specific type of store instruction,
      *           which must be of kind {@link Opcode.Kind#STORE}
      * @param slot the local variable slot to store to
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#STORE}.
      */
     static StoreInstruction of(Opcode op, int slot) {
         Util.checkKind(op, Opcode.Kind.STORE);
diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/TypeCheckInstruction.java b/src/java.base/share/classes/java/lang/classfile/instruction/TypeCheckInstruction.java
index b4107a528d019..d93f5244ca962 100644
--- a/src/java.base/share/classes/java/lang/classfile/instruction/TypeCheckInstruction.java
+++ b/src/java.base/share/classes/java/lang/classfile/instruction/TypeCheckInstruction.java
@@ -59,6 +59,8 @@ public sealed interface TypeCheckInstruction extends Instruction
      * @param op the opcode for the specific type of type check instruction,
      *           which must be of kind {@link Opcode.Kind#TYPE_CHECK}
      * @param type the type against which to check or cast
+     * @throws IllegalArgumentException if the opcode kind is not
+     *         {@link Opcode.Kind#TYPE_CHECK}.
      */
     static TypeCheckInstruction of(Opcode op, ClassEntry type) {
         Util.checkKind(op, Opcode.Kind.TYPE_CHECK);
diff --git a/src/java.base/share/classes/java/lang/classfile/package-info.java b/src/java.base/share/classes/java/lang/classfile/package-info.java
index 39244e98cfc..2b7a9cbb9cb 100644
--- a/src/java.base/share/classes/java/lang/classfile/package-info.java
+++ b/src/java.base/share/classes/java/lang/classfile/package-info.java
@@ -232,6 +232,12 @@
  * the convenience method {@code CodeBuilder.invokeInstruction}, which in turn behaves
  * as if it calls method {@code CodeBuilder.with}. This composing of method calls on the
  * builder enables the composing of transforms (as described later).
+ * <p>
+ * Unless otherwise noted, passing a {@code null} argument to a constructor
+ * or method of any Class-File API class or interface will cause a {@link
+ * java.lang.NullPointerException NullPointerException} to be thrown. Additionally,
+ * invoking a method with an array or collection containing a {@code null} element
+ * will cause a {@code NullPointerException}, unless otherwise specified. </p>
  *
  * <h3>Symbolic information</h3>
  * To describe symbolic information for classes and types, the API uses the
```

 
Comments
Moving to Approved.
15-04-2024

Thank you for pointing out the NPE. I've added relevant statement to the package-info.
09-04-2024

Moving back to Provisional. [~asotona], before this is Finalized, please have another engineer review the CSR. A point to consider: for null pointer handling, we'll often have a class-level of package level statement "Methods throw NPE on a null argument unless otherwise documented, etc." rather than including `@throws NPE` clauses in every method's javadoc. Did you consider making an analagous high-level blanket statement here? HTH
04-04-2024

Moving to Provisional, not Approved. I assume this is intended for JDK 23 as part of the general class file API work.
02-04-2024