Skip to content

Ticket/8460 #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ trait ContextErrors {
// (pt at the point of divergence gives less information to the user)
// Note: it is safe to delay error message generation in this case
// becasue we don't modify implicits' infos.
case class DivergentImplicitTypeError(underlyingTree: Tree, pt0: Type, sym: Symbol)
case class DivergentImplicitTypeError(underlyingTree: Tree, pt0: Type, sym: Symbol, explanation: String)
extends TreeTypeError {
def errMsg: String = errMsgForPt(pt0)
def withPt(pt: Type): AbsTypeError = this.copy(pt0 = pt)
private def errMsgForPt(pt: Type) =
s"diverging implicit expansion for type ${pt}\nstarting with ${sym.fullLocationString}"
s"diverging implicit expansion for type ${pt}\nstarting with ${sym.fullLocationString}.\n$explanation"
}

case class AmbiguousImplicitTypeError(underlyingTree: Tree, errMsg: String)
Expand Down Expand Up @@ -1236,8 +1236,8 @@ trait ContextErrors {
}
}

def DivergingImplicitExpansionError(tree: Tree, pt: Type, sym: Symbol)(implicit context0: Context) =
issueTypeError(DivergentImplicitTypeError(tree, pt, sym))
def DivergingImplicitExpansionError(tree: Tree, pt: Type, sym: Symbol, explanation: String = "")(implicit context0: Context) =
issueTypeError(DivergentImplicitTypeError(tree, pt, sym, explanation))
}

