Skip to content

Commit 2e41763

Browse files
oderskyodersky
authored
Fix implicit search failure reporting (#20261)
Fixes #19414.
2 parents 3920665 + ebf58bb commit 2e41763

15 files changed

+209
-46
lines changed

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2953,11 +2953,20 @@ class MissingImplicitArgument(
29532953

29542954
def location(preposition: String) = if (where.isEmpty) "" else s" $preposition $where"
29552955

2956+
/** Default error message for non-nested ambiguous implicits. */
29562957
def defaultAmbiguousImplicitMsg(ambi: AmbiguousImplicits) =
29572958
s"Ambiguous given instances: ${ambi.explanation}${location("of")}"
29582959

2960+
/** Default error messages for non-ambiguous implicits, or nested ambiguous
2961+
* implicits.
2962+
*
2963+
* The default message is shown for ambiguous implicits only if they have
2964+
* the `nested` flag set. In this case, we output "no best given instance"
2965+
* instead of "no given instance".
2966+
*/
29592967
def defaultImplicitNotFoundMessage =
2960-
i"No given instance of type $pt was found${location("for")}"
2968+
val bestStr = if arg.tpe.isInstanceOf[AmbiguousImplicits] then " best" else ""
2969+
i"No$bestStr given instance of type $pt was found${location("for")}"
29612970

29622971
/** Construct a custom error message given an ambiguous implicit
29632972
* candidate `alt` and a user defined message `raw`.
@@ -2995,7 +3004,7 @@ class MissingImplicitArgument(
29953004
* def foo(implicit foo: Foo): Any = ???
29963005
*/
29973006
arg.tpe match
2998-
case ambi: AmbiguousImplicits =>
3007+
case ambi: AmbiguousImplicits if !ambi.nested =>
29993008
(ambi.alt1, ambi.alt2) match
30003009
case (alt @ AmbiguousImplicitMsg(msg), _) =>
30013010
userDefinedAmbiguousImplicitMsg(alt, msg)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ object Implicits:
537537
end TooUnspecific
538538

539539
/** An ambiguous implicits failure */
540-
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree) extends SearchFailureType:
540+
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
541541

542542
def msg(using Context): Message =
543543
var str1 = err.refStr(alt1.ref)

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

Lines changed: 43 additions & 43 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,37 @@ 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
3874+
/** Reports errors for arguments of `appTree` that have a
3875+
* `SearchFailureType`.
3876+
*/
3877+
def issueErrors(fun: Tree, args: List[Tree]): Tree =
3878+
def firstFailure = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType)
3879+
val errorType =
3880+
firstFailure match
3881+
case tp: AmbiguousImplicits =>
3882+
AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true)
3883+
case tp =>
3884+
tp
3885+
val res = untpd.Apply(fun, args).withType(errorType)
39033886

39043887
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
3905-
arg.tpe match {
3888+
arg.tpe match
39063889
case failure: SearchFailureType =>
3890+
val methodStr = err.refStr(methPart(fun).tpe)
3891+
val paramStr = implicitParamString(paramName, methodStr, fun)
3892+
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
3893+
val paramSymWithMethodCallTree = paramSym.map((_, res))
39073894
report.error(
3908-
missingArgMsg(arg, formal, implicitParamString(paramName, methodStr, tree), paramSymWithMethodTree(paramName)),
3909-
tree.srcPos.endPos
3910-
)
3895+
missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree),
3896+
tree.srcPos.endPos
3897+
)
39113898
case _ =>
3912-
}
39133899
}
3914-
untpd.Apply(tree, args).withType(propFail)
3915-
}
39163900

