-
Type:
Bug
-
Resolution: Not an Issue
-
Priority:
P4
-
Affects Version/s: repo-valhalla
-
Component/s: tools
-
None
-
generic
-
generic
I asked in chat hallway:
I am observing this behavior in javac code base that automatically downgrades primitive/value classes encountered in class files to identity classes if the prevailing --source version would not allow primitive/classes in source. For example the method com.sun.tools.javac.jvm.ClassReader#adjustClassFlags which internalizes the flags from a class file's header has this:
long adjustClassFlags(long flags) {
if ((flags & ACC_MODULE) != 0) {
flags &= ~ACC_MODULE;
flags |= MODULE;
}
if ((flags & ACC_PRIMITIVE) != 0) {
flags &= ~ACC_PRIMITIVE;
if (allowPrimitiveClasses) {
flags |= PRIMITIVE_CLASS;
}
}
if ((flags & ACC_VALUE) != 0) {
flags &= ~ACC_VALUE;
if (allowValueClasses) {
flags |= VALUE_CLASS;
}
}
return flags & ~ACC_SUPER; // SUPER and SYNCHRONIZED bits overloaded
}
Where the booleans allowPrimitiveClasses and allowValueClasses are preset based on whether primitive/value classes feature is supported in the current source level. This behavior is also tested for in this manner:
/*
* @test
* @summary Check that casting to a value type involves no null check when values are not recognized in source.
*/
public class CastNoNullCheckTest {
public static void main(String... args) {
Object o = null;
Point p = (Point) o; // No NPE expected.
}
}
ATM, I am unable to relate this back to the JEP drafts or the JVM draft specs we have and unable to say whether this behavior is justified. Language/VM designers - please comment.
Dan Smith responded with:
I think the intended behavior, when we switch to using --enable-preview, is something like:
error: classfile for ./Record.class uses preview features of Java SE 15.
(use --enable-preview to allow loading of classfiles which contain preview features)
But we're a bit inconsistent on this: when the class reader sees a preview class and --enable-preview is off, you get the above error. When the class reader sees a regular class but --release targets an older version, it will treat the class as valid.
Kind of a dangerous move to make, because the class reader is somehow mapping the API of a "future" class file version into the model of an older release. The details of that mapping are totally ad hoc. (In this case, when primitive classes are final, I suppose we would want to be treating mentions of Point as L types. Or just rejecting the uses.
Having thought about it more: I think the right thing to do when --release doesn't support primitive classes, given existing javac behavior, is to quietly load the class, but treat the type Point as an error, along with any references to fields/methods with Q types in their descriptors. The idea being that we'll do what we can to support the loaded class file, but can't use new JVM features in the currently-compiling class, and want to guarantee that, should it be recompiled with a future --release, its meaning won't change.
This is in anticipation of a final feature, though. While it's a preview feature, the "uses preview features" error should occur when the class is read, as above.
I am observing this behavior in javac code base that automatically downgrades primitive/value classes encountered in class files to identity classes if the prevailing --source version would not allow primitive/classes in source. For example the method com.sun.tools.javac.jvm.ClassReader#adjustClassFlags which internalizes the flags from a class file's header has this:
long adjustClassFlags(long flags) {
if ((flags & ACC_MODULE) != 0) {
flags &= ~ACC_MODULE;
flags |= MODULE;
}
if ((flags & ACC_PRIMITIVE) != 0) {
flags &= ~ACC_PRIMITIVE;
if (allowPrimitiveClasses) {
flags |= PRIMITIVE_CLASS;
}
}
if ((flags & ACC_VALUE) != 0) {
flags &= ~ACC_VALUE;
if (allowValueClasses) {
flags |= VALUE_CLASS;
}
}
return flags & ~ACC_SUPER; // SUPER and SYNCHRONIZED bits overloaded
}
Where the booleans allowPrimitiveClasses and allowValueClasses are preset based on whether primitive/value classes feature is supported in the current source level. This behavior is also tested for in this manner:
/*
* @test
* @summary Check that casting to a value type involves no null check when values are not recognized in source.
*/
public class CastNoNullCheckTest {
public static void main(String... args) {
Object o = null;
Point p = (Point) o; // No NPE expected.
}
}
ATM, I am unable to relate this back to the JEP drafts or the JVM draft specs we have and unable to say whether this behavior is justified. Language/VM designers - please comment.
Dan Smith responded with:
I think the intended behavior, when we switch to using --enable-preview, is something like:
error: classfile for ./Record.class uses preview features of Java SE 15.
(use --enable-preview to allow loading of classfiles which contain preview features)
But we're a bit inconsistent on this: when the class reader sees a preview class and --enable-preview is off, you get the above error. When the class reader sees a regular class but --release targets an older version, it will treat the class as valid.
Kind of a dangerous move to make, because the class reader is somehow mapping the API of a "future" class file version into the model of an older release. The details of that mapping are totally ad hoc. (In this case, when primitive classes are final, I suppose we would want to be treating mentions of Point as L types. Or just rejecting the uses.
Having thought about it more: I think the right thing to do when --release doesn't support primitive classes, given existing javac behavior, is to quietly load the class, but treat the type Point as an error, along with any references to fields/methods with Q types in their descriptors. The idea being that we'll do what we can to support the loaded class file, but can't use new JVM features in the currently-compiling class, and want to guarantee that, should it be recompiled with a future --release, its meaning won't change.
This is in anticipation of a final feature, though. While it's a preview feature, the "uses preview features" error should occur when the class is read, as above.
- relates to
-
JDK-8282107 Valhalla: javac does not correctly recognize value class factory <init>
-
- Resolved
-