SI-8941 Idempotent presentation compilation of implicit classes

When we name an implicit class, `enterImplicitWrapper` is called,
which enters the symbol for the factory method into the
owning scope. The tree defining this factory method is stowed into
`unit.synthetics`, from whence it will be retrieved and incorporated
into the enclosing tree during typechecking (`addDerivedTrees`).
The entry in `unit.synthetics` is removed at that point.

However, in the presentation compiler, we can typecheck a unit
more than once in a single run. For example, if, as happens
in the enclosed test, a call to ask for a type at a given
position interrupts type checking of the entire unit, we
can get into a situation whereby the first type checking
invocation has consumed the entry from `unit.synthetics`,
and the second will crash when it can't find an entry.

Similar problems have been solved in the past in
`enterExistingSym` in the presentation compiler. This method
is called when the namer encounters a tree that already has
a symbol attached. See 0b78a0196 / 148736c3df.

This commit takes a two pronged approach.

First, `enterExistingSym` is extended to handle implicit classes.
Any previous factory method in the owning scope is removed, and
`enterImplicitWrapper` is called to place a new tree for the factory
into `unit.synthetics` and to enter its symbol into the owning scope.

Second, the assertions that could be tripped in `addDerivedTrees`
and in `ImplicitClassWrapper#derivedSym` have been converted to
positioned errors.

The first change is sufficient to fix this bug, but the second
is also enough to make the enclosed test pass, and has been retained
as an extra layer of defence.

6 files changed, 43 insertions, 7 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
index ba183fe3e6..0aa62d771e 100644
--- a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
@@ -212,7 +212,9 @@ trait MethodSynthesis {
             List(cd, mdef)
           case _ =>
             // Shouldn't happen, but let's give ourselves a reasonable error when it does
-            abort("No synthetics for " + meth + ": synthetics contains " + context.unit.synthetics.keys.mkString(", "))
+            context.error(cd.pos, s"Internal error: Symbol for synthetic factory method not found among ${context.unit.synthetics.keys.mkString(", ")}")
+            // Soldier on for the sake of the presentation compiler
+            List(cd)
         }
       case _ =>
         stat :: Nil
@@ -355,8 +357,9 @@ trait MethodSynthesis {
       def derivedSym: Symbol = {
         // Only methods will do! Don't want to pick up any stray
         // companion objects of the same name.
-        val result = enclClass.info decl name suchThat (x => x.isMethod && x.isSynthetic)
-        assert(result != NoSymbol, "not found: "+name+" in "+enclClass+" "+enclClass.info.decls)
+        val result = enclClass.info decl name filter (x => x.isMethod && x.isSynthetic)
+        if (result == NoSymbol || result.isOverloaded)
+          context.error(tree.pos, s"Internal error: Unable to find the synthetic factory method corresponding to implicit class $name in $enclClass / ${enclClass.info.decls}")
         result
       }
       def derivedTree: DefDef          =
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index e876d4a6af..8bdf5c33ba 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -296,7 +296,7 @@ trait Namers extends MethodSynthesis {
       }
       tree.symbol match {
         case NoSymbol => try dispatch() catch typeErrorHandler(tree, this.context)
-        case sym      => enterExistingSym(sym)
+        case sym      => enterExistingSym(sym, tree)
       }
     }
 
@@ -736,7 +736,9 @@ trait Namers extends MethodSynthesis {
     }
 
     // Hooks which are overridden in the presentation compiler
-    def enterExistingSym(sym: Symbol): Context = this.context
+    def enterExistingSym(sym: Symbol, tree: Tree): Context = {
+      this.context
+    }
     def enterIfNotThere(sym: Symbol) { }
 
     def enterSyntheticSym(tree: Tree): Symbol = {
diff --git a/src/interactive/scala/tools/nsc/interactive/Global.scala b/src/interactive/scala/tools/nsc/interactive/Global.scala
index 174254d523..e308bc3bf0 100644
--- a/src/interactive/scala/tools/nsc/interactive/Global.scala
+++ b/src/interactive/scala/tools/nsc/interactive/Global.scala
@@ -64,7 +64,7 @@ trait InteractiveAnalyzer extends Analyzer {
     // that case the definitions that were already attributed as
     // well as any default parameters of such methods need to be
     // re-entered in the current scope.
-    override def enterExistingSym(sym: Symbol): Context = {
+    override def enterExistingSym(sym: Symbol, tree: Tree): Context = {
       if (sym != null && sym.owner.isTerm) {
         enterIfNotThere(sym)
         if (sym.isLazy)
@@ -72,8 +72,13 @@ trait InteractiveAnalyzer extends Analyzer {
 
         for (defAtt <- sym.attachments.get[DefaultsOfLocalMethodAttachment])
           defAtt.defaultGetters foreach enterIfNotThere
+      } else if (sym != null && sym.isClass && sym.isImplicit) {
+        val owningInfo = sym.owner.info
+        val existingDerivedSym = owningInfo.decl(sym.name.toTermName).filter(sym => sym.isSynthetic && sym.isMethod)
+        existingDerivedSym.alternatives foreach (owningInfo.decls.unlink)
+        enterImplicitWrapper(tree.asInstanceOf[ClassDef])
       }
-      super.enterExistingSym(sym)
+      super.enterExistingSym(sym, tree)
     }
     override def enterIfNotThere(sym: Symbol) {
       val scope = context.scope
diff --git a/test/files/presentation/t8941.check b/test/files/presentation/t8941.check
new file mode 100644
index 0000000000..341804903a
--- /dev/null
+++ b/test/files/presentation/t8941.check
@@ -0,0 +1,7 @@
+reload: Source.scala
+
+askType at Source.scala(6,7)
+================================================================================
+[response] askTypeAt (6,7)
+scala.this.Predef.???
+================================================================================
diff --git a/test/files/presentation/t8941/Runner.scala b/test/files/presentation/t8941/Runner.scala
new file mode 100644
index 0000000000..0a8923a583
--- /dev/null
+++ b/test/files/presentation/t8941/Runner.scala
@@ -0,0 +1,11 @@
+import scala.tools.nsc.interactive.tests.InteractiveTest
+
+object Test extends InteractiveTest {
+  override def runDefaultTests() {
+    // make sure typer is done.. the virtual pattern matcher might translate
+    // some trees and mess up positions. But we'll catch it red handed!
+    // sourceFiles foreach (src => askLoadedTyped(src).get)
+    super.runDefaultTests()
+  }
+
+}
diff --git a/test/files/presentation/t8941/src/Source.scala b/test/files/presentation/t8941/src/Source.scala
new file mode 100644
index 0000000000..7438cccb03
--- /dev/null
+++ b/test/files/presentation/t8941/src/Source.scala
@@ -0,0 +1,8 @@
+object Foo {
+  implicit class MatCreator(val ctx: StringContext) extends AnyVal {
+    def m(args: Any*): Unit = {
+      ctx.checkLengths(args)
+    }
+    ???/*?*/
+  }
+}