3917-
if (propFail.exists) {
3901+
res
3902+
3903+
val args = implicitArgs(wtp.paramInfos, 0, pt)
3904+
if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
39183905
// If there are several arguments, some arguments might already
39193906
// have influenced the context, binding variables, but later ones
39203907
// might fail. In that case the constraint and instantiated variables
@@ -3923,18 +3910,31 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
39233910

39243911
// If method has default params, fall back to regular application
39253912
// 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-
}
3913+
if hasDefaultParams then
3914+
// Only keep the arguments that don't have an error type, or that
3915+
// have an `AmbiguousImplicits` error type. The later ensures that a
3916+
// default argument can't override an ambiguous implicit. See tests
3917+
// `given-ambiguous-default*` and `19414*`.
3918+
val namedArgs =
3919+
wtp.paramNames.lazyZip(args)
3920+
.filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits])
3921+
.map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg)))
3922+
39303923
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
39313924
val needsUsing = wtp.isContextualMethod || wtp.match
39323925
case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`)
39333926
case _ => false
39343927
if needsUsing then app.setApplyKind(ApplyKind.Using)
39353928
typr.println(i"try with default implicit args $app")
3936-
typed(app, pt, locked)
3937-
else issueErrors()
3929+
val retyped = typed(app, pt, locked)
3930+
3931+
// If the retyped tree still has an error type and is an `Apply`
3932+
// node, we can report the errors for each argument nicely.
3933+
// Otherwise, we don't report anything here.
3934+
retyped match
3935+
case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args)
3936+
case _ => retyped
3937+
else issueErrors(tree, args)
39383938
}
39393939
else tree match {
39403940
case tree: Block =>

‎tests/neg/19414-desugared.check

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------
2+
22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
3+
| ^
4+
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
5+
|I found:
6+
|
7+
| given_BodySerializer_B[B](
8+
| writer =
9+
| /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */
10+
| summon[Writer[B]]
11+
| ,
12+
| this.given_BodySerializer_B$default$2[B])
13+
|
14+
|But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- [E172] Type Error: tests/neg/19414.scala:15:34 ----------------------------------------------------------------------
2+
15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
3+
| ^
4+
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
5+
|I found:
6+
|
7+
| given_BodySerializer_B[B](
8+
| evidence$1 =
9+
| /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */
10+
| summon[Writer[B]]
11+
| ,
12+
| this.given_BodySerializer_B$default$2[B])
13+
|
14+
|But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[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-1.check

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
-- [E172] Type Error: tests/neg/given-ambiguous-1.scala:12:23 ----------------------------------------------------------
2+
12 |def f: Unit = summon[B] // error: Ambiguous given instances
3+
| ^
4+
| No best given instance of type B was found for parameter x of method summon in object Predef.
5+
| I found:
6+
|
7+
| given_B(/* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
8+
|
9+
| But both given instance a1 and given instance a2 match type A.

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class A
2+
class B
3+
given a1: A = ???
4+
given a2: A = ???
5+
given (using a: A): B = ???
6+
7+
// In this case, the ambiguous given instance is not directly the argument of
8+
// `summon`; it is the argument of `given_B` which is needed for the argument of
9+
// `summon`. This is a nested ambiguous implicit, thus we report an error in
10+
// the style "I found ... but". See `given-ambiguous-2` for a direct ambiguous
11+
// implicit error.
12+
def f: Unit = summon[B] // error: Ambiguous given instances

‎tests/neg/given-ambiguous-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-2.scala:10:15 ----------------------------------------------------------
2+
10 |def f: Unit = g // error: Ambiguous given instances
3+
| ^
4+
| Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of method g

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
class A
2+
class B
3+
given a1: A = ???
4+
given a2: A = ???
5+
def g(using a: A): B = ???
6+
7+
// In this case, the ambiguous given instance is directly the argument of
8+
// `summon`. This is a direct ambiguous implicit, thus we report the error
9+
// directly. See `given-ambiguous-1` for a nested ambiguous implicit error.
10+
def f: Unit = g // error: Ambiguous given instances

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
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+
| No best given instance of type B was found for parameter x of method summon in object Predef.
5+
| I found:
6+
|
7+
| given_B(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
8+
|
9+
| But both given instance a1 and given instance a2 match type A.

‎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: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
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+
|No best given instance of type C was found for parameter x of method summon in object Predef.
5+
|I found:
6+
|
7+
| given_C(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A], this.given_C$default$2)
8+
|
9+
|But both given instance a1 and given instance a2 match type A.

‎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

0 commit comments

Comments
 (0)