Clean up and document some usages of flags in the backend

1 files changed, 60 insertions, 15 deletions

diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index cc750fb88a..b6cce4524b 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -55,23 +55,29 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
   def newFreeTypeSymbol(name: TypeName, flags: Long = 0L, origin: String): FreeTypeSymbol =
     new FreeTypeSymbol(name, origin) initFlags flags
 
-  /** The original owner of a class. Used by the backend to generate
-   *  EnclosingMethod attributes.
+  /**
+   * This map stores the original owner the the first time the owner of a symbol is re-assigned.
+   * The original owner of a symbol is needed in some places in the backend. Ideally, owners should
+   * be versioned like the type history.
    */
-  val originalOwner = perRunCaches.newMap[Symbol, Symbol]()
+  private val originalOwnerMap = perRunCaches.newMap[Symbol, Symbol]()
 
   // TODO - don't allow the owner to be changed without checking invariants, at least
   // when under some flag. Define per-phase invariants for owner/owned relationships,
   // e.g. after flatten all classes are owned by package classes, there are lots and
   // lots of these to be declared (or more realistically, discovered.)
-  protected def saveOriginalOwner(sym: Symbol) {
-    if (originalOwner contains sym) ()
-    else originalOwner(sym) = sym.rawowner
+  protected def saveOriginalOwner(sym: Symbol): Unit = {
+    // some synthetic symbols have NoSymbol as owner initially
+    if (sym.owner != NoSymbol) {
+      if (originalOwnerMap contains sym) ()
+      else originalOwnerMap(sym) = sym.rawowner
+    }
   }
+
   protected def originalEnclosingMethod(sym: Symbol): Symbol = {
     if (sym.isMethod || sym == NoSymbol) sym
     else {
-      val owner = originalOwner.getOrElse(sym, sym.rawowner)
+      val owner = sym.originalOwner
       if (sym.isLocalDummy) owner.enclClass.primaryConstructor
       else originalEnclosingMethod(owner)
     }
@@ -757,8 +763,22 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
      *  So "isModuleNotMethod" exists not for its achievement in
      *  brevity, but to encapsulate the relevant condition.
      */
-    def isModuleNotMethod = isModule && !isMethod
-    def isStaticModule    = isModuleNotMethod && isStatic
+    def isModuleNotMethod = {
+      if (isModule) {
+        if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there.
+        !isMethod
+      } else false
+    }
+
+    // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should
+    // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module
+    // symbols:
+    //   trait T { object A }
+    //   object O extends T
+    // The module symbol A is cloned into T$impl (addInterfaces), and then cloned into O (mixin).
+    // Since the original A is not static, it's turned into a method. The clone in O however is
+    // static (owned by a module), but it's also a method.
+    def isStaticModule = isModuleNotMethod && isStatic
 
     final def isInitializedToDefault = !isType && hasAllFlags(DEFAULTINIT | ACCESSOR)
     final def isThisSym = isTerm && owner.thisSym == this
@@ -909,10 +929,31 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     )
     final def isModuleVar = hasFlag(MODULEVAR)
 
-    /** Is this symbol static (i.e. with no outer instance)?
-     *  Q: When exactly is a sym marked as STATIC?
-     *  A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or any number of levels deep.
-     *  http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6
+    /**
+     * Is this symbol static (i.e. with no outer instance)?
+     * Q: When exactly is a sym marked as STATIC?
+     * A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or
+     * any number of levels deep.
+     * http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6
+     *
+     * TODO: should this only be invoked on class / module symbols? because there's also `isStaticMember`.
+     *
+     * Note: the result of `isStatic` changes over time.
+     *  - Lambdalift local definitions to the class level, the `owner` field is modified.
+     *      object T { def foo { object O } }
+     *    After lambdalift, the OModule.isStatic is true.
+     *
+     *  - After flatten, nested classes are moved to the package level. Invoking `owner` on a
+     *    class returns a package class, for which `isStaticOwner` is true. For example,
+     *      class C { object O }
+     *    OModuleClass.isStatic is true after flatten. Using phase travel to get before flatten,
+     *    method `owner` returns the class C.
+     *
+     * Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the
+     * current implementation. For example
+     *   trait T { def foo = 1 }
+     * The method `foo` in the implementation class T$impl will be `isStatic`, because trait
+     * impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true).
      */
     def isStatic = (this hasFlag STATIC) || owner.isStaticOwner
 
@@ -1106,7 +1147,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
      *  - After lambdalift, all local method and class definitions (those not owned by a class
      *    or package class) change their owner to the enclosing class. This is done through
      *    a destructive "sym.owner = sym.owner.enclClass". The old owner is saved by
-     *    saveOriginalOwner for the backend (needed to generate the EnclosingMethod attribute).
+     *    saveOriginalOwner.
      *  - After flatten, all classes are owned by a PackageClass. This is done through a
      *    phase check (if after flatten) in the (overridden) method "def owner" in
      *    ModuleSymbol / ClassSymbol. The `rawowner` field is not modified.
@@ -1129,6 +1170,11 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     final def safeOwner: Symbol   = if (this eq NoSymbol) NoSymbol else owner
     final def assertOwner: Symbol = if (this eq NoSymbol) abort("no-symbol does not have an owner") else owner
 
+    /**
+     * The initial owner of this symbol.
+     */
+    def originalOwner: Symbol = originalOwnerMap.getOrElse(this, rawowner)
+
     // TODO - don't allow the owner to be changed without checking invariants, at least
     // when under some flag. Define per-phase invariants for owner/owned relationships,
     // e.g. after flatten all classes are owned by package classes, there are lots and
@@ -1142,7 +1188,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
 
     def ownerChain: List[Symbol] = this :: owner.ownerChain
-    def originalOwnerChain: List[Symbol] = this :: originalOwner.getOrElse(this, rawowner).originalOwnerChain
 
     // Non-classes skip self and return rest of owner chain; overridden in ClassSymbol.
     def enclClassChain: List[Symbol] = owner.enclClassChain