Skip to content

Commit 11fee57

Browse files
mbovelmbovel
committed
Fix implicit search failure reporting
1 parent 2ca0ce7 commit 11fee57

14 files changed

+150
-79
lines changed

‎compiler/src/dotty/tools/dotc/typer/Typer.scala‎

Lines changed: 53 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3707,7 +3707,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
37073707

37083708
private def adapt1(tree: Tree, pt: Type, locked: TypeVars)(using Context): Tree = {
37093709
assert(pt.exists && !pt.isInstanceOf[ExprType] || ctx.reporter.errorsReported, i"tree: $tree, pt: $pt")
3710-
def methodStr = err.refStr(methPart(tree).tpe)
37113710

37123711
def readapt(tree: Tree)(using Context) = adapt(tree, pt, locked)
37133712
def readaptSimplified(tree: Tree)(using Context) = readapt(simplify(tree, pt, locked))
@@ -3872,49 +3871,34 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38723871
arg :: inferArgsAfter(arg)
38733872
end implicitArgs
38743873

3875-
val args = implicitArgs(wtp.paramInfos, 0, pt)
3876-
3877-
def propagatedFailure(args: List[Tree]): Type = args match {
3878-
case arg :: args1 =>
3879-
arg.tpe match {
3880-
case ambi: AmbiguousImplicits =>
3881-
propagatedFailure(args1) match {
3882-
case NoType | (_: AmbiguousImplicits) => ambi
3883-
case failed => failed
3884-
}
3885-
case failed: SearchFailureType => failed
3886-
case _ => propagatedFailure(args1)
3887-
}
3888-
case Nil => NoType
3889-
}
3890-
3891-
val propFail = propagatedFailure(args)
3892-
3893-
def issueErrors(): Tree = {
3894-
def paramSymWithMethodTree(paramName: TermName) =
3895-
if tree.symbol.exists then
3896-
tree.symbol.paramSymss.flatten
3897-
.map(sym => sym.name -> sym)
3898-
.toMap
3899-
.get(paramName)
3900-
.map((_, tree))
3901-
else
3902-
None
3903-
3904-
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
3905-
arg.tpe match {
3874+
/** Reports errors for arguments of `appTree` that have a
3875+
* `SearchFailureType`, recursively traversing arguments that are
3876+
* themselves applications. `mt` must be the type of `appTree.fun`.
3877+
*/
3878+
def reportErrors(appTree: Apply, mt: MethodType): Unit =
3879+
val Apply(fun, args) = appTree
3880+
for (paramName, formal, arg) <- mt.paramNames.lazyZip(mt.paramInfos).lazyZip(args) do
3881+
arg.tpe match
39063882
case failure: SearchFailureType =>
3907-
report.error(
3908-
missingArgMsg(arg, formal, implicitParamString(paramName, methodStr, tree), paramSymWithMethodTree(paramName)),
3909-
tree.srcPos.endPos
3910-
)
3911-
case _ =>
3912-
}
3913-
}
3914-
untpd.Apply(tree, args).withType(propFail)
3915-
}
3883+
arg match
3884+
case childAppTree: Apply =>
3885+
childAppTree.fun.tpe.widen match
3886+
case childMt: MethodType => reportErrors(childAppTree, childMt)
3887+
case _ => ()
3888+
case _ => ()
3889+
3890+
val methodStr = err.refStr(methPart(fun).tpe)
3891+
val paramStr = implicitParamString(paramName, methodStr, fun)
3892+
val paramSymWithMethodCallTree =
3893+
fun.symbol.paramSymss.flatten
3894+
.find(_.name == paramName)
3895+
.map((_, appTree))
3896+
val message = missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree)
3897+
report.error(message, fun.srcPos.endPos)
3898+
case _ => ()
39163899

