On Mon, 7 Oct 2024 16:45:28 GMT, Luca Kellermann <d...@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
