wip: modules during fields

adriaanm · adriaanm · commit 8b41f19380ec · 2016-05-16T23:53:46.000-07:00
class C { object o }

trait T { object o }
class MixedIn extends T

abstract class AbsModAcc { def o: O ; class O }
class ImplModAcc extends AbsModAcc { object o extends O }

object top { object stat }

class MethHolder { def m = { object local; local } }
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
@@ -261,7 +261,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
   private def memberClassesForInnerClassTable(classSymbol: Symbol): List[Symbol] = classSymbol.info.decls.collect({
     case sym if sym.isClass && !considerAsTopLevelImplementationArtifact(sym) =>
       sym
-    case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) =>
+    case sym if sym.isModuleNotMethod && !considerAsTopLevelImplementationArtifact(sym) =>
       val r = exitingPickler(sym.moduleClass)
       assert(r != NoSymbol, sym.fullLocationString)
       r
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -492,7 +492,7 @@ abstract class Erasure extends InfoTransform
       // If `member` is a ModuleSymbol, the bridge should not also be a ModuleSymbol. Otherwise we
       // end up with two module symbols with the same name in the same scope, which is surprising
       // when implementing later phases.
-      if (member.isModule) newFlags = (newFlags | METHOD) & ~(MODULE | lateMETHOD | STABLE)
+      if (member.isModule) newFlags = (newFlags | METHOD) & ~(MODULE | STABLE)
       val bridge = other.cloneSymbolImpl(root, newFlags) setPos root.pos
 
       debuglog("generating bridge from %s (%s): %s to %s: %s".format(
diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -14,7 +14,7 @@ import scala.tools.nsc.settings.ScalaVersion
 import scala.tools.nsc.settings.NoScalaVersion
 
 import symtab.Flags._
-import transform.InfoTransform
+import transform.Transform
 
 
 /** <p>
@@ -43,7 +43,7 @@ import transform.InfoTransform
  *
  *  @todo    Check whether we always check type parameter bounds.
  */
-abstract class RefChecks extends InfoTransform with scala.reflect.internal.transform.RefChecks {
+abstract class RefChecks extends Transform with scala.reflect.internal.transform.RefChecks {
 
   val global: Global               // need to repeat here because otherwise last mixin defines global as
                                    // SymbolTable. If we had DOT this would not be an issue
@@ -54,31 +54,9 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
 
   /** the following two members override abstract members in Transform */
   val phaseName: String = "refchecks"
-  override def phaseNewFlags: Long = lateMETHOD
 
   def newTransformer(unit: CompilationUnit): RefCheckTransformer =
     new RefCheckTransformer(unit)
-  override def changesBaseClasses = false
-
-  override def transformInfo(sym: Symbol, tp: Type): Type = {
-    // !!! This is a sketchy way to do things.
-    // It would be better to replace the module symbol with a method symbol
-    // rather than creating this module/method hybrid which must be special
-    // cased all over the place. Look for the call sites which use(d) some
-    // variation of "isMethod && !isModule", which to an observer looks like
-    // a nonsensical condition. (It is now "isModuleNotMethod".)
-    if (sym.isModule && !sym.isStatic) {
-      sym setFlag lateMETHOD | STABLE
-      // Note that this as far as we can see it works equally well
-      // to set the METHOD flag here and dump lateMETHOD, but it does
-      // mean that under separate compilation the typer will see
-      // modules as methods (albeit stable ones with singleton types.)
-      // So for now lateMETHOD lives while we try to convince ourselves
-      // we can live without it or deliver that info some other way.
-      log(s"Stabilizing module method for ${sym.fullLocationString}")
-    }
-    super.transformInfo(sym, tp)
-  }
 
   val toJavaRepeatedParam  = new SubstSymMap(RepeatedParamClass -> JavaRepeatedParamClass)
   val toScalaRepeatedParam = new SubstSymMap(JavaRepeatedParamClass -> RepeatedParamClass)
@@ -1180,69 +1158,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       finally popLevel()
     }
 
-    /** Eliminate ModuleDefs. In all cases the ModuleDef (carrying a module symbol) is
-     *  replaced with a ClassDef (carrying the corresponding module class symbol) with additional
-     *  trees created as follows:
-     *
-     *  1) A statically reachable object (either top-level or nested only in objects) receives
-     *     no additional trees.
-     *  2) An inner object which matches an existing member (e.g. implements an interface)
-     *     receives an accessor DefDef to implement the interface.
-     *  3) An inner object otherwise receives a private ValDef which declares a module var
-     *     (the field which holds the module class - it has a name like Foo$module) and an
-     *     accessor for that field. The instance is created lazily, on first access.
-     */
-    private def eliminateModuleDefs(moduleDef: Tree): List[Tree] = exitingRefchecks {
-      val ModuleDef(_, _, impl) = moduleDef
-      val module        = moduleDef.symbol
-      val site          = module.owner
-      val moduleName    = module.name.toTermName
-      // The typer doesn't take kindly to seeing this ClassDef; we have to
-      // set NoType so it will be ignored.
-      val cdef          = ClassDef(module.moduleClass, impl) setType NoType
-
-      def matchingInnerObject() = {
-        val newFlags = (module.flags | STABLE) & ~MODULE
-        val newInfo  = NullaryMethodType(module.moduleClass.tpe)
-        val accessor = site.newMethod(moduleName, module.pos, newFlags) setInfoAndEnter newInfo
-
-        DefDef(accessor, Select(This(site), module)) :: Nil
-      }
-      val newTrees = cdef :: (
-        if (module.isStatic)
-          // trait T { def f: Object }; object O extends T { object f }. Need to generate method f in O.
-          if (module.isOverridingSymbol) matchingInnerObject() else Nil
-        else
-          newInnerObject(site, module)
-      )
-      transformTrees(newTrees map localTyper.typedPos(moduleDef.pos))
-    }
-    def newInnerObject(site: Symbol, module: Symbol): List[Tree] = {
-      if (site.isTrait)
-        DefDef(module, EmptyTree) :: Nil
-      else {
-        val moduleVar = site newModuleVarSymbol module
-        // used for the mixin case: need a new symbol owned by the subclass for the accessor, rather than repurposing the module symbol
-        def mkAccessorSymbol =
-          site.newMethod(module.name.toTermName, site.pos, STABLE | MODULE | MIXEDIN)
-            .setInfo(moduleVar.tpe)
-            .andAlso(self => if (module.isPrivate) self.expandName(module.owner))
-
-        val accessor = if (module.owner == site) module else mkAccessorSymbol
-        val accessorDef = DefDef(accessor, gen.mkAssignAndReturn(moduleVar, gen.newModule(module, moduleVar.tpe)).changeOwner(moduleVar -> accessor))
-
-        ValDef(moduleVar) :: accessorDef :: Nil
-      }
-    }
 
-    def mixinModuleDefs(clazz: Symbol): List[Tree] = {
-      val res = for {
-        mixinClass <- clazz.mixinClasses.iterator
-        module     <- mixinClass.info.decls.iterator.filter(_.isModule)
-        newMember  <- newInnerObject(clazz, module)
-      } yield transform(localTyper.typedPos(clazz.pos)(newMember))
-      res.toList
-    }
 
     def transformStat(tree: Tree, index: Int): List[Tree] = tree match {
       case t if treeInfo.isSelfConstrCall(t) =>
@@ -1253,7 +1169,6 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
           debuglog("refsym = " + currentLevel.refsym)
           reporter.error(currentLevel.refpos, "forward reference not allowed from self constructor invocation")
         }
-      case ModuleDef(_, _, _) => eliminateModuleDefs(tree)
       case ValDef(_, _, _, _) =>
         val tree1 = transform(tree) // important to do before forward reference check
         if (tree1.symbol.isLazy) tree1 :: Nil
@@ -1693,13 +1608,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
             checkOverloadedRestrictions(currentOwner, currentOwner)
             // SI-7870 default getters for constructors live in the companion module
             checkOverloadedRestrictions(currentOwner, currentOwner.companionModule)
-            val bridges = addVarargBridges(currentOwner)
-            val moduleDesugared = if (currentOwner.isTrait) Nil else mixinModuleDefs(currentOwner)
+            val bridges = addVarargBridges(currentOwner) // TODO: do this during uncurry?
             checkAllOverrides(currentOwner)
             checkAnyValSubclass(currentOwner)
             if (currentOwner.isDerivedValueClass)
               currentOwner.primaryConstructor makeNotPrivate NoSymbol // SI-6601, must be done *after* pickler!
-            if (bridges.nonEmpty || moduleDesugared.nonEmpty) deriveTemplate(tree)(_ ::: bridges ::: moduleDesugared) else tree
+            if (bridges.nonEmpty) deriveTemplate(tree)(_ ::: bridges) else tree
 
           case dc@TypeTreeWithDeferredRefCheck() => abort("adapt should have turned dc: TypeTreeWithDeferredRefCheck into tpt: TypeTree, with tpt.original == dc")
           case tpt@TypeTree() =>
diff --git a/src/reflect/scala/reflect/internal/Phase.scala b/src/reflect/scala/reflect/internal/Phase.scala
@@ -47,7 +47,9 @@ abstract class Phase(val prev: Phase) {
   final val specialized: Boolean   = ((prev ne null) && (prev ne NoPhase)) && (prev.name == "specialize" || prev.specialized)
   final val refChecked: Boolean    = ((prev ne null) && (prev ne NoPhase)) && (prev.name == "refchecks"  || prev.refChecked)
 
-  // are we past the fields phase, so that we should allow writing to vals (as part of type checking trait setters)
+  // are we past the fields phase, so that:
+  //   - we should allow writing to vals (as part of type checking trait setters)
+  //   - modules have module accessors
   final val assignsFields: Boolean = ((prev ne null) && (prev ne NoPhase)) && (prev.name == "fields"     || prev.assignsFields)
 
   /** This is used only in unsafeTypeParams, and at this writing is
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -324,17 +324,12 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     final def newImport(pos: Position): TermSymbol =
       newTermSymbol(nme.IMPORT, pos)
 
-    def newModuleVarSymbol(accessor: Symbol): TermSymbol = {
-      val newName  = nme.moduleVarName(accessor.name.toTermName)
-      val newFlags = MODULEVAR | ( if (this.isClass) PrivateLocal | SYNTHETIC else 0 )
-      val newInfo  = thisType.memberType(accessor).finalResultType
-      val mval     = newVariable(newName, accessor.pos.focus, newFlags.toLong) addAnnotation VolatileAttr
-
-      if (this.isClass)
-        mval setInfoAndEnter newInfo
-      else
-        mval setInfo newInfo
-    }
+    def newModuleVarSymbol(accessor: Symbol): TermSymbol =
+      newVariable(
+        nme.moduleVarName(accessor.name.toTermName),
+        accessor.pos.focus,
+        (MODULEVAR | (if (isClass) PrivateLocal | SYNTHETIC else 0)).toLong) addAnnotation VolatileAttr
+
 
     final def newModuleSymbol(name: TermName, pos: Position = NoPosition, newFlags: Long = 0L): ModuleSymbol =
       newTermSymbol(name, pos, newFlags).asInstanceOf[ModuleSymbol]
@@ -2771,7 +2766,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     override def isCapturedVariable = hasAllFlags(MUTABLE | CAPTURED) && !hasFlag(METHOD)
 
     override def companionSymbol: Symbol = companionClass
-    override def moduleClass = if (isModule) referenced else NoSymbol
+    override def moduleClass = if (isModuleNotMethod) referenced else NoSymbol
 
     override def isBridge           = this hasFlag BRIDGE
     override def isEarlyInitialized = this hasFlag PRESUPER
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
@@ -3441,10 +3441,10 @@ trait Types
     if (!sym.isOverridableMember || sym.owner == pre.typeSymbol) sym
     else pre.nonPrivateMember(sym.name).suchThat { sym =>
       // SI-7928 `isModuleNotMethod` is here to avoid crashing with spuriously "overloaded" module accessor and module symbols.
-      //         These appear after refchecks eliminates ModuleDefs that implement an interface.
+      //         These appear after the fields phase eliminates ModuleDefs that implement an interface.
       //         Here, we exclude the module symbol, which allows us to bind to the accessor.
-      // SI-8054 We must only do this after refchecks, otherwise we exclude the module symbol which does not yet have an accessor!
-      val isModuleWithAccessor = phase.refChecked && sym.isModuleNotMethod
+      // SI-8054 We must only do this after fields, otherwise we exclude the module symbol which does not yet have an accessor!
+      val isModuleWithAccessor = phase.assignsFields && sym.isModuleNotMethod
       sym.isType || (!isModuleWithAccessor && sym.isStable && !sym.hasVolatileType)
     } orElse sym
   }

Original file line number	Diff line number	Diff line change
`@@ -261,7 +261,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {`
`261`	`261`	`private def memberClassesForInnerClassTable(classSymbol: Symbol): List[Symbol] = classSymbol.info.decls.collect({`
`262`	`262`	`case sym if sym.isClass && !considerAsTopLevelImplementationArtifact(sym) =>`
`263`	`263`	`sym`
`264`		`- case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) =>`
	`264`	`+ case sym if sym.isModuleNotMethod && !considerAsTopLevelImplementationArtifact(sym) =>`
`265`	`265`	`val r = exitingPickler(sym.moduleClass)`
`266`	`266`	`assert(r != NoSymbol, sym.fullLocationString)`
`267`	`267`	`r`