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

Rename TypeKind enum constants to follow code style

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Approved
    • Icon: P3 P3
    • 24
    • core-libs
    • None
    • source
    • minimal
    • Changes to preview APIs have minimal compatibility risk.
    • Java API
    • SE

      Summary

      Rename TypeKind enum constants to follow code style and improve specifications.

      Problem

      TypeKind enum constants are using upper camel case UpperCamelCase, which violates Java convention of using upper snake case UPPER_SNAKE_CASE. And TypeKind is missing specifications and clarifications, such as:

      1. Subword types are represented as ints in the JVM; no mention on how they are converted.
      2. void is not really a type, and there's legacy returnAddress type not mentioned.
      3. AnnotationValue couldn't explain why different types of constant values share entries without computational types.
      4. Array load/store instructions don't mention that byte load/store also handles boolean load/stores.
      5. The typeName and descriptor methods are useless and error prone, as descriptor is not a real descriptor for objects and some object descriptors like [I</code> for <code class="prettyprint" >int[] does not start with L.

      Solution

      1. Rename TypeKind enum constants
      2. Improve TypeKind specifications, mention computational types and subword type conversions and returnAddress.
      3. Note in AnnotationValue that constant() is not sufficient for deriving annotation value due to computational types.
      4. Note that array load/store for byte is also for boolean.
      5. Removed typeName and descriptor and added ClassDesc upperBound(); users can use upperBound().displayName() and upperBound().descriptorString() instead.

      Specification

      --- a/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java
      +++ b/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java
      @@ -102,7 +102,9 @@ sealed interface OfConstant extends AnnotationValue {
                * {@return the constant pool entry backing this constant element}
                *
                * @apiNote
      -         * Different types of constant values may share the same type of entry.
      +         * Different types of constant values may share the same type of entry
      +         * because they have the same {@linkplain TypeKind##computational-type
      +         * computational type}.
                * For example, {@link OfInt} and {@link OfChar} are both
                * backed by {@link IntegerEntry}. Use {@link #resolvedValue
                * resolvedValue()} for a value of accurate type.
      --- a/src/java.base/share/classes/java/lang/classfile/Opcode.java
      +++ b/src/java.base/share/classes/java/lang/classfile/Opcode.java
      @@ -45,55 +45,55 @@ public enum Opcode {
      
      -    /** Load byte from array */
      -    BALOAD(ClassFile.BALOAD, 1, Kind.ARRAY_LOAD, TypeKind.ByteType),
      +    /** Load byte or boolean from array */
      +    BALOAD(ClassFile.BALOAD, 1, Kind.ARRAY_LOAD, TypeKind.BYTE),
      
      -    /** Store into byte array */
      -    BASTORE(ClassFile.BASTORE, 1, Kind.ARRAY_STORE, TypeKind.ByteType),
      +    /** Store into byte or boolean array */
      +    BASTORE(ClassFile.BASTORE, 1, Kind.ARRAY_STORE, TypeKind.BYTE),
      
      diff --git a/src/java.base/share/classes/java/lang/classfile/TypeKind.java b/src/java.base/share/classes/java/lang/classfile/TypeKind.java
      index 5ba566b3d06..439da412893 100644
      --- a/src/java.base/share/classes/java/lang/classfile/TypeKind.java
      +++ b/src/java.base/share/classes/java/lang/classfile/TypeKind.java
      @@ -25,107 +25,196 @@
      
       /**
      - * Describes the types that can be part of a field or method descriptor.
      + * Describes the data types Java Virtual Machine operates on.
      + * This omits {@code returnAddress} (JVMS {@jvms 2.3.3}),
      + * which is only used by discontinued {@link
      + * DiscontinuedInstruction.JsrInstruction jsr} and {@link
      + * DiscontinuedInstruction.RetInstruction ret} instructions,
      + * and includes {@link #VOID void} (JVMS {@jvms 4.3.3}), which
      + * appears as a method return type.
        *
      + * <h2 id="computational-type">Computational Type</h2>
      + * In the {@code class} file format, local variables (JVMS {@jvms 2.6.1}),
      + * and the operand stack (JVMS {@jvms 2.6.2}) of the Java Virtual Machine,
      + * {@link #BOOLEAN boolean}, {@link #BYTE byte}, {@link #CHAR char},
      + * {@link #SHORT short} types do not exist and are {@linkplain
      + * #asLoadable() represented} by the {@link #INT int} computational type.
      + * {@link #INT int}, {@link #FLOAT float}, {@link #REFERENCE reference},
      + * {@code returnAddress}, {@link #LONG long}, and {@link #DOUBLE doule}
      + * are the computational types of the Java Virtual Machine.
      + *
      + * @jvms 2.2 Data Types
      + * @jvms 2.11.1 Types and the Java Virtual Machine
        * @since 22
        */
       @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
       public enum TypeKind {
      -    /** the primitive type byte */
      -    ByteType("byte", "B", 8),
      -    /** the primitive type short */
      -    ShortType("short", "S", 9),
      -    /** the primitive type int */
      -    IntType("int", "I", 10),
      -    /** the primitive type float */
      -    FloatType("float", "F", 6),
      -    /** the primitive type long */
      -    LongType("long", "J", 11),
      -    /** the primitive type double */
      -    DoubleType("double", "D", 7),
      -    /** a reference type */
      -    ReferenceType("reference type", "L", -1),
      -    /** the primitive type char */
      -    CharType("char", "C", 5),
      -    /** the primitive type boolean */
      -    BooleanType("boolean", "Z", 4),
      -    /** void */
      -    VoidType("void", "V", -1);
      +    // Elements are grouped so frequently used switch ranges such as
      +    // primitives (boolean - double) and computational (int - void) are together.
      +    // This also follows the order of typed opcodes
      +    // Begin primitive types
      +    /**
      +     * The primitive type {@code boolean}. Its {@linkplain ##computational-type
      +     * computational type} is {@link #INT int}. {@code 0} represents {@code false},
      +     * and {@code 1} represents {@code true}. It is zero-extended to an {@code int}
      +     * when loaded onto the operand stack and narrowed by taking the bitwise AND
      +     * with {@code 1} when stored.
      +     *
      +     * @jvms 2.3.4 The {@code boolean} Type
      +     */
      +    BOOLEAN(1, 4),
      +    /**
      +     * The primitive type {@code byte}. Its {@linkplain ##computational-type
      +     * computational type} is {@link #INT int}. It is sign-extended to an
      +     * {@code int} when loaded onto the operand stack and truncated when
      +     * stored.
      +     */
      +    BYTE(1, 8),
      +    /**
      +     * The primitive type {@code char}. Its {@linkplain ##computational-type
      +     * computational type} is {@link #INT int}. It is zero-extended to an
      +     * {@code int} when loaded onto the operand stack and truncated when
      +     * stored.
      +     */
      +    CHAR(1, 5),
      +    /**
      +     * The primitive type {@code short}. Its {@linkplain ##computational-type
      +     * computational type} is {@link #INT int}. It is sign-extended to an
      +     * {@code int} when loaded onto the operand stack and truncated when
      +     * stored.
      +     */
      +    SHORT(1, 9),
      +    // Begin computational types
      +    /**
      +     * The primitive type {@code int}.
      +     */
      +    INT(1, 10),
      +    /**
      +     *  The primitive type {@code long}. It is of {@linkplain #slotSize() category} 2.
      +     */
      +    LONG(2, 11),
      +    /**
      +     * The primitive type {@code float}.
      +     */
      +    FLOAT(1, 6),
      +    /**
      +     * The primitive type {@code double}. It is of {@linkplain #slotSize() category} 2.
      +     */
      +    DOUBLE(2, 7),
      +    // End primitive types
      +    /**
      +     * A reference type.
      +     * @jvms 2.4 Reference Types and Values
      +     */
      +    REFERENCE(1, -1),
      +    /**
      +     * The {@code void} type, for absence of a value. While this is not a data type,
      +     * this can be a method return type indicating no change in {@linkplain #slotSize()
      +     * operand stack depth}.
      +     *
      +     * @jvms 4.3.3 Method Descriptors
      +     */
      +    VOID(0, -1);
      +    // End computational types
      
      -    /** {@return the human-readable name corresponding to this type} */
      -    public String typeName() { return name; }
      
      -    /** {@return the field descriptor character corresponding to this type} */
      -    public String descriptor() { return descriptor; }
      +    /**
      +     * {@return the most specific upper bound field descriptor that can store any value
      +     * of this type} This is the primitive class descriptor for primitive types and
      +     * {@link #VOID void} and {@link ConstantDescs#CD_Object Object} descriptor for
      +     * {@link #REFERENCE reference}.
      +     */
      +    public ClassDesc upperBound() {
      
           /**
      -     * {@return the code used by the {@code newarray} opcode corresponding to this type}
      +     * {@return the code used by the {@link Opcode#NEWARRAY newarray} instruction to create an array
      +     * of this component type, or {@code -1} if this type is not supported by {@code newarray}}
            * @since 23
      +     * @jvms 6.5.newarray <i>newarray</i>
            */
           public int newarrayCode() {
      
           /**
      -     * {@return the number of local variable slots consumed by this type}
      +     * {@return the number of local variable index or operand stack depth consumed by this type}
      +     * This is also the category of this type for instructions operating on the operand stack without
      +     * regard to type (JVMS {@jvms 2.11.1}), such as {@link Opcode#POP pop} versus {@link Opcode#POP2
      +     * pop2}.
      +     * @jvms 2.6.1 Local Variables
      +     * @jvms 2.6.2 Operand Stacks
            */
           public int slotSize() {
      
           /**
      -     * Erase this type kind to the type which will be used for xLOAD, xSTORE,
      -     * and xRETURN bytecodes
      -     * @return the erased type kind
      +     * {@return the {@linkplain ##computational-type computational type} for this type, or {@link #VOID void}
      +     * for {@code void}}
            */
           public TypeKind asLoadable() {
      
           /**
      -     * {@return the type kind associated with the array type described by the
      -     * array code used as an operand to {@code newarray}}
      +     * {@return the component type described by the array code used as an operand to {@link Opcode#NEWARRAY
      +     * newarray}}
            * @param newarrayCode the operand of the {@code newarray} instruction
            * @throws IllegalArgumentException if the code is invalid
            * @since 23
      +     * @jvms 6.5.newarray <i>newarray</i>
            */
           public static TypeKind fromNewarrayCode(int newarrayCode) {
      
           /**
      -     * {@return the type kind associated with the specified field descriptor}
      +     * {@return the type associated with the specified field descriptor}
            * @param s the field descriptor
            * @throws IllegalArgumentException only if the descriptor is not valid
            */
      @@ -134,27 +223,27 @@ public static TypeKind fromDescriptor(CharSequence s) {
      
           /**
      -     * {@return the type kind associated with the specified field descriptor}
      +     * {@return the type associated with the specified field descriptor}
            * @param descriptor the field descriptor
            */
           public static TypeKind from(TypeDescriptor.OfField<?> descriptor) {
      
      --- a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java
      +++ b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayLoadInstruction.java
      @@ -45,7 +45,9 @@
       public sealed interface ArrayLoadInstruction extends Instruction
               permits AbstractInstruction.UnboundArrayLoadInstruction {
           /**
      -     * {@return the component type of the array}
      +     * {@return the component type of the array} The {@link TypeKind#BYTE byte}
      +     * type load instruction {@link Opcode#BALOAD baload} also operates on
      +     * {@link TypeKind#BOOLEAN boolean} arrays.
            */
           TypeKind typeKind();
      
      --- a/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java
      +++ b/src/java.base/share/classes/java/lang/classfile/instruction/ArrayStoreInstruction.java
      @@ -45,7 +45,9 @@
       public sealed interface ArrayStoreInstruction extends Instruction
               permits AbstractInstruction.UnboundArrayStoreInstruction {
           /**
      -     * {@return the component type of the array}
      +     * {@return the component type of the array} The {@link TypeKind#BYTE byte}
      +     * type store instruction {@link Opcode#BASTORE bastore} also operates on
      +     * {@link TypeKind#BOOLEAN boolean} arrays.
            */
           TypeKind typeKind();

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

              Created:
              Updated:
              Resolved: