From 90a98f7c41cfcfdb60dc25db5ac33d0d0ff10f99 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 9 Nov 2014 18:29:35 +1000 Subject: SI-5639 Fix spurious discarding of implicit import In Scala fa0cdc7b (just before 2.9.0), a regression in implicit search, SI-2866, was discovered by Lift and fixed. The nature of the regression was that an in-scope, non-implicit symbol failed to shadow an eponymous implicit import. The fix for this introduced `isQualifyingImplicit` which discards in-scope implicits when the current `Context`'s scope contains a name-clashing entry. Incidentally, this proved to be a shallow solution, and we later improved shadowing detection in SI-4270 / 9129cfe9. That picked up cases where a locally defined symbol in an intervening scope shadowed an implicit. This commit includes the test case from the comments of SI-2866. Part of it is tested as a neg test (to test reporting of ambiguities), and the rest is tested in a run test (to test which implicits are picked.) However, in the test case of SI-5639, we see that the scope lookup performend by `isQualifyingImplicit` is fooled by a "ghost" module symbol. The apparition I'm referring to is entered when `initializeFromClassPath` / `enterClassAndModule` encounters a class file named 'Baz.class', and speculatively enters _both_ a class and module symbol. AFAIK, this is done to defer parsing the class file to determine what inside. If it happens to be a Java compiled class, the module symbol is needed to house the static members. This commit adds a condition that `Symbol#exists` which shines a torch (forces the info) in the direction of the ghost module symbol. In our test, this causes it to vanish, as we only need a class symbol for the Scala defined `class Baz`. The existing `pos` test for this actually did not exercise the bug, separate compilation is required. It was originally checked in to `pending` with this error, and then later moved to `pos` when someone noticed it was not failing. --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (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 e278130437..acd8a6ea7b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -798,7 +798,7 @@ trait Contexts { self: Analyzer => isAccessible(sym, pre) && !(imported && { val e = scope.lookupEntry(name) - (e ne null) && (e.owner == scope) + (e ne null) && (e.owner == scope) && e.sym.exists }) private def collectImplicits(syms: Scope, pre: Type, imported: Boolean = false): List[ImplicitInfo] = -- cgit v1.2.3 From a77f01f546312cf6601f03794f909a09d34c5445 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 9 Nov 2014 18:30:23 +1000 Subject: SI-5639 Predicate bug fix on -Xsource:2.12 To be conservative, I've predicated this fix in `-Xsource:2.12`. This is done in separate commit to show that the previous fix passes the test suite, rather than just tests with `-Xsource:2.12`. --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 2 +- test/files/neg/t5639b.check | 4 ++++ test/files/neg/t5639b/A_1.scala | 17 +++++++++++++++++ test/files/neg/t5639b/A_2.scala | 11 +++++++++++ test/files/pos/t5639.flags | 1 + 5 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t5639b.check create mode 100644 test/files/neg/t5639b/A_1.scala create mode 100644 test/files/neg/t5639b/A_2.scala create mode 100644 test/files/pos/t5639.flags (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 acd8a6ea7b..b13f9e94cc 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -798,7 +798,7 @@ trait Contexts { self: Analyzer => isAccessible(sym, pre) && !(imported && { val e = scope.lookupEntry(name) - (e ne null) && (e.owner == scope) && e.sym.exists + (e ne null) && (e.owner == scope) && (!settings.isScala212 || e.sym.exists) }) private def collectImplicits(syms: Scope, pre: Type, imported: Boolean = false): List[ImplicitInfo] = diff --git a/test/files/neg/t5639b.check b/test/files/neg/t5639b.check new file mode 100644 index 0000000000..faa1766660 --- /dev/null +++ b/test/files/neg/t5639b.check @@ -0,0 +1,4 @@ +A_2.scala:6: error: could not find implicit value for parameter e: Int + implicitly[Int] + ^ +one error found diff --git a/test/files/neg/t5639b/A_1.scala b/test/files/neg/t5639b/A_1.scala new file mode 100644 index 0000000000..c5da10eae4 --- /dev/null +++ b/test/files/neg/t5639b/A_1.scala @@ -0,0 +1,17 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 + // This implicit was being ignored by `isQualifyingImplicit` + // if the classpath contained a class file for `class Baz`. + // This is because the package scope contains a speculative + // symbol for `object Baz` which is entered by `SymbolLoaders` + // before looking inside the class file. (A Java originated + // classfile results in the class/module symbol pair.) +} diff --git a/test/files/neg/t5639b/A_2.scala b/test/files/neg/t5639b/A_2.scala new file mode 100644 index 0000000000..2bb36273e0 --- /dev/null +++ b/test/files/neg/t5639b/A_2.scala @@ -0,0 +1,11 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 +} diff --git a/test/files/pos/t5639.flags b/test/files/pos/t5639.flags new file mode 100644 index 0000000000..0acce1e7ce --- /dev/null +++ b/test/files/pos/t5639.flags @@ -0,0 +1 @@ +-Xsource:2.12 -- cgit v1.2.3