SI-8058 Better support for enum trees

Replace the approach of “detect some magic combination of flags to
trigger some action” by introducing an enum flag which makes the
semantics and the intentions of the code using it more explicit.

This basically involves wiring up the existing ACC_ENUM bit to a new
modifier flag and making sure it is set correctly when encountering
enums.

The existing enum tests files/pos/t5165 and files/pos/t2764 keep
working, showing that this hasn't introduced any obvious regressions.

Tests for the changes in Namer which prevent scalac from messing with
enum trees can't be added yet, because one currently can't define an
enum in Scala without the help of the macro paradise.
The intention is to add the @enum macro as a full test suite as soon
as one can depend on macro annotations.

One might wonder why we don't check for
    clazz.superClass == JavaEnumClass (where clazz is the owning class)
in isEnumConstant.

The reason is that this causes illegal cyclic reference error.

Explanation by Eugene why this happens:

(23:17:52) xeno_by: so here's what happens as far as I can understand at 11pm :)
(23:18:09) xeno_by: scalac tries to complete the signature of the newly expanded class
(23:18:11) xeno_by: to do that
(23:18:40) xeno_by: to do that it needs three things
(23:18:51) xeno_by: because the signature of a class is ClassInfoType which consists of three things
(23:19:05) xeno_by: parents
(23:19:08) xeno_by: decls
(23:19:09) xeno_by: and symbol
(23:19:20) xeno_by: symbol is easy - it's already there
(23:19:30) xeno_by: parents are also easy
(23:19:39) xeno_by: you just typecheck the things that come after "extends"
(23:19:42) xeno_by: but decls are tricky
(23:19:51) xeno_by: scalac goes through all the members of the class
(23:20:03) xeno_by: and doesn't typecheck them... no, it doesn't
(23:20:07) xeno_by: it just enters them
(23:20:32) xeno_by: i.e. creates symbols for them and assigns lazy completers to those symbols so that if someone wants to know their signatures, they will go through the completers
(23:20:34) xeno_by: and then
(23:20:38) xeno_by: wait
(23:20:40) xeno_by: there's one but
(23:20:42) xeno_by: BUT
(23:20:47) xeno_by: while we enter those symbols
(23:20:53) xeno_by: our ClassInfoType is not ready yet
(23:21:09) xeno_by: the class we're completing is still considered to be in the middle of being completing
(23:21:12) xeno_by: so
(23:21:24) xeno_by: when inside enterSym you try to ask that class for its super class
(23:21:35) xeno_by: what happens is that check asks the class for its type signature
(23:21:45) xeno_by: the ClassInfoType that consists of parents and decls
(23:21:54) xeno_by: even though the parents are already calculated
(23:22:01) xeno_by: the ClassInfoType as a whole is not
(23:22:16) xeno_by: so scalac says that you're trying to complete something that's currently being completed
(23:22:20) xeno_by: cyclic reference error
(23:22:59) xeno_by: "cyclic" in English looks an awful lot like "суслик" in Russian (which means "gopher")

Author: soc

src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala

@@ -835,6 +835,7 @@ abstract class BCodeTypes extends BCodeIdiomatic {
       if (sym.isBridge) ACC_BRIDGE | ACC_SYNTHETIC else 0,
       if (sym.isArtifact) ACC_SYNTHETIC else 0,
       if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
+      if (sym.hasEnumFlag) ACC_ENUM else 0,
       if (sym.isVarargsMethod) ACC_VARARGS else 0,
       if (sym.hasFlag(symtab.Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0
     )

src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala

@@ -244,6 +244,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
       if (sym.isBridge) ACC_BRIDGE | ACC_SYNTHETIC else 0,
       if (sym.isArtifact) ACC_SYNTHETIC else 0,
       if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
+      if (sym.hasEnumFlag) ACC_ENUM else 0,
       if (sym.isVarargsMethod) ACC_VARARGS else 0,
       if (sym.hasFlag(Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0
     )

src/compiler/scala/tools/nsc/javac/JavaParsers.scala

@@ -792,7 +792,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
       val superclazz =
         AppliedTypeTree(javaLangDot(tpnme.Enum), List(enumType))
       addCompanionObject(consts ::: statics ::: predefs, atPos(pos) {
-        ClassDef(mods, name, List(),
+        ClassDef(mods | Flags.ENUM, name, List(),
                  makeTemplate(superclazz :: interfaces, body))
       })
     }
@@ -811,10 +811,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
           skipAhead()
           accept(RBRACE)
         }
-        // The STABLE flag is to signal to namer that this was read from a
-        // java enum, and so should be given a Constant type (thereby making
-        // it usable in annotations.)
-        ValDef(Modifiers(Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr)
+        ValDef(Modifiers(Flags.ENUM | Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr)
       }
     }
 

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -515,7 +515,7 @@ abstract class ClassfileParser {
       val info    = readType()
       val sym     = ownerForFlags(jflags).newValue(name.toTermName, NoPosition, sflags)
 
-      // Note: the info may be overrwritten later with a generic signature
+      // Note: the info may be overwritten later with a generic signature
       // parsed from SignatureATTR
       sym setInfo {
         if (jflags.isEnum) ConstantType(Constant(sym))

src/compiler/scala/tools/nsc/typechecker/Namers.scala

@@ -122,10 +122,31 @@ trait Namers extends MethodSynthesis {
       || (vd.mods.isPrivateLocal && !vd.mods.isCaseAccessor)
       || (vd.name startsWith nme.OUTER)
       || (context.unit.isJava)
+      || isEnumConstant(vd)
     )
+
     def noFinishGetterSetter(vd: ValDef) = (
          (vd.mods.isPrivateLocal && !vd.mods.isLazy) // all lazy vals need accessors, even private[this]
-      || vd.symbol.isModuleVar)
+      || vd.symbol.isModuleVar
+      || isEnumConstant(vd))
+
+    /** Determines whether this field holds an enum constant.
+      * To qualify, the following conditions must be met:
+      *  - The field's class has the ENUM flag set
+      *  - The field's class extends java.lang.Enum
+      *  - The field has the ENUM flag set
+      *  - The field is static
+      *  - The field is stable
+      */
+    def isEnumConstant(vd: ValDef) = {
+      val ownerHasEnumFlag =
+        // Necessary to check because scalac puts Java's static members into the companion object
+        // while Scala's enum constants live directly in the class.
+        // We don't check for clazz.superClass == JavaEnumClass, because this causes a illegal
+        // cyclic reference error. See the commit message for details.
+        if (context.unit.isJava) owner.companionClass.hasEnumFlag else owner.hasEnumFlag
+      vd.mods.hasAllFlags(ENUM | STABLE | STATIC) && ownerHasEnumFlag
+    }
 
     def setPrivateWithin[T <: Symbol](tree: Tree, sym: T, mods: Modifiers): T =
       if (sym.isPrivateLocal || !mods.hasAccessBoundary) sym
@@ -609,11 +630,7 @@ trait Namers extends MethodSynthesis {
       else
         enterGetterSetter(tree)
 
-      // When java enums are read from bytecode, they are known to have
-      // constant types by the jvm flag and assigned accordingly.  When
-      // they are read from source, the java parser marks them with the
-      // STABLE flag, and now we receive that signal.
-      if (tree.symbol hasAllFlags STABLE | JAVA)
+      if (isEnumConstant(tree))
         tree.symbol setInfo ConstantType(Constant(tree.symbol))
     }
 

src/reflect/scala/reflect/api/FlagSets.scala

@@ -169,6 +169,14 @@ trait FlagSets { self: Universe =>
 
     /** Flag indicating that tree was generated by the compiler */
     val SYNTHETIC: FlagSet
+
+    /** Flag indicating that tree represents an enum.
+     *
+     *  It can only appear at
+     *  - the enum's class
+     *  - enum constants
+     **/
+    val ENUM: FlagSet
   }
 
   /** The empty set of flags

src/reflect/scala/reflect/internal/Definitions.scala

@@ -363,6 +363,7 @@ trait Definitions extends api.StandardDefinitions {
     lazy val ComparableClass       = requiredClass[java.lang.Comparable[_]] modifyInfo fixupAsAnyTrait
     lazy val JavaCloneableClass    = requiredClass[java.lang.Cloneable]
     lazy val JavaNumberClass       = requiredClass[java.lang.Number]
+    lazy val JavaEnumClass         = requiredClass[java.lang.Enum[_]]
     lazy val RemoteInterfaceClass  = requiredClass[java.rmi.Remote]
     lazy val RemoteExceptionClass  = requiredClass[java.rmi.RemoteException]
 

src/reflect/scala/reflect/internal/FlagSets.scala

@@ -43,5 +43,6 @@ trait FlagSets extends api.FlagSets { self: SymbolTable =>
     val PRESUPER      : FlagSet = Flags.PRESUPER
     val DEFAULTINIT   : FlagSet = Flags.DEFAULTINIT
     val SYNTHETIC     : FlagSet = Flags.SYNTHETIC
+    val ENUM          : FlagSet = Flags.ENUM
   }
 }

src/reflect/scala/reflect/internal/Flags.scala

@@ -63,7 +63,7 @@ import scala.collection.{ mutable, immutable }
 // 45:  SYNCHRONIZED/M
 // 46:        ARTIFACT
 // 47: DEFAULTMETHOD/M
-// 48:
+// 48:            ENUM
 // 49:
 // 50:
 // 51:    lateDEFERRED
@@ -119,6 +119,7 @@ class ModifierFlags {
   final val DEFAULTINIT   = 1L << 41      // symbol is initialized to the default value: used by -Xcheckinit
   final val ARTIFACT      = 1L << 46      // symbol should be ignored when typechecking; will be marked ACC_SYNTHETIC in bytecode
   final val DEFAULTMETHOD = 1L << 47      // symbol is a java default method
+  final val ENUM          = 1L << 48      // symbol is an enum
 
   /** Symbols which are marked ARTIFACT. (Expand this list?)
    *
@@ -142,7 +143,7 @@ class ModifierFlags {
 }
 object ModifierFlags extends ModifierFlags
 
-/** All flags and associated operatins */
+/** All flags and associated operations */
 class Flags extends ModifierFlags {
   final val METHOD        = 1 << 6        // a method
   final val MODULE        = 1 << 8        // symbol is module or class implementing a module
@@ -446,7 +447,7 @@ class Flags extends ModifierFlags {
     case        SYNCHRONIZED => "<synchronized>"                      // (1L << 45)
     case            ARTIFACT => "<artifact>"                          // (1L << 46)
     case       DEFAULTMETHOD => "<defaultmethod>"                     // (1L << 47)
-    case    0x1000000000000L => ""                                    // (1L << 48)
+    case                ENUM => "<enum>"                              // (1L << 48)
     case    0x2000000000000L => ""                                    // (1L << 49)
     case    0x4000000000000L => ""                                    // (1L << 50)
     case      `lateDEFERRED` => "<latedeferred>"                      // (1L << 51)

src/reflect/scala/reflect/internal/HasFlags.scala

@@ -82,6 +82,7 @@ trait HasFlags {
   def hasAbstractFlag    = hasFlag(ABSTRACT)
   def hasAccessorFlag    = hasFlag(ACCESSOR)
   def hasDefault         = hasFlag(DEFAULTPARAM) && hasFlag(METHOD | PARAM) // Second condition disambiguates with TRAIT
+  def hasEnumFlag        = hasFlag(ENUM)
   def hasLocalFlag       = hasFlag(LOCAL)
   def hasModuleFlag      = hasFlag(MODULE)
   def hasPackageFlag     = hasFlag(PACKAGE)