From 77a45858777554c6e1fb7b9583359a6a492ec066 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 1 Nov 2012 13:01:59 -0700 Subject: The improvements made possible by the scope changes. --- .../nsc/symtab/classfile/ClassfileParser.scala | 2 +- .../scala/tools/nsc/transform/Flatten.scala | 14 +++--- .../scala/tools/nsc/typechecker/Contexts.scala | 52 +++++++--------------- .../scala/tools/nsc/typechecker/Namers.scala | 4 +- 4 files changed, 25 insertions(+), 47 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 42589874fe..e69b4bee94 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..f2409ea482 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -758,15 +758,12 @@ 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 symbolDepth: Int = -1 // the depth of 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 - def finish(qual: Tree, sym: Symbol): NameLookup = ( if (lookupError ne null) lookupError else sym match { @@ -789,23 +786,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,22 +797,24 @@ 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 - if (!defSym.exists) - cx = cx.outer + val entries = (cx.scope lookupUnshadowedEntries name filter (e => qualifies(e.sym))).toList + + pre = cx.enclClass.prefix + symbolDepth = if (entries.isEmpty) cx.depth else (cx.depth - cx.scope.nestingLevel) + entries.head.depth + defSym = entries match { + case Nil => searchPrefix + case hd :: Nil => hd.sym + case alts => logResult(s"!!! lookup overloaded")(cx.owner.newOverloaded(pre, entries map (_.sym))) + } + + if (defSym.exists) // we have a winner: record the symbol depth + symbolDepth = ( + if (entries.isEmpty) cx.depth + else (cx.depth - cx.scope.nestingLevel) + entries.head.depth + ) + else cx = cx.outer // push further outward } - val symbolDepth = cx.depth - localScopeDepth var impSym: Symbol = NoSymbol var imports = Context.this.imports // impSym != NoSymbol => it is imported from imports.head def imp1 = imports.head diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 99b927af66..d8d021f64d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -381,8 +381,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 -- cgit v1.2.3 From 9809721f0bab937029984aa97496d56db08ff61f Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 1 Nov 2012 23:22:05 -0700 Subject: Revamp import ambiguity logic. Code reviewer prodded me into figuring out where my earlier attempts to simplify the import logic broke down. Now it should be much easier to follow. --- .../scala/tools/nsc/typechecker/Contexts.scala | 90 ++++++++++------------ 1 file changed, 42 insertions(+), 48 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index f2409ea482..03d30a6029 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -759,10 +759,9 @@ trait Contexts { self: Analyzer => 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 defSym: Symbol = NoSymbol // the directly found symbol - var symbolDepth: Int = -1 // the depth of 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 + 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 @@ -778,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 @@ -797,33 +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)) { - val entries = (cx.scope lookupUnshadowedEntries name filter (e => qualifies(e.sym))).toList - pre = cx.enclClass.prefix - symbolDepth = if (entries.isEmpty) cx.depth else (cx.depth - cx.scope.nestingLevel) + entries.head.depth + val entries = (cx.scope lookupUnshadowedEntries name filter (e => qualifies(e.sym))).toList defSym = entries match { case Nil => searchPrefix - case hd :: Nil => hd.sym - case alts => logResult(s"!!! lookup overloaded")(cx.owner.newOverloaded(pre, entries map (_.sym))) + 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) // we have a winner: record the symbol depth - symbolDepth = ( - if (entries.isEmpty) cx.depth - else (cx.depth - cx.scope.nestingLevel) + entries.head.depth - ) - else cx = cx.outer // push further outward + if (!defSym.exists) + cx = cx.outer // push further outward } + if (symbolDepth < 0) + symbolDepth = cx.depth 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)) @@ -844,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) -- cgit v1.2.3