diff options
author | Paul Phillips <paulp@improving.org> | 2012-11-04 13:07:05 -0800 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2012-11-04 13:07:05 -0800 |
commit | 3d248efcc1925acb7f73b2b2db2184f8d33b68ad (patch) | |
tree | 161086d04159db62e8ece2d28697e022111f9530 | |
parent | 2c6777fd53b93b95a261aadc87a5cbc03c14d503 (diff) | |
parent | c04a4edcac9e6ecf653159942394a82d579f1f88 (diff) | |
download | scala-3d248efcc1925acb7f73b2b2db2184f8d33b68ad.tar.gz scala-3d248efcc1925acb7f73b2b2db2184f8d33b68ad.tar.bz2 scala-3d248efcc1925acb7f73b2b2db2184f8d33b68ad.zip |
Merge pull request #1554 from paulp/scope-lookup
Improvements to scope lookup.
-rw-r--r-- | src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala | 2 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Flatten.scala | 14 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 114 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Namers.scala | 4 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Scopes.scala | 60 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Symbols.scala | 19 | ||||
-rw-r--r-- | test/files/neg/dbldef.check | 4 |
7 files changed, 122 insertions, 95 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 diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala index a4b541e34d..352bc8fdc9 100644 --- a/src/reflect/scala/reflect/internal/Scopes.scala +++ b/src/reflect/scala/reflect/internal/Scopes.scala @@ -25,8 +25,9 @@ trait Scopes extends api.Scopes { self: SymbolTable => */ var next: ScopeEntry = null + def depth = owner.nestingLevel override def hashCode(): Int = sym.name.start - override def toString(): String = sym.toString() + override def toString() = s"$sym (depth=$depth)" } /** @@ -216,14 +217,46 @@ trait Scopes extends api.Scopes { self: SymbolTable => } } - /** lookup a symbol - * - * @param name ... - * @return ... + /** Lookup a module or a class, filtering out matching names in scope + * which do not match that requirement. + */ + def lookupModule(name: Name): Symbol = lookupAll(name.toTermName) find (_.isModule) getOrElse NoSymbol + def lookupClass(name: Name): Symbol = lookupAll(name.toTypeName) find (_.isClass) getOrElse NoSymbol + + /** True if the name exists in this scope, false otherwise. */ + def containsName(name: Name) = lookupEntry(name) != null + + /** Lookup a symbol. */ def lookup(name: Name): Symbol = { val e = lookupEntry(name) - if (e eq null) NoSymbol else e.sym + if (e eq null) NoSymbol + else if (lookupNextEntry(e) eq null) e.sym + else { + // We shouldn't get here: until now this method was picking a random + // symbol when there was more than one with the name, so this should + // only be called knowing that there are 0-1 symbols of interest. So, we + // can safely return an overloaded symbol rather than throwing away the + // rest of them. Most likely we still break, but at least we will break + // in an understandable fashion (unexpectedly overloaded symbol) rather + // than a non-deterministic bizarre one (see any bug involving overloads + // in package objects.) + val alts = lookupAll(name).toList + log("!!! scope lookup of $name found multiple symbols: $alts") + // FIXME - how is one supposed to create an overloaded symbol without + // knowing the correct owner? Using the symbol owner is not correct; + // say for instance this is List's scope and the symbols are its three + // mkString members. Those symbols are owned by TraversableLike, which + // is no more meaningful an owner than NoSymbol given that we're in + // List. Maybe it makes no difference who owns the overloaded symbol, in + // which case let's establish that and have a canonical creation method. + // + // FIXME - a similar question for prefix, although there are more + // clues from the symbols on that one, as implemented here. In general + // the distinct list is one type and lub becomes the identity. + val prefix = lub(alts map (_.info.prefix) distinct) + NoSymbol.newOverloaded(prefix, alts) + } } /** Returns an iterator yielding every symbol with given name in this scope. @@ -231,7 +264,20 @@ trait Scopes extends api.Scopes { self: SymbolTable => def lookupAll(name: Name): Iterator[Symbol] = new Iterator[Symbol] { var e = lookupEntry(name) def hasNext: Boolean = e ne null - def next(): Symbol = { val r = e.sym; e = lookupNextEntry(e); r } + def next(): Symbol = try e.sym finally e = lookupNextEntry(e) + } + + def lookupAllEntries(name: Name): Iterator[ScopeEntry] = new Iterator[ScopeEntry] { + var e = lookupEntry(name) + def hasNext: Boolean = e ne null + def next(): ScopeEntry = try e finally e = lookupNextEntry(e) + } + + def lookupUnshadowedEntries(name: Name): Iterator[ScopeEntry] = { + lookupEntry(name) match { + case null => Iterator.empty + case e => lookupAllEntries(name) filter (e1 => (e eq e1) || (e.depth == e1.depth && e.sym != e1.sym)) + } } /** lookup a symbol entry matching given name. diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 8cebfabe6f..53a236fa3c 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -1672,12 +1672,23 @@ trait Symbols extends api.Symbols { self: SymbolTable => def filter(cond: Symbol => Boolean): Symbol = if (isOverloaded) { - val alts = alternatives - val alts1 = alts filter cond - if (alts1 eq alts) this + var changed = false + var alts0: List[Symbol] = alternatives + var alts1: List[Symbol] = Nil + + while (alts0.nonEmpty) { + if (cond(alts0.head)) + alts1 ::= alts0.head + else + changed = true + + alts0 = alts0.tail + } + + if (!changed) this else if (alts1.isEmpty) NoSymbol else if (alts1.tail.isEmpty) alts1.head - else owner.newOverloaded(info.prefix, alts1) + else owner.newOverloaded(info.prefix, alts1.reverse) } else if (cond(this)) this else NoSymbol diff --git a/test/files/neg/dbldef.check b/test/files/neg/dbldef.check index 3ee63475e4..b896c4cdcf 100644 --- a/test/files/neg/dbldef.check +++ b/test/files/neg/dbldef.check @@ -6,9 +6,7 @@ dbldef.scala:1: error: type mismatch; required: Int case class test0(x: Int, x: Float) ^ -dbldef.scala:1: error: type mismatch; - found : Float - required: Int +dbldef.scala:1: error: in class test0, multiple overloaded alternatives of x define default arguments case class test0(x: Int, x: Float) ^ three errors found |