Skip to content

Commit accaa31

Browse files
committed
SI-7291: No exception throwing for diverging implicit expansion
Since we don't throw exceptions for normal errors it was a bit odd that we don't do that for DivergingImplicit. As SI-7291 shows, the logic behind catching/throwing exception was broken for divergence. Instead of patching it, I rewrote the mechanism so that we now another SearchFailure type related to diverging expansion, similar to ambiguous implicit scenario. The logic to prevent diverging expansion from stopping the search had to be slightly adapted but works as usual. The upside is that we don't have to catch diverging implicit for example in the presentation compiler which was again showing that something was utterly broken with the exception approach.
1 parent f33af58 commit accaa31

File tree

9 files changed

+116
-76
lines changed

9 files changed

+116
-76
lines changed

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,23 @@ trait ContextErrors {
5959
def errPos = tree.pos
6060
}
6161

62+
// Unlike other type errors diverging implicit expansion
63+
// will be re-issued explicitly on failed implicit argument search.
64+
// This is because we want to:
65+
// 1) provide better error message than just "implicit not found"
66+
// 2) provide the type of the implicit parameter for which we got diverging expansion
67+
// (pt at the point of divergence gives less information to the user)
68+
// Note: it is safe to delay error message generation in this case
69+
// becasue we don't modify implicits' infos.
70+
case class DivergentImplicitTypeError(tree: Tree, pt0: Type, sym: Symbol) extends AbsTypeError {
71+
def errPos: Position = tree.pos
72+
def errMsg: String = errMsgForPt(pt0)
73+
def kind = ErrorKinds.Divergent
74+
def withPt(pt: Type): AbsTypeError = NormalTypeError(tree, errMsgForPt(pt), kind)
75+
private def errMsgForPt(pt: Type) =
76+
s"diverging implicit expansion for type ${pt}\nstarting with ${sym.fullLocationString}"
77+
}
78+
6279
case class AmbiguousTypeError(underlyingTree: Tree, errPos: Position, errMsg: String, kind: ErrorKind = ErrorKinds.Ambiguous) extends AbsTypeError
6380

6481
case class PosAndMsgTypeError(errPos: Position, errMsg: String, kind: ErrorKind = ErrorKinds.Normal) extends AbsTypeError
@@ -72,9 +89,6 @@ trait ContextErrors {
7289
issueTypeError(SymbolTypeError(sym, msg))
7390
}
7491

75-
def issueDivergentImplicitsError(tree: Tree, msg: String)(implicit context: Context) {
76-
issueTypeError(NormalTypeError(tree, msg, ErrorKinds.Divergent))
77-
}
7892

7993
def issueAmbiguousTypeError(pre: Type, sym1: Symbol, sym2: Symbol, err: AmbiguousTypeError)(implicit context: Context) {
8094
context.issueAmbiguousError(pre, sym1, sym2, err)
@@ -1182,9 +1196,7 @@ trait ContextErrors {
11821196
}
11831197

11841198
def DivergingImplicitExpansionError(tree: Tree, pt: Type, sym: Symbol)(implicit context0: Context) =
1185-
issueDivergentImplicitsError(tree,
1186-
"diverging implicit expansion for type "+pt+"\nstarting with "+
1187-
sym.fullLocationString)
1199+
issueTypeError(DivergentImplicitTypeError(tree, pt, sym))
11881200
}
11891201

