Skip to content

Commit 5f7c92c

Browse files
WojciechMazurWojciechMazur
authored
Backport "Fix implicit search failure reporting" to LTS (#21089)
Backports #20261 to the LTS branch. PR submitted by the release tooling. [skip ci]
2 parents eac4a88 + a04403c commit 5f7c92c

15 files changed

+210
-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
@@ -2913,11 +2913,20 @@ class MissingImplicitArgument(
29132913

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

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

2920+
/** Default error messages for non-ambiguous implicits, or nested ambiguous
2921+
* implicits.
2922+
*
2923+
* The default message is shown for ambiguous implicits only if they have
2924+
* the `nested` flag set. In this case, we output "no best given instance"
2925+
* instead of "no given instance".
2926+
*/
29192927
def defaultImplicitNotFoundMessage =
2920-
i"No given instance of type $pt was found${location("for")}"
2928+
val bestStr = if arg.tpe.isInstanceOf[AmbiguousImplicits] then " best" else ""
2929+
i"No$bestStr given instance of type $pt was found${location("for")}"
29212930

29222931
/** Construct a custom error message given an ambiguous implicit
29232932
* candidate `alt` and a user defined message `raw`.
@@ -2955,7 +2964,7 @@ class MissingImplicitArgument(
29552964
* def foo(implicit foo: Foo): Any = ???
29562965
*/
29572966
arg.tpe match
2958-
case ambi: AmbiguousImplicits =>
2967+
case ambi: AmbiguousImplicits if !ambi.nested =>
29592968
(ambi.alt1, ambi.alt2) match
29602969
case (alt @ AmbiguousImplicitMsg(msg), _) =>
29612970
userDefinedAmbiguousImplicitMsg(alt, msg)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,8 @@ object Implicits:
514514
override def toString = s"TooUnspecific"
515515

516516
/** An ambiguous implicits failure */
517-
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree) extends SearchFailureType {
517+
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType {
518+
518519
def msg(using Context): Message =
519520
var str1 = err.refStr(alt1.ref)
520521
var str2 = err.refStr(alt2.ref)

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

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

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

36313630
def readapt(tree: Tree)(using Context) = adapt(tree, pt, locked)
36323631
def readaptSimplified(tree: Tree)(using Context) = readapt(simplify(tree, pt, locked))
@@ -3803,49 +3802,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38033802
arg :: inferArgsAfter(arg)
38043803
end implicitArgs
38053804

3806-
val args = implicitArgs(wtp.paramInfos, 0, pt)
3807-
3808-
def propagatedFailure(args: List[Tree]): Type = args match {
3809-
case arg :: args1 =>
3810-
arg.tpe match {
3811-
case ambi: AmbiguousImplicits =>
3812-
propagatedFailure(args1) match {
3813-
case NoType | (_: AmbiguousImplicits) => ambi
3814-
case failed => failed
3815-
}
3816-
case failed: SearchFailureType => failed
3817-
case _ => propagatedFailure(args1)
3818-
}
3819-
case Nil => NoType
3820-
}
3821-
3822-
val propFail = propagatedFailure(args)
3823-
3824-
def issueErrors(): Tree = {
3825-
def paramSymWithMethodTree(paramName: TermName) =
3826-
if tree.symbol.exists then
3827-
tree.symbol.paramSymss.flatten
3828-
.map(sym => sym.name -> sym)
3829-
.toMap
3830-
.get(paramName)
3831-
.map((_, tree))
3832-
else
3833-
None
3805+
/** Reports errors for arguments of `appTree` that have a
3806+
* `SearchFailureType`.
3807+
*/
3808+
def issueErrors(fun: Tree, args: List[Tree]): Tree =
3809+
def firstFailure = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType)
3810+
val errorType =
3811+
firstFailure match
3812+
case tp: AmbiguousImplicits =>
3813+
AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true)
3814+
case tp =>
3815+
tp
3816+
val res = untpd.Apply(fun, args).withType(errorType)
38343817

38353818
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
3836-
arg.tpe match {
3819+
arg.tpe match
38373820
case failure: SearchFailureType =>
3821+
val methodStr = err.refStr(methPart(fun).tpe)
3822+
val paramStr = implicitParamString(paramName, methodStr, fun)
3823+
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
3824+
val paramSymWithMethodCallTree = paramSym.map((_, res))
38383825
report.error(
3839-
missingArgMsg(arg, formal, implicitParamString(paramName, methodStr, tree), paramSymWithMethodTree(paramName)),
3840-
tree.srcPos.endPos
3841-
)
3826+
missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree),
3827+
tree.srcPos.endPos
3828+
)
38423829
case _ =>
3843-
}
38443830
}
3845-
untpd.Apply(tree, args).withType(propFail)
3846-
}
38473831

3848-
if (propFail.exists) {
3832+
res
3833+
3834+
val args = implicitArgs(wtp.paramInfos, 0, pt)
3835+
if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
38493836
// If there are several arguments, some arguments might already
38503837
// have influenced the context, binding variables, but later ones
38513838
// might fail. In that case the constraint and instantiated variables
@@ -3854,15 +3841,28 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38543841

38553842
// If method has default params, fall back to regular application
38563843
// where all inferred implicits are passed as named args.
3857-
if hasDefaultParams && !propFail.isInstanceOf[AmbiguousImplicits] then
3858-
val namedArgs = wtp.paramNames.lazyZip(args).flatMap { (pname, arg) =>
3859-
if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil
3860-
}
3844+
if hasDefaultParams then
3845+
// Only keep the arguments that don't have an error type, or that
3846+
// have an `AmbiguousImplicits` error type. The later ensures that a
3847+
// default argument can't override an ambiguous implicit. See tests
3848+
// `given-ambiguous-default*` and `19414*`.
3849+
val namedArgs =
3850+
wtp.paramNames.lazyZip(args)
3851+
.filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits])
3852+
.map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg)))
3853+
38613854
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
38623855
if (wtp.isContextualMethod) app.setApplyKind(ApplyKind.Using)
38633856
typr.println(i"try with default implicit args $app")
3864-
typed(app, pt, locked)
3865-
else issueErrors()
3857+
val retyped = typed(app, pt, locked)
3858+
3859+
// If the retyped tree still has an error type and is an `Apply`
3860+
// node, we can report the errors for each argument nicely.
3861+
// Otherwise, we don't report anything here.
3862+
retyped match
3863+
case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args)
3864+
case _ => retyped
3865+
else issueErrors(tree, args)
38663866
}
38673867
else tree match {
38683868
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)