"Do the right thing" when compiling against value/primitive class files using a --source/--release version of earlier vintage

XMLWordPrintable

    • Type: Bug
    • Resolution: Not an Issue
    • Priority: P4
    • repo-valhalla
    • 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.

            Assignee:
            Vicente Arturo Romero Zaldivar
            Reporter:
            Srikanth Adayapalam (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: