Replace open package module logic by special member lookups in a package class.

Rather than fiddling with scopes, which can lead to race conditions, we now special case member lookup and mamber name filters in ClassDenotations that represent packages.

3 files changed, 52 insertions, 46 deletions

diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala
index 90a6a373c..bc7545abd 100644
--- a/src/dotty/tools/dotc/core/Denotations.scala
+++ b/src/dotty/tools/dotc/core/Denotations.scala
@@ -306,6 +306,7 @@ object Denotations {
     }
 
     final def asSingleDenotation = asInstanceOf[SingleDenotation]
+    final def asSymDenotation = asInstanceOf[SymDenotation]
 
     def toText(printer: Printer): Text = printer.toText(this)
   }
@@ -532,8 +533,6 @@ object Denotations {
      */
     def copyIfParentInvalid(implicit ctx: Context): SingleDenotation = this
 
-    final def asSymDenotation = asInstanceOf[SymDenotation]
-
     override def toString =
       if (symbol == NoSymbol) symbol.toString
       else s"<SingleDenotation of type $info>"
diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala
index 5278a08d2..7b9d4cd48 100644
--- a/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -29,7 +29,9 @@ trait SymDenotations { this: Context =>
     initInfo: Type,
     initPrivateWithin: Symbol = NoSymbol)(implicit ctx: Context): SymDenotation = {
     val result =
-      if (symbol.isClass) new ClassDenotation(symbol, owner, name, initFlags, initInfo, initPrivateWithin)
+      if (symbol.isClass)
+        if (initFlags is Package) new PackageClassDenotation(symbol, owner, name, initFlags, initInfo, initPrivateWithin)
+        else new ClassDenotation(symbol, owner, name, initFlags, initInfo, initPrivateWithin)
       else new SymDenotation(symbol, owner, name, initFlags, initInfo, initPrivateWithin)
     result.validFor = stablePeriod
     result
@@ -983,7 +985,7 @@ object SymDenotations {
       } else computeNPMembersNamed(name)
     }
 
-    private def computeNPMembersNamed(name: Name)(implicit ctx: Context): PreDenotation = /*>|>*/ Stats.track("computeNPMembersNamed") /*<|<*/ {
+    private[core] def computeNPMembersNamed(name: Name)(implicit ctx: Context): PreDenotation = /*>|>*/ Stats.track("computeNPMembersNamed") /*<|<*/ {
       if (!classSymbol.hasChildren ||
           !Config.useFingerPrints ||
           (memberFingerPrint contains name)) {
@@ -1124,6 +1126,51 @@ object SymDenotations {
     }
   }
 
+  /** The denotation of a package class.
+   *  It overrides ClassDenotation to take account of package objects when looking for members
+   */
+  class PackageClassDenotation private[SymDenotations] (
+    symbol: Symbol,
+    ownerIfExists: Symbol,
+    name: Name,
+    initFlags: FlagSet,
+    initInfo: Type,
+    initPrivateWithin: Symbol = NoSymbol)
+    extends ClassDenotation(symbol, ownerIfExists, name, initFlags, initInfo, initPrivateWithin) {
+
+    private[this] var packageObjCache: SymDenotation = _
+    private[this] var packageObjRunId: RunId = NoRunId
+
+    /** The package object in this class, of one exists */
+    def packageObj(implicit ctx: Context): SymDenotation = {
+      if (packageObjRunId != ctx.runId) {
+        packageObjRunId = ctx.runId
+        packageObjCache = NoDenotation // break cycle in case we are looking for package object itself
+        packageObjCache = findMember(nme.PACKAGE, thisType, EmptyFlags).asSymDenotation
+      }
+      packageObjCache
+    }
+
+    /** Look first for members in package; if none are found look in package object */
+    override def computeNPMembersNamed(name: Name)(implicit ctx: Context): PreDenotation = {
+      val denots = super.computeNPMembersNamed(name)
+      if (denots.exists) denots
+      else packageObj.moduleClass.denot match {
+        case pcls: ClassDenotation => pcls.computeNPMembersNamed(name)
+        case _ => denots
+      }
+    }
+
+    /** The union of the member names of the package and the package object */
+    override def memberNames(keepOnly: NameFilter)(implicit ctx: Context): Set[Name] = {
+      val ownNames = super.memberNames(keepOnly)
+      packageObj.moduleClass.denot match {
+        case pcls: ClassDenotation => ownNames union pcls.memberNames(keepOnly)
+        case _ => ownNames
+      }
+    }
+  }
+
   object NoDenotation extends SymDenotation(
     NoSymbol, NoSymbol, "<none>".toTermName, Permanent, NoType) {
     override def exists = false
diff --git a/src/dotty/tools/dotc/core/SymbolLoaders.scala b/src/dotty/tools/dotc/core/SymbolLoaders.scala
index 543b7cc1d..d88e84ff7 100644
--- a/src/dotty/tools/dotc/core/SymbolLoaders.scala
+++ b/src/dotty/tools/dotc/core/SymbolLoaders.scala
@@ -163,49 +163,9 @@ class SymbolLoaders {
           if (maybeModuleClass(classRep) && !root.decls.lookup(classRep.name.toTypeName).exists)
             initializeFromClassPath(root.symbol, classRep)
       }
-      if (!root.isEmptyPackage) {
-        for (pkg <- classpath.packages) {
+      if (!root.isEmptyPackage)
+        for (pkg <- classpath.packages)
           enterPackage(root.symbol, pkg)
-        }
-        openPackageModule(root.symbol.asClass)
-      }
-    }
-  }
-
-  /** if there's a `package` member object in `pkgClass`, enter its members into it. */
-  def openPackageModule(pkgClass: ClassSymbol)(implicit ctx: Context): Unit = {
-    val pkgModule = pkgClass.info.decl(nme.PACKAGEkw).symbol
-    if ((pkgModule is Module) &&
-        (pkgModule.isCompleted ||
-         !pkgModule.completer.isInstanceOf[SourcefileLoader]))
-      // println("open "+pkgModule)//DEBUG
-      openPackageModule(pkgModule, pkgClass)
-  }
-
-  // todo: revise to really include the members?
-  def openPackageModule(container: Symbol, dest: ClassSymbol)(implicit ctx: Context): Unit = {
-    def isImportable(sym: Symbol) = !(sym is Private) && !sym.isConstructor
-    // unlink existing symbols in the package
-    for (member <- container.info.decls.iterator) {
-      if (isImportable(member)) {
-        // todo: handle overlapping definitions in some way: mark as errors
-        // or treat as abstractions. For now the symbol in the package module takes precedence.
-        for (existing <- dest.info.decl(member.name).alternatives)
-          dest.delete(existing.symbol)
-      }
-    }
-    // enter non-private decls in the class
-    for (member <- container.info.decls.iterator) {
-      if (isImportable(member)) {
-        dest.enterNoReplace(member, dest.decls.asInstanceOf[MutableScope])
-      }
-    }
-    // !!! TODO info.decls -> decls
-    // enter decls of parent classes
-    for (p <- container.info.parents) {
-      if (p.symbol != defn.ObjectClass) {
-        openPackageModule(p.symbol, dest)
-      }
     }
   }
 }