Unwind abstract enum kludge

scala · Jun 9, 2023 · 51ce3e8 · 51ce3e8
1 parent 40b7585
commit 51ce3e8
Show file tree

Hide file tree

Showing 17 changed files with 60 additions and 84 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,17 @@ 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) |
+      // 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.hasAbstractFlag) ACC_ABSTRACT  else 0) |
       ( if (classSym.isArtifact)                                   ACC_SYNTHETIC else 0) |
-      ( if (classSym.hasJavaEnumFlag)                              enumFlags     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 abbie = body.exists {
+        case m: MemberDef => m.mods.hasFlag(Flags.DEFERRED)
+        case _ => false
+      }
       val finalFlag = if (enumIsFinal) Flags.FINAL else 0L
+      val abstractFlag = if (abbie) 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/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
@@ -129,7 +129,7 @@ trait TreeAndTypeAnalysis extends Debugging {
       def filterAndSortChildren(children: Set[Symbol]) = {
         // symbols which are both sealed and abstract need not be covered themselves,
         // because all of their children must be and they cannot otherwise be created.
-        val children1 = children.toList.filterNot(child => child.isSealed && child.isAbstractClass).sortBy(_.sealedSortName)
+        val children1 = children.toList.filterNot(child => child.isSealed && child.isAbstractClass || child.isSealedEnum).sortBy(_.sealedSortName)
         children1.filterNot { child =>
           // remove private abstract children that are superclasses of other children, for example in t6159 drop X2
           child.isPrivate && child.isAbstractClass && children1.exists(sym => (sym ne child) && sym.isSubClass(child))
@@ -874,7 +874,7 @@ trait MatchAnalysis extends MatchApproximation {
                       case args                            => args
                     }.map(ListExample)
                   case _ if isTupleSymbol(cls)                  => args(brevity = true).map(TupleExample)
-                  case _ if cls.isSealed && cls.isAbstractClass =>
+                  case _ if cls.isSealed && cls.isAbstractClass || cls.isSealedEnum =>
                     // don't report sealed abstract classes, since
                     // 1) they can't be instantiated
                     // 2) we are already reporting any missing subclass (since we know the full domain)

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/package.scala b/src/compiler/scala/tools/nsc/transform/patmat/package.scala
@@ -0,0 +1,25 @@
+/*
+ * Scala (https://www.scala-lang.org)
+ *
+ * Copyright EPFL and Lightbend, Inc.
+ *
+ * Licensed under Apache License 2.0
+ * (http://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package scala.tools.nsc.transform
+
+import scala.reflect.internal.Symbols
+
+package object patmat {
+  private[patmat] implicit class `symbol helpers`(val symbol: Symbols#Symbol) extends AnyVal {
+
+    //def isAbstractClassOrEnum: Boolean = symbol.isAbstractClass || symbol.hasJavaEnumFlag
+    //def isSealedOrEnum: Boolean = symbol.isSealed || symbol.hasJavaEnumFlag
+    // Distinguish the Enum<T> class from its instances, which are also marked as JAVA_ENUM.
+    def isSealedEnum: Boolean = symbol.isSealed && symbol.hasJavaEnumFlag
+  }
+}
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
+}