SI-10081 Avoid diverging implicit scope search

[szeiger]

Storing empty infos into the infoMap (thus avoiding recomputation)
prevents the endless recursion.

This is an alternative to #5734 that
avoids the expensive annotation removal and the regression in
pos/t1203a.scala.

[szeiger]

Rebased on top of 2.12.x head in order to build a version that also includes #5708, so I can use it in collection-strawman.

[retronym]

I've started to take a look at this. One concrete suggestion is to filter out the Nil entries after the computation.

commit 27fb723071fe7b1263df00fea67c1db8eaa2e9d9
Author: Jason Zaugg <jzaugg@gmail.com>
Date:   Sat Mar 4 13:38:02 2017 +1000

    Remove empty entries (cycle markers) from companionImplicitMap after computation

diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
index 5b89faa3e8..4a39752a91 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -1130,7 +1130,7 @@ trait Implicits {
         case None =>
           val start = if (Statistics.canEnable) Statistics.startTimer(subtypeETNanos) else null
           //        val implicitInfoss = companionImplicits(pt)
-          val implicitInfoss1 = companionImplicitMap(pt).valuesIterator.toList
+          val implicitInfoss1 = companionImplicitMap(pt).valuesIterator.filterNot(_.isEmpty).toList
           //        val is1 = implicitInfoss.flatten.toSet
           //        val is2 = implicitInfoss1.flatten.toSet
           //        for (i <- is1)

I'm still trying to reason about whether or not this could change the result of this method for some type that currently does not diverge.

Here's what I've tried so far. I was worried this would

scala> class C; object C { implicit def foo = 1 }
defined class C
defined object C

scala> :power
Power mode enabled. :phase is at typer.
import scala.tools.nsc._, intp.global._, definitions._
Try :help or completions for vals._ and power._

scala> val search = new analyzer.ImplicitSearch(EmptyTree, NoType, false, analyzer.NoContext, NoPosition)
search: $r.intp.global.analyzer.ImplicitSearch = scala.tools.nsc.typechecker.Implicits$ImplicitSearch@6b2f16b9

scala> val method = search.getClass.getDeclaredMethods.find(_.getName == "companionImplicitMap").get
method: java.lang.reflect.Method = private scala.collection.mutable.LinkedHashMap scala.tools.nsc.typechecker.Implicits$ImplicitSearch.companionImplicitMap(scala.reflect.internal.Types$Type)

scala> method.setAccessible(true)

scala> method.invoke(search, typeOf[(Some[String], Some[C])]).asInstanceOf[collection.Map[Symbol, List[Symbol]]].filter(_._2.nonEmpty)
here
res9: scala.collection.Map[$r.intp.global.Symbol,List[$r.intp.global.Symbol]] = Map(class Option -> List(option2Iterable: ?), class C -> List(foo: ?))

This shows that your change does not prevent recursion into the type arguments (C) of a class that has already been visited (Some), which is good! But I still want to try a bit harder to find a counter example / understand the code better.

[szeiger]

This shows that your change does not prevent recursion into the type arguments

That was my concern as well. I convinced myself that the change would be OK because this shouldn't depend on coming up with empty or non-empty infos for the symbol.

[lrytz]

I took this chance to get a little more familiar with the implementation of implicit search, looks good from what I can see. I agree with Jason that filtering out empty infos would be cleaner.

[szeiger]

Replaced by #5951