RFR: 8339260: Move rarely used constants out of ClassFile [v9]
Chen Liang
liach at openjdk.org
Mon Oct 7 17:10:39 UTC 2024
On Mon, 7 Oct 2024 16:45:28 GMT, Luca Kellermann <duke at openjdk.org> wrote:
>> First, some bit of history. These API methods are added before those constants, and this patch did not change the type of these methods or constants.
>>
>> I believe the methods have restricted return types to be informative: `tag()` is a U1, but it can be interpreted as a char (see [JVMS 4.7.16.1](https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-4.html#jvms-4.7.16.1)) and thus is returned as one. CP tag is a U1 too (see [JVMS 4.4](https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-4.html#jvms-4.7.16.1)) so maybe `byte` is just used out of convenience.
>>
>> However, we usually use `int` to represent U1, also called unsigned bytes. See [`DataInput::readUnsignedByte`](https://github.com/openjdk/jdk/blob/c43202baf6eb7e49ec458037971a9efa392d053e/src/java.base/share/classes/java/io/DataInput.java#L323) and [`ClassReader::readU1`](https://github.com/openjdk/jdk/blob/c43202baf6eb7e49ec458037971a9efa392d053e/src/java.base/share/classes/java/lang/classfile/ClassReader.java#L133). Guess that's why we used `int` for the constants. (Also, note that byte and char are all [erased to int](https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/lang/classfile/TypeKind.html#computational-type) in bytecode, so this has little practical effect and that might be why I didn't bother to change these)
>
>> First, some bit of history. These API methods are added before those constants, and this patch did not change the type of these methods or constants.
>
> Sure, makes sense to separate this patch from a potential type change of the methods/constants.
>
>> I believe the methods have restricted return types to be informative: `tag()` is a U1, but it can be interpreted as a char (see [JVMS 4.7.16.1](https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-4.html#jvms-4.7.16.1)) and thus is returned as one. CP tag is a U1 too (see [JVMS 4.4](https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-4.html#jvms-4.7.16.1)) so maybe `byte` is just used out of convenience.
>
> I think `byte` and `char` as the types are a good choice given the way these tag items are described in the JVMS.
>
>> However, we usually use `int` to represent U1, also called unsigned bytes. See [`DataInput::readUnsignedByte`](https://github.com/openjdk/jdk/blob/c43202baf6eb7e49ec458037971a9efa392d053e/src/java.base/share/classes/java/io/DataInput.java#L323) and [`ClassReader::readU1`](https://github.com/openjdk/jdk/blob/c43202baf6eb7e49ec458037971a9efa392d053e/src/java.base/share/classes/java/lang/classfile/ClassReader.java#L133). Guess that's why we used `int` for the constants. (Also, note that byte and char are all [erased to int](https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/lang/classfile/TypeKind.html#computational-type) in bytecode, so this has little practical effect and that might be why I didn't bother to change these)
>
> One scenario where the unequal types of the methods and constants _does_ have a practical effect are JVM languages that don't have [binary numeric promotion](https://docs.oracle.com/javase/specs/jls/se23/html/jls-5.html#jls-5.6) for their [numerical equality operators](https://docs.oracle.com/javase/specs/jls/se23/html/jls-15.html#jls-15.21.1).
>
> This is the case for Kotlin's [value equality expressions](https://kotlinlang.org/spec/expressions.html#value-equality-expressions). This Kotlin code [would not compile](https://pl.kotl.in/A9D1WDdS8):
>
> fun isArray(a: AnnotationValue): Boolean {
> return a.tag() == AnnotationValue.TAG_ARRAY
> }
>
> Instead you would have to write [this code](https://pl.kotl.in/qSrwSIL39):
>
> fun isArray(a: AnnotationValue): Boolean {
> return a.tag() == AnnotationValue.TAG_ARRAY.toChar()
> }
Your example is an exact antipattern from our data-oriented model: we would want users to check the object type with `instanceof` (should be `is` in kotlin) instead of checking these constants.
Yet I think we can consider promoting Constant Pool tag from byte or char, short, or int to represent a u1 in case it goes over 127.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20773#discussion_r1790591290
More information about the core-libs-dev
mailing list