Unwind abstract enum kludge

scala · Jun 19, 2023 · 11db53c · 11db53c
1 parent 8d5cb07
commit 11db53c
Show file tree

Hide file tree

Showing 15 changed files with 37 additions and 87 deletions.
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
@@ -305,24 +305,16 @@ abstract class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
     def javaClassfileFlags(classSym: Symbol): Int = {
       assert(classSym.isJava, s"Expected Java class symbol, got ${classSym.fullName}")
       import asm.Opcodes._
-      def enumFlags = ACC_ENUM | {
-        // Java enums have the `ACC_ABSTRACT` flag if they have a deferred method.
-        // We cannot trust `hasAbstractFlag`: the ClassfileParser adds `ABSTRACT` and `SEALED` to all
-        // Java enums for exhaustiveness checking.
-        val hasAbstractMethod = classSym.info.decls.exists(s => s.isMethod && s.isDeferred)
-        if (hasAbstractMethod) ACC_ABSTRACT else 0
-      }
-        // scala/bug#9393: the classfile / java source parser make java annotation symbols look like classes.
-        // here we recover the actual classfile flags.
-        ( if (classSym.hasJavaAnnotationFlag)                        ACC_ANNOTATION | ACC_INTERFACE | ACC_ABSTRACT else 0) |
-      ( if (classSym.isPublic)                                     ACC_PUBLIC    else 0) |
-      ( if (classSym.isFinal)                                      ACC_FINAL     else 0) |
+      // scala/bug#9393: the classfile / java source parser make java annotation symbols look like classes.
+      // here we recover the actual classfile flags.
+      (if (classSym.hasJavaAnnotationFlag) ACC_ANNOTATION | ACC_INTERFACE | ACC_ABSTRACT else 0) |
+      (if (classSym.isPublic)              ACC_PUBLIC    else 0) |
+      (if (classSym.isFinal)               ACC_FINAL     else 0) |
        // see the link above. javac does the same: ACC_SUPER for all classes, but not interfaces.)
-      ( if (classSym.isInterface)                                  ACC_INTERFACE else ACC_SUPER) |
-       // for Java enums, we cannot trust `hasAbstractFlag` (see comment in enumFlags))
-      ( if (!classSym.hasJavaEnumFlag && classSym.hasAbstractFlag) ACC_ABSTRACT  else 0) |
-      ( if (classSym.isArtifact)                                   ACC_SYNTHETIC else 0) |
-      ( if (classSym.hasJavaEnumFlag)                              enumFlags     else 0)
+      (if (classSym.isInterface)           ACC_INTERFACE else ACC_SUPER) |
+      (if (classSym.hasAbstractFlag)       ACC_ABSTRACT  else 0) |
+      (if (classSym.isArtifact)            ACC_SYNTHETIC else 0) |
+      (if (classSym.hasJavaEnumFlag)       ACC_ENUM      else 0)
     }
 
     // Check for hasAnnotationFlag for scala/bug#9393: the classfile / java source parsers add

diff --git a/src/compiler/scala/tools/nsc/javac/JavaParsers.scala b/src/compiler/scala/tools/nsc/javac/JavaParsers.scala
@@ -1030,11 +1030,14 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
       accept(RBRACE)
       val superclazz =
         AppliedTypeTree(javaLangDot(tpnme.Enum), List(enumType))
+      val hasAbstractMember = body.exists {
+        case m: MemberDef => m.mods.hasFlag(Flags.DEFERRED)
+        case _ => false
+      }
       val finalFlag = if (enumIsFinal) Flags.FINAL else 0L
+      val abstractFlag = if (hasAbstractMember) Flags.ABSTRACT else 0L
       addCompanionObject(consts ::: statics ::: predefs, atPos(pos) {
-        // Marking the enum class SEALED | ABSTRACT enables exhaustiveness checking. See also ClassfileParser.
-        // This is a bit of a hack and requires excluding the ABSTRACT flag in the backend, see method javaClassfileFlags.
-        ClassDef(mods | Flags.JAVA_ENUM | Flags.SEALED | Flags.ABSTRACT | finalFlag, name, List(),
+        ClassDef(mods | Flags.JAVA_ENUM | Flags.SEALED | abstractFlag | finalFlag, name, List(),
                  makeTemplate(superclazz :: interfaces, body))
       })
     }

diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -598,10 +598,6 @@ abstract class ClassfileParser(reader: ReusableInstance[ReusableDataReader]) {
           case NoSymbol =>
             devWarning(s"no linked class for java enum $sym in ${sym.owner}. A referencing class file might be missing an InnerClasses entry.")
           case enumClass =>
-            // Marking the enum class SEALED | ABSTRACT enables exhaustiveness checking. See also JavaParsers.
-            // This is a bit of a hack and requires excluding the ABSTRACT flag in the backend, see method javaClassfileFlags.
-            // Java enums may be already sealed by virtue of permittedSubclasses, if an element had a body.
-            enumClass.setFlag(SEALED | ABSTRACT)
             enumClass.addChild(sym)
         }
     }

diff --git a/src/reflect/scala/reflect/internal/ClassfileConstants.scala b/src/reflect/scala/reflect/internal/ClassfileConstants.scala
@@ -353,7 +353,7 @@ object ClassfileConstants {
       case JAVA_ACC_STATIC     => STATIC
       case JAVA_ACC_ABSTRACT   => if (isClass) ABSTRACT else DEFERRED
       case JAVA_ACC_INTERFACE  => TRAIT | INTERFACE | ABSTRACT
-      case JAVA_ACC_ENUM       => JAVA_ENUM
+      case JAVA_ACC_ENUM       => if (isClass) JAVA_ENUM | SEALED else JAVA_ENUM
       case JAVA_ACC_ANNOTATION => JAVA_ANNOTATION
       case _                   => 0L
     }

diff --git a/test/files/neg/sealed-java-enums.scala b/test/files/neg/sealed-java-enums.scala
@@ -1,4 +1,4 @@
-// scalac: -Xfatal-warnings
+// scalac: -Werror
 //
 import java.lang.Thread.State
 import java.lang.Thread.State._

diff --git a/test/files/neg/t8700b-new.check b/test/files/neg/t8700b-new.check
diff --git a/test/files/neg/t8700b-new/Bar_2.scala b/test/files/neg/t8700b-new/Bar_2.scala
diff --git a/test/files/neg/t8700b-new/Baz_1.java b/test/files/neg/t8700b-new/Baz_1.java
diff --git a/test/files/neg/t8700b-new/Foo_1.java b/test/files/neg/t8700b-new/Foo_1.java
diff --git a/test/files/neg/t8700b-old/Bar_2.scala b/test/files/neg/t8700b-old/Bar_2.scala
diff --git a/test/files/neg/t8700b-old/Foo_1.java b/test/files/neg/t8700b-old/Foo_1.java
diff --git a/test/files/neg/t8700b-old.check → test/files/neg/t8700b.check b/test/files/neg/t8700b-old.check → test/files/neg/t8700b.check
@@ -1,11 +1,11 @@
 Bar_2.scala:4: warning: match may not be exhaustive.
 It would fail on the following input: B
-  def bar1(foo: Foo_1) = foo match {
-                         ^
+  def bar1(foo: Foo) = foo match {
+                       ^
 Bar_2.scala:8: warning: match may not be exhaustive.
 It would fail on the following input: B
-  def bar2(foo: Baz_1) = foo match {
-                         ^
+  def bar2(foo: Baz) = foo match {
+                       ^
 error: No warnings can be incurred under -Werror.
 2 warnings
 1 error
diff --git a/test/files/neg/t8700b/Bar_2.scala b/test/files/neg/t8700b/Bar_2.scala
@@ -0,0 +1,11 @@
+// scalac: -Werror
+//
+object Bar {
+  def bar1(foo: Foo) = foo match {
+    case Foo.A => 1
+  }
+
+  def bar2(foo: Baz) = foo match {
+    case Baz.A => 1
+  }
+}
diff --git a/test/files/neg/t8700b-old/Baz_1.java → test/files/neg/t8700b/Baz.java b/test/files/neg/t8700b-old/Baz_1.java → test/files/neg/t8700b/Baz.java
@@ -1,5 +1,4 @@
-// javaVersion: 8
-public enum Baz_1 {
+public enum Baz {
     A {
     	public void baz1() {}
     },

diff --git a/test/files/neg/t8700b/Foo.java b/test/files/neg/t8700b/Foo.java
@@ -0,0 +1,4 @@
+public enum Foo {
+    A,
+    B
+}