3917-
if (propFail.exists) {
3900+
val args = implicitArgs(wtp.paramInfos, 0, pt)
3901+
if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
39183902
// If there are several arguments, some arguments might already
39193903
// have influenced the context, binding variables, but later ones
39203904
// might fail. In that case the constraint and instantiated variables
@@ -3923,18 +3907,39 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
39233907

39243908
// If method has default params, fall back to regular application
39253909
// where all inferred implicits are passed as named args.
3926-
if hasDefaultParams && !propFail.isInstanceOf[AmbiguousImplicits] then
3927-
val namedArgs = wtp.paramNames.lazyZip(args).flatMap { (pname, arg) =>
3928-
if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil
3929-
}
3910+
if hasDefaultParams then
3911+
// Only keep the arguments that don't have an error type, or that
3912+
// have an `AmbiguousImplicits` error type. The later ensures that a
3913+
// default argument can't override an ambiguous implicit. See tests
3914+
// `given-ambiguous-default*` and `19414*`.
3915+
val namedArgs =
3916+
wtp.paramNames.lazyZip(args)
3917+
.filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits])
3918+
.map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg)))
3919+
39303920
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
39313921
val needsUsing = wtp.isContextualMethod || wtp.match
39323922
case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`)
39333923
case _ => false
39343924
if needsUsing then app.setApplyKind(ApplyKind.Using)
39353925
typr.println(i"try with default implicit args $app")
3936-
typed(app, pt, locked)
3937-
else issueErrors()
3926+
val retyped = typed(app, pt, locked)
3927+
3928+
// If the retyped tree still has an error type and is an `Apply`
3929+
// node, we can report the errors for each argument nicely.
3930+
// Otherwise, we don't report anything here.
3931+
retyped match
3932+
case retyped: Apply if retyped.tpe.isError => reportErrors(retyped, wtp)
3933+
case _ => ()
3934+
3935+
retyped
3936+
else
3937+
val firstNonAmbiguous = args.tpes.find(tp => tp.isError && !tp.isInstanceOf[AmbiguousImplicits])
3938+
def firstError = args.tpes.find(_.isError)
3939+
val errorType = firstNonAmbiguous.orElse(firstError).getOrElse(NoType)
3940+
val res = untpd.Apply(tree, args).withType(errorType)
3941+
reportErrors(res, wtp)
3942+
res
39383943
}
39393944
else tree match {
39403945
case tree: Block =>

‎tests/neg/19414-desugared.check‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------
2+
22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
3+
| ^
4+
|Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of parameter writer of given instance given_BodySerializer_B

‎tests/neg/19414-desugared.scala‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
trait JsValue
2+
trait JsObject extends JsValue
3+
4+
trait Writer[T]
5+
trait BodySerializer[-B]
6+
7+
class Printer
8+
9+
given Writer[JsValue] = ???
10+
given Writer[JsObject] = ???
11+
12+
// This is not an exact desugaring of the original code: currently the compiler
13+
// actually changes the modifier of the parameter list from `using` to
14+
// `implicit` when desugaring the context-bound `B: Writer` to `implicit writer:
15+
// Writer[B]`, but we can't write it in user code as this is not valid syntax.
16+
given [B](using
17+
writer: Writer[B],
18+
printer: Printer = new Printer
19+
): BodySerializer[B] = ???
20+
21+
def f: Unit =
22+
summon[BodySerializer[JsObject]] // error: Ambiguous given instances

‎tests/neg/19414.check‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- [E172] Type Error: tests/neg/19414.scala:15:34 ----------------------------------------------------------------------
2+
15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
3+
| ^
4+
|Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of a context parameter of given instance given_BodySerializer_B

‎tests/neg/19414.scala‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
trait JsValue
2+
trait JsObject extends JsValue
3+
4+
trait Writer[T]
5+
trait BodySerializer[-B]
6+
7+
class Printer
8+
9+
given Writer[JsValue] = ???
10+
given Writer[JsObject] = ???
11+
12+
given [B: Writer](using printer: Printer = new Printer): BodySerializer[B] = ???
13+
14+
def f: Unit =
15+
summon[BodySerializer[JsObject]] // error: Ambiguous given instances

‎tests/neg/given-ambiguous-default-1.check‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- [E172] Type Error: tests/neg/given-ambiguous-default-1.scala:18:23 --------------------------------------------------
2+
18 |def f: Unit = summon[B] // error: Ambiguous given instances
3+
| ^
4+
|Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of given instance given_B

‎tests/neg/given-ambiguous-default-1.scala‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/** This test checks that provided ambiguous given instances take precedence
2+
* over default given arguments. In the following code, the compiler must
3+
* report an "Ambiguous implicits" error for the parameter `a`, and must not
4+
* use its default value.
5+
*
6+
* See also:
7+
* - tests/neg/19414.scala
8+
* - tests/neg/19414-desugared.scala
9+
* - tests/neg/given-ambiguous-default-2.scala
10+
*/
11+
12+
class A
13+
class B
14+
given a1: A = ???
15+
given a2: A = ???
16+
given (using a: A = A()): B = ???
17+
18+
def f: Unit = summon[B] // error: Ambiguous given instances

‎tests/neg/given-ambiguous-default-2.check‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- [E172] Type Error: tests/neg/given-ambiguous-default-2.scala:18:23 --------------------------------------------------
2+
18 |def f: Unit = summon[C] // error: Ambiguous given instances
3+
| ^
4+
|Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of given instance given_C

‎tests/neg/given-ambiguous-default-2.scala‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/** This test checks that provided given instances take precedence over default
2+
* given arguments, even when there are multiple default arguments. Before the
3+
* fix for issue #19414, this code would compile without errors.
4+
*
5+
* See also:
6+
* - tests/neg/given-ambiguous-default-1.scala
7+
* - tests/neg/19414.scala
8+
* - tests/neg/19414-desugared.scala
9+
*/
10+
11+
class A
12+
class B
13+
class C
14+
given a1: A = ???
15+
given a2: A = ???
16+
given (using a: A = A(), b: B = B()): C = ???
17+
18+
def f: Unit = summon[C] // error: Ambiguous given instances

‎tests/neg/i8827a.check‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
-- [E172] Type Error: tests/neg/i8827a.scala:16:26 ---------------------------------------------------------------------
22
16 | summon[Order[List[Foo]]] // error
33
| ^
4-
| No given instance of type pkg.Order[List[pkg.Foo]] was found for parameter x of method summon in object Predef.
5-
| I found:
6-
|
7-
| pkg.Order.orderList[pkg.Foo](/* missing */summon[pkg.Order[pkg.Foo]])
8-
|
9-
| But no implicit values were found that match type pkg.Order[pkg.Foo].
4+
| No given instance of type pkg.Order[pkg.Foo] was found for parameter orderA of method orderList in object Order
105
|
116
| The following import might fix the problem:
127
|

0 commit comments

Comments
 (0)