Skip to content

Commit

Permalink
SI-8460 Fix regression in divergent implicit recovery
Browse files Browse the repository at this point in the history
Implicit search detects likely cycles by looking at the stack of
open implicits and checking the same implicit appears twice, and
if the second occurrence is trying satisfy an implicit search for
a "dominant" type.

Originally, this condition immediately failed the entire implicit
search. However, since Scala 2.10, this mechanism has been refined to
continue searching after the first divergent implicit is detected.
If a second divergence is found, we fail immediately. If the followup
search fails, we report the first divergence. Otherwise, we
take the successful result.

This mechanism was originally built around exceptions. This proved
to be fragile, and was refactored in SI-7291 / accaa31 to instead
use the `Context.errors` to control the process.

But, since that change, the pattern of implicits in scalanlp/breeze
and Shapeless have been prone to reporting the divergent implicit
errors where they used to recover.

So long as we left the `DivergentImplictTypeError` that originates
from a nested implicit search in `context.errors`, we are unable to
successfully typecheck other candidates. This commit instead
stashes the first such error away in `DivergentImplicitRecovery`,
to clear the way for the alternative path to succeed.

We must retain any other divergent implicit errors, as witnessed by
test/files/neg/t2031.scala, which loops unless we retain divergent
implicit errors that we don't stash in `DivergentImplicitRecovery`.
  • Loading branch information
retronym authored and adriaanm committed Mar 31, 2014
1 parent 5e795fc commit 1c330e6
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
21 changes: 17 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/Implicits.scala
Expand Up @@ -839,11 +839,22 @@ trait Implicits {
if (divergentError.isEmpty) divergentError = Some(err) if (divergentError.isEmpty) divergentError = Some(err)
} }


def retainRemainingDivergentErrors() = {
val saved = divergentError.getOrElse(null)
context.reportBuffer.retainErrors {
case err: DivergentImplicitTypeError => err ne saved
}
}

def issueSavedDivergentError() { def issueSavedDivergentError() {
divergentError foreach (err => context.issue(err)) divergentError foreach (err => context.issue(err))
} }


def apply(search: SearchResult, i: ImplicitInfo, errors: Seq[AbsTypeError]): SearchResult = { 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) { if (search.isDivergent && divergentError.isEmpty) {
// Divergence triggered by `i` at this level of the implicit serach. We haven't // 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 // seen divergence so far, we won't issue this error just yet, and instead temporarily
Expand Down Expand Up @@ -879,16 +890,18 @@ trait Implicits {
case Nil => acc case Nil => acc
case i :: is => case i :: is =>
val typedImplicitResult = typedImplicit(i, ptChecked = true, isLocalToCallsite) val typedImplicitResult = typedImplicit(i, 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.
val recoveredResult = DivergentImplicitRecovery(typedImplicitResult, i, context.errors) val recoveredResult = DivergentImplicitRecovery(typedImplicitResult, i, context.errors)
recoveredResult match { recoveredResult match {
case sr if sr.isDivergent => case sr if sr.isDivergent =>
Nil Nil
case sr if sr.isFailure => case sr if sr.isFailure =>
// We don't want errors that occur during checking implicit info // We don't want errors that occur during checking implicit info
// to influence the check of further infos. // to influence the check of further infos, but we should retain divergent implicit errors
context.reportBuffer.retainErrors { // (except for the one we already squirreled away)
case err: DivergentImplicitTypeError => true DivergentImplicitRecovery.retainRemainingDivergentErrors()
}
rankImplicits(is, acc) rankImplicits(is, acc)
case newBest => case newBest =>
best = newBest best = newBest
Expand Down
6 changes: 5 additions & 1 deletion test/files/neg/divergent-implicit.check
Expand Up @@ -3,6 +3,10 @@ divergent-implicit.scala:4: error: type mismatch;
required: String required: String
val x1: String = 1 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; divergent-implicit.scala:14: error: type mismatch;
found : Test2.Foo found : Test2.Foo
required: Test2.Bar required: Test2.Bar
Expand All @@ -13,4 +17,4 @@ divergent-implicit.scala:15: error: type mismatch;
required: Test2.Bar required: Test2.Bar
val y: Bar = new Baz val y: Bar = new Baz
^ ^
three errors found four errors found
File renamed without changes.

1 comment on commit 1c330e6

@retronym
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a precarious tightrope on which we tread. Thanks for the patch! Will digest it and report back.

Please sign in to comment.