Merge pull request scala#3669 from adriaanm/rebase-3667

SI-8460 Fix regression in implicit divergence recovery

Author: retronym

src/compiler/scala/tools/nsc/typechecker/Implicits.scala

@@ -833,19 +833,41 @@ trait Implicits {
        *  so that if there is a best candidate it can still be selected.
        */
       object DivergentImplicitRecovery {
-        // symbol of the implicit that caused the divergence.
-        // Initially null, will be saved on first diverging expansion.
-        private var implicitSym: Symbol    = _
-        private var countdown: Int = 1
-
-        def sym: Symbol = implicitSym
-        def apply(search: SearchResult, i: ImplicitInfo): SearchResult =
-          if (search.isDivergent && countdown > 0) {
-            countdown -= 1
-            implicitSym = i.sym
-            log(s"discarding divergent implicit $implicitSym during implicit search")
+        private var divergentError: Option[DivergentImplicitTypeError] = None
+
+        private def saveDivergent(err: DivergentImplicitTypeError) {
+          if (divergentError.isEmpty) divergentError = Some(err)
+        }
+
+        def issueSavedDivergentError() {
+          divergentError foreach (err => context.issue(err))
+        }
+
+        def apply(search: SearchResult, i: ImplicitInfo, errors: Seq[AbsTypeError]): SearchResult = {
+          // A divergent error from a nested implicit search will be found in `errors`. Stash that
+          // aside to be re-issued if this implicit search fails.
+          errors.collectFirst { case err: DivergentImplicitTypeError => err } foreach saveDivergent
+
+          if (search.isDivergent && divergentError.isEmpty) {
+            // Divergence triggered by `i` at this level of the implicit serach. We haven't
+            // seen divergence so far, we won't issue this error just yet, and instead temporarily
+            // treat `i` as a failed candidate.
+            saveDivergent(DivergentImplicitTypeError(tree, pt, i.sym))
+            log(s"discarding divergent implicit ${i.sym} during implicit search")
             SearchFailure
-          } else search
+          } else {
+            if (search.isFailure) {
+              // We don't want errors that occur during checking implicit info
+              // to influence the check of further infos, but we should retain divergent implicit errors
+              // (except for the one we already squirreled away)
+              val saved = divergentError.getOrElse(null)
+              context.reportBuffer.retainErrors {
+                case err: DivergentImplicitTypeError => err ne saved
+              }
+            }
+            search
+          }
+        }
       }
 
       /** Sorted list of eligible implicits.
@@ -869,31 +891,33 @@ trait Implicits {
        *   - if it matches, forget about all others it improves upon
        */
       @tailrec private def rankImplicits(pending: Infos, acc: Infos): Infos = pending match {
-        case Nil      => acc
-        case i :: is  =>
-          DivergentImplicitRecovery(typedImplicit(i, ptChecked = true, isLocalToCallsite), i) match {
-            case sr if sr.isDivergent =>
-              Nil
-            case sr if sr.isFailure =>
-              // We don't want errors that occur during checking implicit info
-              // to influence the check of further infos.
-              context.reportBuffer.retainErrors {
-                case err: DivergentImplicitTypeError => true
+        case Nil                          => acc
+        case firstPending :: otherPending =>
+          def firstPendingImproves(alt: ImplicitInfo) =
+            firstPending == alt || (
+              try improves(firstPending, alt)
+              catch {
+                case e: CyclicReference =>
+                  debugwarn(s"Discarding $firstPending during implicit search due to cyclic reference.")
+                  true
               }
-              rankImplicits(is, acc)
-            case newBest        =>
-              best = newBest
-              val newPending = undoLog undo {
-                is filterNot (alt => alt == i || {
-                  try improves(i, alt)
-                  catch {
-                    case e: CyclicReference =>
-                      debugwarn(s"Discarding $i during implicit search due to cyclic reference")
-                      true
-                  }
-                })
+            )
+
+          val typedFirstPending = typedImplicit(firstPending, ptChecked = true, isLocalToCallsite)
+
+          // Pass the errors to `DivergentImplicitRecovery` so that it can note
+          // the first `DivergentImplicitTypeError` that is being propagated
+          // from a nested implicit search; this one will be
+          // re-issued if this level of the search fails.
+          DivergentImplicitRecovery(typedFirstPending, firstPending, context.errors) match {
+            case sr if sr.isDivergent => Nil
+            case sr if sr.isFailure   => rankImplicits(otherPending, acc)
+            case newBest              =>
+              best = newBest // firstPending is our new best, since we already pruned last time around:
+              val pendingImprovingBest = undoLog undo {
+                otherPending filterNot firstPendingImproves
               }
-              rankImplicits(newPending, i :: acc)
+              rankImplicits(pendingImprovingBest, firstPending :: acc)
           }
       }
 
@@ -921,12 +945,9 @@ trait Implicits {
         }
 
         if (best.isFailure) {
-          /* If there is no winner, and we witnessed and caught divergence,
-           * now we can throw it for the error message.
-           */
-          if (DivergentImplicitRecovery.sym != null) {
-            DivergingImplicitExpansionError(tree, pt, DivergentImplicitRecovery.sym)(context)
-          }
+          // If there is no winner, and we witnessed and recorded a divergence error,
+          // our recovery attempt has failed, so we must now issue it.
+          DivergentImplicitRecovery.issueSavedDivergentError()
 
           if (invalidImplicits.nonEmpty)
             setAddendum(pos, () =>

test/files/neg/divergent-implicit.check

@@ -3,6 +3,10 @@ divergent-implicit.scala:4: error: type mismatch;
  required: String
   val x1: String = 1
                    ^
+divergent-implicit.scala:5: error: diverging implicit expansion for type Int => String
+starting with method $conforms in object Predef
+  val x2: String = cast[Int, String](1)
+                                    ^
 divergent-implicit.scala:14: error: type mismatch;
  found   : Test2.Foo
  required: Test2.Bar
@@ -13,4 +17,4 @@ divergent-implicit.scala:15: error: type mismatch;
  required: Test2.Bar
   val y: Bar = new Baz
                ^
-three errors found
+four errors found

test/files/pos/t8460.scala

@@ -0,0 +1,25 @@
+object tan extends UFunc {
+  implicit def ImplDouble: Impl[Double, Double] = ???
+}
+
+trait UFunc {
+  trait TC[+A]
+  type Impl[V, VR] = UFunc.UImpl[this.type, V, VR]
+}
+
+object UFunc {
+  class UImpl[A, B, C]
+  implicit def implicitDoubleUTag[Tag, V, VR](implicit conv: V=>Double, impl: UImpl[Tag, Double, VR]):UImpl[Tag, V, VR] = ???
+
+}
+
+object Test {
+  implicitly[tan.Impl[Double, Double]]
+  // we should discard the one and only divergent implicit (`implicitDoubleUTag`)
+  // This is done under `scalac-hash v2.10.4 test.scala`, but not under
+  // `scalac-hash v2.10.4 -Xdivergence211 test.scala`
+  //
+  // This seems to be because the companion implicits contain redundant entries
+  //
+
+}