11901202
object NamesDefaultsErrorsGen {

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

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ trait Implicits {
8080
printTyping("typing implicit: %s %s".format(tree, context.undetparamsString))
8181
val implicitSearchContext = context.makeImplicit(reportAmbiguous)
8282
val result = new ImplicitSearch(tree, pt, isView, implicitSearchContext, pos).bestImplicit
83-
if (saveAmbiguousDivergent && implicitSearchContext.hasErrors) {
83+
if (result.isFailure && saveAmbiguousDivergent && implicitSearchContext.hasErrors) {
8484
context.updateBuffer(implicitSearchContext.reportBuffer.errors.filter(err => err.kind == ErrorKinds.Ambiguous || err.kind == ErrorKinds.Divergent))
8585
debuglog("update buffer: " + implicitSearchContext.reportBuffer.errors)
8686
}
@@ -152,13 +152,19 @@ trait Implicits {
152152

153153
def isFailure = false
154154
def isAmbiguousFailure = false
155+
def isDivergent = false
155156
final def isSuccess = !isFailure
156157
}
157158

158159
lazy val SearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter) {
159160
override def isFailure = true
160161
}
161162

163+
lazy val DivergentSearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter) {
164+
override def isFailure = true
165+
override def isDivergent = true
166+
}
167+
162168
lazy val AmbiguousSearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter) {
163169
override def isFailure = true
164170
override def isAmbiguousFailure = true
@@ -397,22 +403,18 @@ trait Implicits {
397403
(context.openImplicits find { case (tp, tree1) => tree1.symbol == tree.symbol && dominates(pt, tp)}) match {
398404
case Some(pending) =>
399405
//println("Pending implicit "+pending+" dominates "+pt+"/"+undetParams) //@MDEBUG
400-
throw DivergentImplicit
406+
DivergentSearchFailure
401407
case None =>
402408
try {
403409
context.openImplicits = (pt, tree) :: context.openImplicits
404410
// println(" "*context.openImplicits.length+"typed implicit "+info+" for "+pt) //@MDEBUG
405-
typedImplicit0(info, ptChecked, isLocal)
406-
} catch {
407-
case ex: DivergentImplicit =>
411+
val result = typedImplicit0(info, ptChecked, isLocal)
412+
if (result.isDivergent) {
408413
//println("DivergentImplicit for pt:"+ pt +", open implicits:"+context.openImplicits) //@MDEBUG
409-
if (context.openImplicits.tail.isEmpty) {
410-
if (!(pt.isErroneous))
411-
DivergingImplicitExpansionError(tree, pt, info.sym)(context)
412-
SearchFailure
413-
} else {
414-
throw DivergentImplicit
415-
}
414+
if (context.openImplicits.tail.isEmpty && !pt.isErroneous)
415+
DivergingImplicitExpansionError(tree, pt, info.sym)(context)
416+
}
417+
result
416418
} finally {
417419
context.openImplicits = context.openImplicits.tail
418420
}
@@ -789,16 +791,21 @@ trait Implicits {
789791
/** Preventing a divergent implicit from terminating implicit search,
790792
* so that if there is a best candidate it can still be selected.
791793
*/
792-
private var divergence = false
793-
private val divergenceHandler: PartialFunction[Throwable, SearchResult] = {
794-
var remaining = 1;
795-
{ case x: DivergentImplicit if remaining > 0 =>
796-
remaining -= 1
797-
divergence = true
798-
log("discarding divergent implicit during implicit search")
794+
object DivergentImplicitRecovery {
795+
// symbol of the implicit that caused the divergence.
796+
// Initially null, will be saved on first diverging expansion.
797+
private var implicitSym: Symbol = _
798+
private var countdown: Int = 1
799+
800+
def sym: Symbol = implicitSym
801+
def apply(search: SearchResult, i: ImplicitInfo): SearchResult =
802+
if (search.isDivergent && countdown > 0) {
803+
countdown -= 1
804+
implicitSym = i.sym
805+
log("discarding divergent implicit ${implicitSym} during implicit search")
799806
SearchFailure
800-
}
801-
}
807+
} else search
808+
}
802809

803810
/** Sorted list of eligible implicits.
804811
*/
@@ -826,11 +833,9 @@ trait Implicits {
826833
@tailrec private def rankImplicits(pending: Infos, acc: Infos): Infos = pending match {
827834
case Nil => acc
828835
case i :: is =>
829-
def tryImplicitInfo(i: ImplicitInfo) =
830-
try typedImplicit(i, ptChecked = true, isLocal)
831-
catch divergenceHandler
832-
833-
tryImplicitInfo(i) match {
836+
DivergentImplicitRecovery(typedImplicit(i, ptChecked = true, isLocal), i) match {
837+
case sr if sr.isDivergent =>
838+
Nil
834839
case sr if sr.isFailure =>
835840
// We don't want errors that occur during checking implicit info
836841
// to influence the check of further infos.
@@ -882,10 +887,9 @@ trait Implicits {
882887
/* If there is no winner, and we witnessed and caught divergence,
883888
* now we can throw it for the error message.
884889
*/
885-
if (divergence)
886-
throw DivergentImplicit
887-
888-
if (invalidImplicits.nonEmpty)
890+
if (DivergentImplicitRecovery.sym != null) {
891+
DivergingImplicitExpansionError(tree, pt, DivergentImplicitRecovery.sym)(context)
892+
} else if (invalidImplicits.nonEmpty)
889893
setAddendum(pos, () =>
890894
"\n Note: implicit "+invalidImplicits.head+" is not applicable here"+
891895
" because it comes after the application point and it lacks an explicit result type")
@@ -1438,6 +1442,3 @@ object ImplicitsStats {
14381442
val implicitCacheAccs = Statistics.newCounter ("implicit cache accesses", "typer")
14391443
val implicitCacheHits = Statistics.newSubCounter("implicit cache hits", implicitCacheAccs)
14401444
}
1441-
1442-
class DivergentImplicit extends Exception
1443-
object DivergentImplicit extends DivergentImplicit

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,11 @@ trait Typers extends Adaptations with Tags {
151151
mkArg = gen.mkNamedArg // don't pass the default argument (if any) here, but start emitting named arguments for the following args
152152
if (!param.hasDefault && !paramFailed) {
153153
context.reportBuffer.errors.find(_.kind == ErrorKinds.Divergent) match {
154-
case Some(divergentImplicit) =>
154+
case Some(divergent: DivergentImplicitTypeError) =>
155155
// DivergentImplicit error has higher priority than "no implicit found"
156156
// no need to issue the problem again if we are still in silent mode
157157
if (context.reportErrors) {
158-
context.issue(divergentImplicit)
158+
context.issue(divergent.withPt(paramTp))
159159
context.reportBuffer.clearErrors(ErrorKinds.Divergent)
160160
}
161161
case None =>
@@ -1627,22 +1627,27 @@ trait Typers extends Adaptations with Tags {
16271627

16281628
/** Makes sure that the first type tree in the list of parent types is always a class.
16291629
* If the first parent is a trait, prepend its supertype to the list until it's a class.
1630-
*/
1631-
private def normalizeFirstParent(parents: List[Tree]): List[Tree] = parents match {
1632-
case first :: rest if treeInfo.isTraitRef(first) =>
1633-
def explode(supertpt: Tree, acc: List[Tree]): List[Tree] = {
1634-
if (treeInfo.isTraitRef(supertpt)) {
1635-
val supertpt1 = typedType(supertpt)
1636-
if (!supertpt1.isErrorTyped) {
1637-
val supersupertpt = TypeTree(supertpt1.tpe.firstParent) setPos supertpt.pos.focus
1638-
return explode(supersupertpt, supertpt1 :: acc)
1639-
}
1640-
}
1641-
if (supertpt.tpe.typeSymbol == AnyClass) supertpt setType AnyRefClass.tpe
1642-
supertpt :: acc
1643-
}
1644-
explode(first, Nil) ++ rest
1645-
case _ => parents
1630+
*/
1631+
private def normalizeFirstParent(parents: List[Tree]): List[Tree] = {
1632+
@annotation.tailrec
1633+
def explode0(parents: List[Tree]): List[Tree] = {
1634+
val supertpt :: rest = parents // parents is always non-empty here - it only grows
1635+
if (supertpt.tpe.typeSymbol == AnyClass) {
1636+
supertpt setType AnyRefTpe
1637+
parents
1638+
} else if (treeInfo isTraitRef supertpt) {
1639+
val supertpt1 = typedType(supertpt)
1640+
def supersuper = TypeTree(supertpt1.tpe.firstParent) setPos supertpt.pos.focus
1641+
if (supertpt1.isErrorTyped) rest
1642+
else explode0(supersuper :: supertpt1 :: rest)
1643+
} else parents
1644+
}
1645+
1646+
def explode(parents: List[Tree]) =
1647+
if (treeInfo isTraitRef parents.head) explode0(parents)
1648+
else parents
1649+
1650+
if (parents.isEmpty) Nil else explode(parents)
16461651
}
16471652

16481653
/** Certain parents are added in the parser before it is known whether

src/compiler/scala/tools/reflect/ToolBoxFactory.scala

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,15 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
181181
(currentTyper, tree) => {
182182
trace("inferring implicit %s (macros = %s): ".format(if (isView) "view" else "value", !withMacrosDisabled))(showAttributed(pt, true, true, settings.Yshowsymkinds.value))
183183
val context = currentTyper.context
184-
analyzer.inferImplicit(tree, pt, reportAmbiguous = true, isView = isView, context = context, saveAmbiguousDivergent = !silent, pos = pos) match {
185-
case failure if failure.tree.isEmpty =>
186-
trace("implicit search has failed. to find out the reason, turn on -Xlog-implicits: ")(failure.tree)
187-
context.firstError foreach { err =>
188-
throw ToolBoxError("reflective implicit search has failed: %s".format(err.errMsg))
189-
}
190-
EmptyTree
191-
case success =>
192-
success.tree
184+
val result = analyzer.inferImplicit(tree, pt, reportAmbiguous = true, isView = isView, context = context, saveAmbiguousDivergent = !silent, pos = pos)
185+
if (result.isFailure) {
186+
// @H: what's the point of tracing an empty tree?
187+
trace("implicit search has failed. to find out the reason, turn on -Xlog-implicits: ")(result.tree)
188+
context.firstError foreach { err =>
189+
throw ToolBoxError("reflective implicit search has failed: %s".format(err.errMsg))
190+
}
193191
}
192+
result.tree
194193
})
195194

196195
def compile(expr0: Tree): () => Any = {

src/interactive/scala/tools/nsc/interactive/Global.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import scala.reflect.internal.util.{ SourceFile, BatchSourceFile, Position, NoPo
1515
import scala.tools.nsc.reporters._
1616
import scala.tools.nsc.symtab._
1717
import scala.tools.nsc.doc.ScaladocAnalyzer
18-
import scala.tools.nsc.typechecker.{ Analyzer, DivergentImplicit }
18+
import scala.tools.nsc.typechecker.Analyzer
1919
import symtab.Flags.{ACCESSOR, PARAMACCESSOR}
2020
import scala.annotation.{ elidable, tailrec }
2121
import scala.language.implicitConversions
@@ -1210,9 +1210,6 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
12101210
case ex: TypeError =>
12111211
debugLog("type error caught: "+ex)
12121212
alt
1213-
case ex: DivergentImplicit =>
1214-
debugLog("divergent implicit caught: "+ex)
1215-
alt
12161213
}
12171214
}
12181215

test/files/neg/t696.check

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
t696.scala:4: error: diverging implicit expansion for type TypeUtil0.Type[Any]
1+
t696.scala:5: error: diverging implicit expansion for type TypeUtil0.Type[Any]
22
starting with method WithType in object TypeUtil0
3-
as[Any](null);
3+
as[Any](null)
44
^
5-
one error found
5+
t696.scala:6: error: diverging implicit expansion for type TypeUtil0.Type[X]
6+
starting with method WithType in object TypeUtil0
7+
def foo[X]() = as[X](null)
8+
^
9+
two errors found

test/files/neg/t696.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
object TypeUtil0 {
2-
trait Type[+T];
2+
trait Type[+T]
33
implicit def WithType[S,T](implicit tpeS : Type[S], tpeT : Type[T]) : Type[S with T] = null
4-
as[Any](null);
5-
def as[T](x : Any)(implicit tpe : Type[T]) = null;
4+
def as[T](x : Any)(implicit tpe : Type[T]) = null
5+
as[Any](null)
6+
def foo[X]() = as[X](null)
67
}

test/files/run/t7291.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
conjure
2+
traversable

test/files/run/t7291.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
trait Fooable[T]
2+
object Fooable {
3+
implicit def conjure[T]: Fooable[T] = {
4+
println("conjure")
5+
new Fooable[T]{}
6+
}
7+
8+
}
9+
10+
object Test {
11+
implicit def traversable[T, Coll[_] <: Traversable[_]](implicit
12+
elem: Fooable[T]): Fooable[Coll[T]] = {
13+
println("traversable")
14+
new Fooable[Coll[T]]{}
15+
}
16+
def main(args: Array[String]) {
17+
implicitly[Fooable[List[Any]]]
18+
}
19+
}

0 commit comments

Comments
 (0)