Merge pull request #1554 from paulp/scope-lookup

Improvements to scope lookup.

4 files changed, 53 insertions, 81 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 5922d67a94..ba68c6b866 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -562,7 +562,7 @@ abstract class ClassfileParser {
         0 until in.nextChar foreach (_ => parseMethod())
         val needsConstructor = (
              !sawPrivateConstructor
-          && instanceScope.lookup(nme.CONSTRUCTOR) == NoSymbol
+          && !(instanceScope containsName nme.CONSTRUCTOR)
           && (sflags & INTERFACE) == 0
         )
         if (needsConstructor)
diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala
index e2913bea0d..672a11ec30 100644
--- a/src/compiler/scala/tools/nsc/transform/Flatten.scala
+++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala
@@ -18,18 +18,14 @@ abstract class Flatten extends InfoTransform {
   /** the following two members override abstract members in Transform */
   val phaseName: String = "flatten"
 
-  /** Updates the owning scope with the given symbol; returns the old symbol.
+  /** Updates the owning scope with the given symbol, unlinking any others.
    */
-  private def replaceSymbolInCurrentScope(sym: Symbol): Symbol = exitingFlatten {
+  private def replaceSymbolInCurrentScope(sym: Symbol): Unit = exitingFlatten {
     val scope = sym.owner.info.decls
-    val old   = scope lookup sym.name andAlso scope.unlink
+    val old   = (scope lookupUnshadowedEntries sym.name).toList
+    old foreach (scope unlink _)
     scope enter sym
-
-    if (old eq NoSymbol)
-      log(s"lifted ${sym.fullLocationString}")
-    else
-      log(s"lifted ${sym.fullLocationString} after unlinking existing $old from scope.")
-
+    log(s"lifted ${sym.fullLocationString}" + ( if (old.isEmpty) "" else " after unlinking $old from scope." ))
     old
   }
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index 92e2bc186e..03d30a6029 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -758,14 +758,10 @@ trait Contexts { self: Analyzer =>
     def lookupSymbol(name: Name, qualifies: Symbol => Boolean): NameLookup = {
       var lookupError: NameLookup  = null       // set to non-null if a definite error is encountered
       var inaccessible: NameLookup = null       // records inaccessible symbol for error reporting in case none is found
-      var defEntry: ScopeEntry     = null       // the scope entry of defSym, if defined in a local scope
       var defSym: Symbol           = NoSymbol   // the directly found symbol
       var pre: Type                = NoPrefix   // the prefix type of defSym, if a class member
-      var cx: Context              = this
-      var needsQualifier           = false      // working around package object overloading bug
-
-      def defEntrySymbol  = if (defEntry eq null) NoSymbol else defEntry.sym
-      def localScopeDepth = if (defEntry eq null) 0 else cx.scope.nestingLevel - defEntry.owner.nestingLevel
+      var cx: Context              = this       // the context under consideration
+      var symbolDepth: Int         = -1         // the depth of the directly found symbol
 
       def finish(qual: Tree, sym: Symbol): NameLookup = (
         if (lookupError ne null) lookupError
@@ -781,7 +777,7 @@ trait Contexts { self: Analyzer =>
           || unit.exists && s.sourceFile != unit.source.file
         )
       )
-      def requiresQualifier(s: Symbol) = needsQualifier || (
+      def requiresQualifier(s: Symbol) = (
            s.owner.isClass
         && !s.owner.isPackageClass
         && !s.isTypeParameterOrSkolem
@@ -789,23 +785,6 @@ trait Contexts { self: Analyzer =>
       def lookupInPrefix(name: Name)    = pre member name filter qualifies
       def accessibleInPrefix(s: Symbol) = isAccessible(s, pre, superAccess = false)
 
-      def correctForPackageObject(sym: Symbol): Symbol = {
-        if (sym.isTerm && isInPackageObject(sym, pre.typeSymbol)) {
-          val sym1 = lookupInPrefix(sym.name)
-          if ((sym1 eq NoSymbol) || (sym eq sym1)) sym else {
-            needsQualifier = true
-            log(s"""
-              |  !!! Overloaded package object member resolved incorrectly.
-              |        prefix: $pre
-              |     Discarded: ${sym.defString}
-              |         Using: ${sym1.defString}
-              """.stripMargin)
-            sym1
-          }
-        }
-        else sym
-      }
-
       def searchPrefix = {
         cx = cx.enclClass
         val found0 = lookupInPrefix(name)
@@ -817,31 +796,35 @@ trait Contexts { self: Analyzer =>
       }
       // cx.scope eq null arises during FixInvalidSyms in Duplicators
       while (defSym == NoSymbol && (cx ne NoContext) && (cx.scope ne null)) {
-        pre = cx.enclClass.prefix
-        // !!! FIXME.  This call to lookupEntry is at the root of all the
-        // bad behavior with overloading in package objects.  lookupEntry
-        // just takes the first symbol it finds in scope, ignoring the rest.
-        // When a selection on a package object arrives here, the first
-        // overload is always chosen.  "correctForPackageObject" exists to
-        // undo that decision.  Obviously it would be better not to do it in
-        // the first place; however other things seem to be tied to obtaining
-        // that ScopeEntry, specifically calculating the nesting depth.
-        defEntry = cx.scope lookupEntry name
-        defSym   = defEntrySymbol filter qualifies map correctForPackageObject orElse searchPrefix
+        pre         = cx.enclClass.prefix
+        val entries = (cx.scope lookupUnshadowedEntries name filter (e => qualifies(e.sym))).toList
+        defSym      = entries match {
+          case Nil       => searchPrefix
+          case hd :: tl  =>
+            // we have a winner: record the symbol depth
+            symbolDepth = (cx.depth - cx.scope.nestingLevel) + hd.depth
+            if (tl.isEmpty) hd.sym
+            else logResult(s"!!! lookup overloaded")(cx.owner.newOverloaded(pre, entries map (_.sym)))
+        }
         if (!defSym.exists)
-          cx = cx.outer
+          cx = cx.outer // push further outward
       }
+      if (symbolDepth < 0)
+        symbolDepth = cx.depth
 
-      val symbolDepth    = cx.depth - localScopeDepth
       var impSym: Symbol = NoSymbol
-      var imports        = Context.this.imports   // impSym != NoSymbol => it is imported from imports.head
+      var imports        = Context.this.imports
       def imp1           = imports.head
+      def imp2           = imports.tail.head
+      def imp1Explicit   = imp1 isExplicitImport name
+      def imp2Explicit   = imp2 isExplicitImport name
 
       while (!qualifies(impSym) && imports.nonEmpty && imp1.depth > symbolDepth) {
         impSym = importedAccessibleSymbol(imp1, name)
         if (!impSym.exists)
           imports = imports.tail
       }
+
       if (defSym.exists && impSym.exists) {
         // imported symbols take precedence over package-owned symbols in different compilation units.
         if (isPackageOwnedInDifferentUnit(defSym))
@@ -862,40 +845,33 @@ trait Contexts { self: Analyzer =>
           finish(EmptyTree, defSym)
       }
       else if (impSym.exists) {
-        // Imports against which we will test impSym for any ambiguities
-        var importsTail  = imports.tail
-        val imp1Explicit = imp1 isExplicitImport name
-        def imp2         = importsTail.head
-        def sameDepth    = imp1.depth == imp2.depth
-        def isDone       = importsTail.isEmpty || imp1Explicit && !sameDepth
-
+        def sameDepth  = imp1.depth == imp2.depth
+        def needsCheck = if (sameDepth) imp1Explicit == imp2Explicit else imp1Explicit || imp2Explicit
+        def isDone     = imports.tail.isEmpty || (!sameDepth && imp1Explicit)
+        def ambiguous  = needsCheck && isAmbiguousImport(imp1, imp2, name) && {
+          lookupError = ambiguousImports(imp1, imp2)
+          true
+        }
+        // Ambiguity check between imports.
+        // The same name imported again is potentially ambiguous if the name is:
+        //  - after explicit import, explicitly imported again at the same or lower depth
+        //  - after explicit import, wildcard imported at lower depth
+        //  - after wildcard import, wildcard imported at the same depth
+        // Under all such conditions isAmbiguousImport is called, which will
+        // examine the imports in case they are importing the same thing; if that
+        // can't be established conclusively, an error is issued.
         while (lookupError == null && !isDone) {
           val other = importedAccessibleSymbol(imp2, name)
-          // Ambiguity check between imports.
-          // The same name imported again is potentially ambiguous if the name is:
-          //  - after explicit import, explicitly imported again at the same or lower depth
-          //  - after explicit import, wildcard imported at lower depth
-          //  - after wildcard import, wildcard imported at the same depth
-          // Under all such conditions isAmbiguousImport is called, which will
-          // examine the imports in case they are importing the same thing; if that
-          // can't be established conclusively, an error is issued.
-          if (qualifies(other)) {
-            val imp2Explicit = imp2 isExplicitImport name
-            val needsCheck = (
-              if (sameDepth) imp1Explicit == imp2Explicit
-              else imp1Explicit || imp2Explicit
-            )
-            log(s"Import ambiguity: imp1=$imp1, imp2=$imp2, sameDepth=$sameDepth, needsCheck=$needsCheck")
-            if (needsCheck && isAmbiguousImport(imp1, imp2, name))
-              lookupError = ambiguousImports(imp1, imp2)
-            else if (imp2Explicit) {
-              // if we weren't ambiguous and imp2 is explicit, imp2 replaces imp1
-              // as the current winner.
-              impSym  = other
-              imports = importsTail
-            }
+          // if the competing import is unambiguous and explicit, it is the new winner.
+          val isNewWinner = qualifies(other) && !ambiguous && imp2Explicit
+          // imports is imp1 :: imp2 :: rest.
+          // If there is a new winner, it is imp2, and imports drops imp1.
+          // If there is not, imp1 is still the winner, and it drops imp2.
+          if (isNewWinner) {
+            impSym = other
+            imports = imports.tail
           }
-          importsTail = importsTail.tail
+          else imports = imp1 :: imports.tail.tail
         }
         // optimization: don't write out package prefixes
         finish(resetPos(imp1.qual.duplicate), impSym)
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index 5e537e3bb3..3bf7976241 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -380,8 +380,8 @@ trait Namers extends MethodSynthesis {
       if (sym eq NoSymbol) return
 
       val ctx    = if (context.owner.isPackageObjectClass) context.outer else context
-      val module = if (sym.isModule) sym else ctx.scope lookup tree.name.toTermName
-      val clazz  = if (sym.isClass) sym else ctx.scope lookup tree.name.toTypeName
+      val module = if (sym.isModule) sym else ctx.scope lookupModule tree.name
+      val clazz  = if (sym.isClass) sym else ctx.scope lookupClass tree.name
       val fails  = (
            module.isModule
         && clazz.isClass