object NamesDefaultsErrorsGen {
Expand Down
80 changes: 49 additions & 31 deletions src/compiler/scala/tools/nsc/typechecker/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,20 +165,20 @@ trait Implicits {
* @param undetparams undeterminted type parameters
*/
class SearchResult(val tree: Tree, val subst: TreeTypeSubstituter, val undetparams: List[Symbol]) {
override def toString = "SearchResult(%s, %s)".format(tree,
if (subst.isEmpty) "" else subst)
override def toString = "%s(%s, %s)".format(getClass.getName, tree, if (subst.isEmpty) "" else subst)

Choose a reason for hiding this comment

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

s"${getClass.getName}($tree, ${if (subst.isEmpty) "" else subst})"


def isFailure = false
def isAmbiguousFailure = false
def isDivergent = false
def explanation: String = ""
final def isSuccess = !isFailure
}

lazy val SearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter, Nil) {
object SearchFailure extends SearchResult(EmptyTree, EmptyTreeTypeSubstituter, Nil) {
override def isFailure = true
}

lazy val DivergentSearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter, Nil) {
final class DivergentSearchFailure(override val explanation: String) extends SearchResult(EmptyTree, EmptyTreeTypeSubstituter, Nil) {
override def isFailure = true
override def isDivergent = true
}
Expand Down Expand Up @@ -443,10 +443,17 @@ trait Implicits {
// otherwise, the macro writer could check `c.openMacros` and `c.openImplicits` and do `c.abort` when expansions are deemed to be divergent
// upon receiving `c.abort` the typechecker will decide that the corresponding implicit search has failed
// which will fail the entire stack of implicit searches, producing a nice error message provided by the programmer
(context.openImplicits find { case OpenImplicit(info, tp, tree1) => !info.sym.isMacro && tree1.symbol == tree.symbol && dominates(pt, tp)}) match {
val dominatedOpenImplicit = context.openImplicits find {
case OpenImplicit(info, tp, tree1) => !info.sym.isMacro && tree1.symbol == tree.symbol && dominates(pt, tp)
}
dominatedOpenImplicit match {
case Some(pending) =>
//println("Pending implicit "+pending+" dominates "+pt+"/"+undetParams) //@MDEBUG
DivergentSearchFailure
val next = (OpenImplicit(info, pt, tree) :: context.openImplicits)
val msg = next.reverse.zipWithIndex.map {
case (OpenImplicit(info, pt, tree), i) =>
s"${" " * i} $tree"
}.mkString("\n")
new DivergentSearchFailure(msg)
case None =>
try {
context.openImplicits = OpenImplicit(info, pt, tree) :: context.openImplicits
Expand Down Expand Up @@ -833,19 +840,30 @@ 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

Choose a reason for hiding this comment

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

'Divergence was (first) triggered' is more clear. (Not sure "first" improves the comment further. Just throwing that out there.)

// 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, search.explanation))

Choose a reason for hiding this comment

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

If I understand correctly, this is the crux of the fix. I assume this is your plan, but it would be ideal structure the PR into pure refactoring + this one-line fix.

lookin' good!

Copy link
Owner Author

Choose a reason for hiding this comment

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

More or less. It is also important to flush the divergent error out of context.errors, otherwise even after supressing it can show up again at a higher level of the nested implicit search.

log(s"discarding divergent implicit ${i.sym} during implicit search")
SearchFailure
} else search
}
}

/** Sorted list of eligible implicits.
Expand All @@ -871,15 +889,17 @@ trait Implicits {
@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 {
val typedImplicitResult = typedImplicit(i, ptChecked = true, isLocalToCallsite)
// We don't want errors that occur during checking implicit info
// to influence the check of further infos. But we do need to pass the errors
// to `DivergentImplicitRecovery` so that it can note `DivergentImplicitTypeError` that is
// being propagated from a nested implicit search; this will be re-issued if this level
// of the search fails.
val recoveredResult = DivergentImplicitRecovery(typedImplicitResult, i, context.flushAndReturnBuffer())
recoveredResult 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
}
rankImplicits(is, acc)
case newBest =>
best = newBest
Expand Down Expand Up @@ -907,7 +927,8 @@ trait Implicits {
// After calling rankImplicits, the least frequent matching one is first and
// earlier elems may improve on later ones, but not the other way.
// So if there is any element not improved upon by the first it is an error.
rankImplicits(eligible, Nil) match {
val ranked: Infos = rankImplicits(eligible, Nil)
ranked match {
case Nil => ()
case chosen :: rest =>
rest find (alt => !improves(chosen, alt)) match {
Expand All @@ -921,12 +942,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 recoreded a divergence error,
// our recovery attempt has failed, so we must now issue it.
DivergentImplicitRecovery.issueSavedDivergentError()

if (invalidImplicits.nonEmpty)
setAddendum(pos, () =>
Expand Down
9 changes: 8 additions & 1 deletion test/files/neg/divergent-implicit.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ 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.
Test1.this.cast[Int, String](1)
Test1.this.cast[Int, Nothing](x)
Test1.this.cast[Int, Nothing](x)
val x2: String = cast[Int, String](1)
^
Copy link
Owner Author

Choose a reason for hiding this comment

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

We lost this error message in SI-3346 / 210dbc7 under the comment:

  1. neg/divergent_implicit.scala, neg/t5578.scala and neg/t7519.scala
    changed their messages to less informative ones, because inapplicable
    implicit views now get disqualified early and therefore don't display
    their error messages to the user. This is an unfortunate but necessary
    byproduct of this change, and one can argue that the behavior is now
    completely consistent with implicit vals (that also don't explain why
    this or that implicit val got disqualified, but rather display a generic
    implicit value not found message).

2.11.0-RC3 issues this if the preceding line in the test is commented out. So there was some cross-talk in play, and restoring it here is a win.

divergent-implicit.scala:14: error: type mismatch;
found : Test2.Foo
required: Test2.Bar
Expand All @@ -13,4 +20,4 @@ divergent-implicit.scala:15: error: type mismatch;
required: Test2.Bar
val y: Bar = new Baz
^
three errors found
four errors found
5 changes: 4 additions & 1 deletion test/files/neg/t5318.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
t5318.scala:7: error: diverging implicit expansion for type CompilerHang.this.TC[F]
starting with method tc in class CompilerHang
starting with method tc in class CompilerHang.
CompilerHang.this.breakage[F]
CompilerHang.this.tc[M]
CompilerHang.this.tc[M]
breakage // type checker doesn't terminate, should report inference failure
^
one error found
7 changes: 5 additions & 2 deletions test/files/neg/t5318b.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
t5318b.scala:7: error: diverging implicit expansion for type DivergingImplicitReported.this.TC[F]
starting with method tc in class DivergingImplicitReported
starting with method tc in class DivergingImplicitReported.
DivergingImplicitReported.this.breakage[F]
DivergingImplicitReported.this.tc[M]
DivergingImplicitReported.this.tc[M]
breakage // correct: diverging implicit expansion
^
one error found
one error found
5 changes: 4 additions & 1 deletion test/files/neg/t5318c.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
t5318c.scala:13: error: diverging implicit expansion for type CompilerHang.this.TC[F]
starting with method tc in class CompilerHang
starting with method tc in class CompilerHang.
CompilerHang.this.breakage[F]
CompilerHang.this.tc[x, CC]
CompilerHang.this.tc[x, CC]
breakage // type checker doesn't terminate, should report inference failure
^
one error found
10 changes: 8 additions & 2 deletions test/files/neg/t696.check
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
t696.scala:5: error: diverging implicit expansion for type TypeUtil0.Type[Any]
starting with method WithType in object TypeUtil0
starting with method WithType in object TypeUtil0.
TypeUtil0.this.as[Any](null)
TypeUtil0.WithType[Any, Any]
TypeUtil0.WithType[Any, Any]
as[Any](null)
^
t696.scala:6: error: diverging implicit expansion for type TypeUtil0.Type[X]
starting with method WithType in object TypeUtil0
starting with method WithType in object TypeUtil0.
TypeUtil0.this.as[X](null)
TypeUtil0.WithType[S, Any]
TypeUtil0.WithType[S, T]
def foo[X]() = as[X](null)
^
two errors found
25 changes: 25 additions & 0 deletions test/files/t8460.scala
Original file line number Diff line number Diff line change
@@ -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
//

}