From 37037fe70651eee7a1e8a77a2f8b6e74968836b6 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 17 Nov 2016 12:49:59 +1000 Subject: SI-3772 Fix detection of term-owned companions Companion detection consults the scopes of enclosing Contexts during typechecking to avoid the cycles that would ensue if we had to look at into the info of enclosing class symbols. For example, this used to typecheck: object CC { val outer = 42 } if ("".isEmpty) { case class CC(c: Int) CC.outer } This logic was not suitably hardened to find companions in exactly the same nesting level. After fixing this problem, a similar problem in `Namer::inCurrentScope` could be solved to be more selective about synthesizing a companion object. In particular, if a manually defined companion trails after the case class, don't create an addiotional synthetic comanpanion object. --- .../scala/tools/nsc/typechecker/Contexts.scala | 21 ++++++++++++++++ .../scala/tools/nsc/typechecker/Namers.scala | 10 ++++---- src/reflect/scala/reflect/internal/Scopes.scala | 29 ++++++++++++++++++++++ test/files/neg/t3772.check | 7 ++++++ test/files/neg/t3772.scala | 17 +++++++++++++ test/files/neg/t8002-nested-scope.check | 4 +++ test/files/neg/t8002-nested-scope.scala | 12 +++++++++ test/files/pos/t3772.scala | 8 ++++++ test/files/pos/t8002-nested-scope.scala | 20 --------------- 9 files changed, 103 insertions(+), 25 deletions(-) create mode 100644 test/files/neg/t3772.check create mode 100644 test/files/neg/t3772.scala create mode 100644 test/files/neg/t8002-nested-scope.check create mode 100644 test/files/neg/t8002-nested-scope.scala create mode 100644 test/files/pos/t3772.scala delete mode 100644 test/files/pos/t8002-nested-scope.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index c73ea54c3d..cbe9e522a2 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -1192,6 +1192,27 @@ trait Contexts { self: Analyzer => } res } + + final def lookupCompanionOf(original: Symbol): Symbol = { + lookupScopeEntry(original) match { + case null => NoSymbol + case entry => entry.owner.lookupCompanion(original).filter(_.isCoDefinedWith(original)) + } + } + + /** Search scopes in current and enclosing contexts for the definition of `symbol` */ + private def lookupScopeEntry(symbol: Symbol): ScopeEntry = { + var res: ScopeEntry = null + var ctx = this + while (res == null && ctx.outer != ctx) { + val s = ctx.scope lookupSymbolEntry symbol + if (s != null) + res = s + else + ctx = ctx.outer + } + res + } } //class Context /** A `Context` focussed on an `Import` tree */ diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 78e8c8c073..60bda3ff9f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -220,7 +220,10 @@ trait Namers extends MethodSynthesis { private def inCurrentScope(m: Symbol): Boolean = { if (owner.isClass) owner == m.owner - else m.owner.isClass && context.scope == m.owner.info.decls + else context.scope.lookupSymbolEntry(m) match { + case null => false + case entry => entry.owner eq context.scope + } } /** Enter symbol into context's scope and return symbol itself */ @@ -1923,10 +1926,7 @@ trait Namers extends MethodSynthesis { // use the lower-level scan through the current Context as a fall back. if (!currentRun.compiles(owner)) owner.initialize original.companionSymbol orElse { - ctx.lookup(original.name.companionName, owner).suchThat(sym => - (original.isTerm || sym.hasModuleFlag) && - (sym isCoDefinedWith original) - ) + ctx.lookupCompanionOf(original) } } diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala index a7bb127506..0df8358d7d 100644 --- a/src/reflect/scala/reflect/internal/Scopes.scala +++ b/src/reflect/scala/reflect/internal/Scopes.scala @@ -282,6 +282,35 @@ trait Scopes extends api.Scopes { self: SymbolTable => } } + final def lookupSymbolEntry(sym: Symbol): ScopeEntry = { + var e = lookupEntry(sym.name) + while (e ne null) { + if (e.sym == sym) return e + e = lookupNextEntry(e) + } + null + } + + final def lookupCompanion(original: Symbol): Symbol = { + lookupSymbolEntry(original) match { + case null => + case entry if entry.sym == original => + var e = lookupEntry(original.name.companionName) + while (e != null) { + // 1) Must be owned by the same Scope, to ensure that in + // `{ class C; { ...; object C } }`, the class is not seen as a comaniopn of the object. + // 2) Must be in exactly the same scope, so that `{ class C; def C }` are not companions + // TODO Exclude type aliases as companions of terms? + if ((e.owner eq entry.owner) && (original.isTerm || e.sym.hasModuleFlag)) { + return e.sym + } + e = lookupNextEntry(e) + } + case _ => + } + NoSymbol + } + /** lookup a symbol entry matching given name. * @note from Martin: I believe this is a hotspot or will be one * in future versions of the type system. I have reverted the previous diff --git a/test/files/neg/t3772.check b/test/files/neg/t3772.check new file mode 100644 index 0000000000..d1ed39d8b6 --- /dev/null +++ b/test/files/neg/t3772.check @@ -0,0 +1,7 @@ +t3772.scala:7: error: value inner is not a member of object CC + CC.inner + ^ +t3772.scala:14: error: value outer is not a member of object CC + CC.outer + ^ +two errors found diff --git a/test/files/neg/t3772.scala b/test/files/neg/t3772.scala new file mode 100644 index 0000000000..cac4932d4a --- /dev/null +++ b/test/files/neg/t3772.scala @@ -0,0 +1,17 @@ +class Test { + def m = { + case class CC(c: Int) + if ("".isEmpty) { + object CC { def inner = 42} + } + CC.inner + } + def n = { + object CC { val outer = 42 } + if ("".isEmpty) { + case class CC(c: Int) + CC(0).c + CC.outer + } + } +} diff --git a/test/files/neg/t8002-nested-scope.check b/test/files/neg/t8002-nested-scope.check new file mode 100644 index 0000000000..f66249e432 --- /dev/null +++ b/test/files/neg/t8002-nested-scope.check @@ -0,0 +1,4 @@ +t8002-nested-scope.scala:8: error: method x in class C cannot be accessed in C + new C().x + ^ +one error found diff --git a/test/files/neg/t8002-nested-scope.scala b/test/files/neg/t8002-nested-scope.scala new file mode 100644 index 0000000000..44704a12b1 --- /dev/null +++ b/test/files/neg/t8002-nested-scope.scala @@ -0,0 +1,12 @@ +class C { + def foo = { + class C { private def x = 0 } + + { + val a = 0 + object C { + new C().x + } + } + } +} diff --git a/test/files/pos/t3772.scala b/test/files/pos/t3772.scala new file mode 100644 index 0000000000..62c433ebd1 --- /dev/null +++ b/test/files/pos/t3772.scala @@ -0,0 +1,8 @@ +class Test { + def m = { + case class C(c: Int) + object C { def xxx = true} + C(42).c + C.xxx + } +} diff --git a/test/files/pos/t8002-nested-scope.scala b/test/files/pos/t8002-nested-scope.scala deleted file mode 100644 index 8ce809e556..0000000000 --- a/test/files/pos/t8002-nested-scope.scala +++ /dev/null @@ -1,20 +0,0 @@ -// This test serves to capture the status quo, but should really -// emit an accessibility error. - -// `Namers#companionSymbolOf` seems too lenient, and currently doesn't -// implement the same-scope checks mentioned: -// -// https://github.com/scala/scala/pull/2816#issuecomment-22555206 -// -class C { - def foo = { - class C { private def x = 0 } - - { - val a = 0 - object C { - new C().x - } - } - } -} -- cgit v1.2.3 From 9502a061fa007ceb1d4e550fdb386a3645c67b1c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 29 Nov 2016 21:55:14 +1000 Subject: Refactor companion lookup methods after code review --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 2 +- src/reflect/scala/reflect/internal/Scopes.scala | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index cbe9e522a2..7988ac9f16 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -1196,7 +1196,7 @@ trait Contexts { self: Analyzer => final def lookupCompanionOf(original: Symbol): Symbol = { lookupScopeEntry(original) match { case null => NoSymbol - case entry => entry.owner.lookupCompanion(original).filter(_.isCoDefinedWith(original)) + case entry => entry.owner.lookupCompanion(original) } } diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala index 0df8358d7d..32ce02bdd6 100644 --- a/src/reflect/scala/reflect/internal/Scopes.scala +++ b/src/reflect/scala/reflect/internal/Scopes.scala @@ -294,19 +294,18 @@ trait Scopes extends api.Scopes { self: SymbolTable => final def lookupCompanion(original: Symbol): Symbol = { lookupSymbolEntry(original) match { case null => - case entry if entry.sym == original => + case entry => var e = lookupEntry(original.name.companionName) while (e != null) { // 1) Must be owned by the same Scope, to ensure that in // `{ class C; { ...; object C } }`, the class is not seen as a comaniopn of the object. - // 2) Must be in exactly the same scope, so that `{ class C; def C }` are not companions - // TODO Exclude type aliases as companions of terms? - if ((e.owner eq entry.owner) && (original.isTerm || e.sym.hasModuleFlag)) { - return e.sym + // 2) Must be a class and module symbol, so that `{ class C; def C }` or `{ type T; object T }` are not companions. + def isClassAndModule(sym1: Symbol, sym2: Symbol) = sym1.isClass && sym2.isModule + if ((e.owner eq entry.owner) && (isClassAndModule(original, e.sym) || isClassAndModule(e.sym, original))) { + return if (e.sym.isCoDefinedWith(original)) e.sym else NoSymbol } e = lookupNextEntry(e) } - case _ => } NoSymbol } -- cgit v1.2.3