Skip to content

Commit

Permalink
Unify scope lookup for companions and default getters
Browse files Browse the repository at this point in the history
In scala#5700, I fixed a bug in the companion lookup, which ensured
they were defined in the same scope.

The same approach applies well to the lookup of default getters.

You may ask, we can't just use:

```
context.lookupSymbol(name, _.owner == expectedOwner)
```

That doesn't individually lookup the entry in each enclosing
nested scopes, but rather relies on the outer scope delegation
in `Scope.lookupEntry` itself. This in turn relies on the way that
nested scopes share the `elems` table with the enclosing scope:

```
  final def newNestedScope(outer: Scope): Scope = {
    val nested = newScope
    nested.elems = outer.elems
    nested.nestinglevel = outer.nestinglevel + 1
    ...
  }
```

If the outer scope is later mutated, in our case by lazily adding
the default getter, the inner scope won't see the new elems.
Context.lookupSymbol will jump immediately jump to search of the
enclosing prefix.

Perhaps a better design would be for the inner scope to retain a
reference to the outer one, rather than just to the head of its
elems linked list at the time the nested scope was created.
  • Loading branch information
retronym committed Jan 29, 2018
1 parent 86f2028 commit da14e9c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 36 deletions.
46 changes: 14 additions & 32 deletions src/compiler/scala/tools/nsc/typechecker/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1065,8 +1065,11 @@ trait Contexts { self: Analyzer =>
found1
}

def lookupInScope(scope: Scope) =
(scope lookupUnshadowedEntries name filter (e => qualifies(e.sym))).toList
def lookupInScope(scope: Scope) = {
val entries = scope lookupUnshadowedEntries name
val result = entries.filter(e => qualifies(e.sym)).toList
result
}

def newOverloaded(owner: Symbol, pre: Type, entries: List[ScopeEntry]) =
logResult(s"overloaded symbol in $pre")(owner.newOverloaded(pre, entries map (_.sym)))
Expand Down Expand Up @@ -1198,32 +1201,14 @@ trait Contexts { self: Analyzer =>
else finish(EmptyTree, NoSymbol)
}

/**
* Find a symbol in this context or one of its outers.
*
* Used to find symbols are owned by methods (or fields), they can't be
* found in some scope.
*
* Examples: companion module of classes owned by a method, default getter
* methods of nested methods. See NamesDefaults.scala
*/
def lookup(name: Name, expectedOwner: Symbol) = {
var res: Symbol = NoSymbol
var ctx = this
while (res == NoSymbol && ctx.outer != ctx) {
ctx.scope.lookupUnshadowedEntries(name).filter(s => s.sym != NoSymbol && s.sym.owner == expectedOwner).toList match {
case Nil =>
ctx = ctx.outer
case found :: Nil =>
res = found.sym
case alts =>
res = expectedOwner.newOverloaded(NoPrefix, alts.map(_.sym))
}
}
res
final def lookupCompanionInIncompleteOwner(original: Symbol): Symbol = {
// Must have both a class and module symbol, so that `{ class C; def C }` or `{ type T; object T }` are not companions.
def isCompanion(sym: Symbol): Boolean =
(original.isModule && sym.isClass || sym.isModule && original.isClass) && sym.isCoDefinedWith(original)
lookupSibling(original, original.name.companionName).filter(isCompanion)
}

final def lookupCompanionInIncompleteOwner(original: Symbol): Symbol = {
final def lookupSibling(original: Symbol, name: Name): Symbol = {
/* Search scopes in current and enclosing contexts for the definition of `symbol` */
def lookupScopeEntry(symbol: Symbol): ScopeEntry = {
var res: ScopeEntry = null
Expand All @@ -1238,15 +1223,12 @@ trait Contexts { self: Analyzer =>
res
}

// 1) Must be owned by the same Scope, to ensure that in
// `{ class C; { ...; object C } }`, the class is not seen as a companion of the object.
// 2) Must be a class and module symbol, so that `{ class C; def C }` or `{ type T; object T }` are not companions.
// Must be owned by the same Scope, to ensure that in
// `{ class C; { ...; object C } }`, the class is not seen as a companion of the object.
lookupScopeEntry(original) match {
case null => NoSymbol
case entry =>
def isCompanion(sym: Symbol): Boolean =
(original.isModule && sym.isClass || sym.isModule && original.isClass) && sym.isCoDefinedWith(original)
entry.owner.lookupNameInSameScopeAs(original, original.name.companionName).filter(isCompanion)
entry.owner.lookupNameInSameScopeAs(original, name)
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
Original file line number Diff line number Diff line change
Expand Up @@ -479,9 +479,8 @@ trait NamesDefaults { self: Analyzer =>
if (param.owner.owner.isClass) {
param.owner.owner.info.member(defGetterName)
} else {
// the owner of the method is another method. find the default
// getter in the context.
context.lookup(defGetterName, param.owner.owner)
// the owner of the method is another method. find the default getter in the context.
context.lookupSibling(param.owner, defGetterName)
}
}
} else NoSymbol
Expand Down
3 changes: 2 additions & 1 deletion test/files/run/names-defaults-nest.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
object Test {
def multinest = {
def baz = bar();
def baz = {bar()}
def bar(x: String = "a"): Any = {
def bar(x: String = "b") = x
bar() + x
};
bar$default$1(0)
assert(baz == "ba", baz)
}
def main(args: Array[String]) {
Expand Down

0 comments on commit da14e9c

Please sign in to comment.