diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 5fc5b4ae66b0..2b4deef72362 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2938,11 +2938,20 @@ class MissingImplicitArgument( def location(preposition: String) = if (where.isEmpty) "" else s" $preposition $where" + /** Default error message for non-nested ambiguous implicits. */ def defaultAmbiguousImplicitMsg(ambi: AmbiguousImplicits) = s"Ambiguous given instances: ${ambi.explanation}${location("of")}" + /** Default error messages for non-ambiguous implicits, or nested ambiguous + * implicits. + * + * The default message is shown for ambiguous implicits only if they have + * the `nested` flag set. In this case, we output "no best given instance" + * instead of "no given instance". + */ def defaultImplicitNotFoundMessage = - i"No given instance of type $pt was found${location("for")}" + val bestStr = if arg.tpe.isInstanceOf[AmbiguousImplicits] then " best" else "" + i"No$bestStr given instance of type $pt was found${location("for")}" /** Construct a custom error message given an ambiguous implicit * candidate `alt` and a user defined message `raw`. @@ -2980,7 +2989,7 @@ class MissingImplicitArgument( * def foo(implicit foo: Foo): Any = ??? */ arg.tpe match - case ambi: AmbiguousImplicits => + case ambi: AmbiguousImplicits if !ambi.nested => (ambi.alt1, ambi.alt2) match case (alt @ AmbiguousImplicitMsg(msg), _) => userDefinedAmbiguousImplicitMsg(alt, msg) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ffd9d7fd8515..bb2fd22b1c93 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -537,7 +537,7 @@ object Implicits: end TooUnspecific /** An ambiguous implicits failure */ - class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree) extends SearchFailureType: + class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType: def msg(using Context): Message = var str1 = err.refStr(alt1.ref) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9150ad6be392..82b722850260 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3707,7 +3707,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer private def adapt1(tree: Tree, pt: Type, locked: TypeVars)(using Context): Tree = { assert(pt.exists && !pt.isInstanceOf[ExprType] || ctx.reporter.errorsReported, i"tree: $tree, pt: $pt") - def methodStr = err.refStr(methPart(tree).tpe) def readapt(tree: Tree)(using Context) = adapt(tree, pt, locked) def readaptSimplified(tree: Tree)(using Context) = readapt(simplify(tree, pt, locked)) @@ -3872,49 +3871,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer arg :: inferArgsAfter(arg) end implicitArgs - val args = implicitArgs(wtp.paramInfos, 0, pt) - - def propagatedFailure(args: List[Tree]): Type = args match { - case arg :: args1 => - arg.tpe match { - case ambi: AmbiguousImplicits => - propagatedFailure(args1) match { - case NoType | (_: AmbiguousImplicits) => ambi - case failed => failed - } - case failed: SearchFailureType => failed - case _ => propagatedFailure(args1) - } - case Nil => NoType - } - - val propFail = propagatedFailure(args) - - def issueErrors(): Tree = { - def paramSymWithMethodTree(paramName: TermName) = - if tree.symbol.exists then - tree.symbol.paramSymss.flatten - .map(sym => sym.name -> sym) - .toMap - .get(paramName) - .map((_, tree)) - else - None + /** Reports errors for arguments of `appTree` that have a + * `SearchFailureType`. + */ + def issueErrors(fun: Tree, args: List[Tree]): Tree = + def firstFailure = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType) + val errorType = + firstFailure match + case tp: AmbiguousImplicits => + AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true) + case tp => + tp + val res = untpd.Apply(fun, args).withType(errorType) wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) => - arg.tpe match { + arg.tpe match case failure: SearchFailureType => + val methodStr = err.refStr(methPart(fun).tpe) + val paramStr = implicitParamString(paramName, methodStr, fun) + val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName) + val paramSymWithMethodCallTree = paramSym.map((_, res)) report.error( - missingArgMsg(arg, formal, implicitParamString(paramName, methodStr, tree), paramSymWithMethodTree(paramName)), - tree.srcPos.endPos - ) + missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree), + tree.srcPos.endPos + ) case _ => - } } - untpd.Apply(tree, args).withType(propFail) - } - if (propFail.exists) { + res + + val args = implicitArgs(wtp.paramInfos, 0, pt) + if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) { // If there are several arguments, some arguments might already // have influenced the context, binding variables, but later ones // might fail. In that case the constraint and instantiated variables @@ -3923,18 +3910,31 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // If method has default params, fall back to regular application // where all inferred implicits are passed as named args. - if hasDefaultParams && !propFail.isInstanceOf[AmbiguousImplicits] then - val namedArgs = wtp.paramNames.lazyZip(args).flatMap { (pname, arg) => - if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil - } + if hasDefaultParams then + // Only keep the arguments that don't have an error type, or that + // have an `AmbiguousImplicits` error type. The later ensures that a + // default argument can't override an ambiguous implicit. See tests + // `given-ambiguous-default*` and `19414*`. + val namedArgs = + wtp.paramNames.lazyZip(args) + .filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits]) + .map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg))) + val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs) val needsUsing = wtp.isContextualMethod || wtp.match case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`) case _ => false if needsUsing then app.setApplyKind(ApplyKind.Using) typr.println(i"try with default implicit args $app") - typed(app, pt, locked) - else issueErrors() + val retyped = typed(app, pt, locked) + + // If the retyped tree still has an error type and is an `Apply` + // node, we can report the errors for each argument nicely. + // Otherwise, we don't report anything here. + retyped match + case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args) + case _ => retyped + else issueErrors(tree, args) } else tree match { case tree: Block => diff --git a/tests/neg/19414-desugared.check b/tests/neg/19414-desugared.check new file mode 100644 index 000000000000..c21806e16c2c --- /dev/null +++ b/tests/neg/19414-desugared.check @@ -0,0 +1,14 @@ +-- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------ +22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances + | ^ + |No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef. + |I found: + | + | given_BodySerializer_B[B]( + | writer = + | /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */ + | summon[Writer[B]] + | , + | this.given_BodySerializer_B$default$2[B]) + | + |But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B]. diff --git a/tests/neg/19414-desugared.scala b/tests/neg/19414-desugared.scala new file mode 100644 index 000000000000..9fc16e2249a2 --- /dev/null +++ b/tests/neg/19414-desugared.scala @@ -0,0 +1,22 @@ +trait JsValue +trait JsObject extends JsValue + +trait Writer[T] +trait BodySerializer[-B] + +class Printer + +given Writer[JsValue] = ??? +given Writer[JsObject] = ??? + +// This is not an exact desugaring of the original code: currently the compiler +// actually changes the modifier of the parameter list from `using` to +// `implicit` when desugaring the context-bound `B: Writer` to `implicit writer: +// Writer[B]`, but we can't write it in user code as this is not valid syntax. +given [B](using + writer: Writer[B], + printer: Printer = new Printer +): BodySerializer[B] = ??? + +def f: Unit = + summon[BodySerializer[JsObject]] // error: Ambiguous given instances diff --git a/tests/neg/19414.check b/tests/neg/19414.check new file mode 100644 index 000000000000..6804546df037 --- /dev/null +++ b/tests/neg/19414.check @@ -0,0 +1,14 @@ +-- [E172] Type Error: tests/neg/19414.scala:15:34 ---------------------------------------------------------------------- +15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances + | ^ + |No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef. + |I found: + | + | given_BodySerializer_B[B]( + | evidence$1 = + | /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */ + | summon[Writer[B]] + | , + | this.given_BodySerializer_B$default$2[B]) + | + |But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B]. diff --git a/tests/neg/19414.scala b/tests/neg/19414.scala new file mode 100644 index 000000000000..bb275ad943b7 --- /dev/null +++ b/tests/neg/19414.scala @@ -0,0 +1,15 @@ +trait JsValue +trait JsObject extends JsValue + +trait Writer[T] +trait BodySerializer[-B] + +class Printer + +given Writer[JsValue] = ??? +given Writer[JsObject] = ??? + +given [B: Writer](using printer: Printer = new Printer): BodySerializer[B] = ??? + +def f: Unit = + summon[BodySerializer[JsObject]] // error: Ambiguous given instances diff --git a/tests/neg/given-ambiguous-1.check b/tests/neg/given-ambiguous-1.check new file mode 100644 index 000000000000..ed64164b351f --- /dev/null +++ b/tests/neg/given-ambiguous-1.check @@ -0,0 +1,9 @@ +-- [E172] Type Error: tests/neg/given-ambiguous-1.scala:12:23 ---------------------------------------------------------- +12 |def f: Unit = summon[B] // error: Ambiguous given instances + | ^ + | No best given instance of type B was found for parameter x of method summon in object Predef. + | I found: + | + | given_B(/* ambiguous: both given instance a1 and given instance a2 match type A */summon[A]) + | + | But both given instance a1 and given instance a2 match type A. diff --git a/tests/neg/given-ambiguous-1.scala b/tests/neg/given-ambiguous-1.scala new file mode 100644 index 000000000000..0ce4f566e615 --- /dev/null +++ b/tests/neg/given-ambiguous-1.scala @@ -0,0 +1,12 @@ +class A +class B +given a1: A = ??? +given a2: A = ??? +given (using a: A): B = ??? + +// In this case, the ambiguous given instance is not directly the argument of +// `summon`; it is the argument of `given_B` which is needed for the argument of +// `summon`. This is a nested ambiguous implicit, thus we report an error in +// the style "I found ... but". See `given-ambiguous-2` for a direct ambiguous +// implicit error. +def f: Unit = summon[B] // error: Ambiguous given instances diff --git a/tests/neg/given-ambiguous-2.check b/tests/neg/given-ambiguous-2.check new file mode 100644 index 000000000000..ec84b750e691 --- /dev/null +++ b/tests/neg/given-ambiguous-2.check @@ -0,0 +1,4 @@ +-- [E172] Type Error: tests/neg/given-ambiguous-2.scala:10:15 ---------------------------------------------------------- +10 |def f: Unit = g // error: Ambiguous given instances + | ^ + | Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of method g diff --git a/tests/neg/given-ambiguous-2.scala b/tests/neg/given-ambiguous-2.scala new file mode 100644 index 000000000000..2c3c52f1ccb0 --- /dev/null +++ b/tests/neg/given-ambiguous-2.scala @@ -0,0 +1,10 @@ +class A +class B +given a1: A = ??? +given a2: A = ??? +def g(using a: A): B = ??? + +// In this case, the ambiguous given instance is directly the argument of +// `summon`. This is a direct ambiguous implicit, thus we report the error +// directly. See `given-ambiguous-1` for a nested ambiguous implicit error. +def f: Unit = g // error: Ambiguous given instances diff --git a/tests/neg/given-ambiguous-default-1.check b/tests/neg/given-ambiguous-default-1.check new file mode 100644 index 000000000000..1a5006c23055 --- /dev/null +++ b/tests/neg/given-ambiguous-default-1.check @@ -0,0 +1,9 @@ +-- [E172] Type Error: tests/neg/given-ambiguous-default-1.scala:18:23 -------------------------------------------------- +18 |def f: Unit = summon[B] // error: Ambiguous given instances + | ^ + | No best given instance of type B was found for parameter x of method summon in object Predef. + | I found: + | + | given_B(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A]) + | + | But both given instance a1 and given instance a2 match type A. diff --git a/tests/neg/given-ambiguous-default-1.scala b/tests/neg/given-ambiguous-default-1.scala new file mode 100644 index 000000000000..140736e9eee3 --- /dev/null +++ b/tests/neg/given-ambiguous-default-1.scala @@ -0,0 +1,18 @@ +/** This test checks that provided ambiguous given instances take precedence + * over default given arguments. In the following code, the compiler must + * report an "Ambiguous implicits" error for the parameter `a`, and must not + * use its default value. + * + * See also: + * - tests/neg/19414.scala + * - tests/neg/19414-desugared.scala + * - tests/neg/given-ambiguous-default-2.scala + */ + +class A +class B +given a1: A = ??? +given a2: A = ??? +given (using a: A = A()): B = ??? + +def f: Unit = summon[B] // error: Ambiguous given instances diff --git a/tests/neg/given-ambiguous-default-2.check b/tests/neg/given-ambiguous-default-2.check new file mode 100644 index 000000000000..cbe8b972a389 --- /dev/null +++ b/tests/neg/given-ambiguous-default-2.check @@ -0,0 +1,9 @@ +-- [E172] Type Error: tests/neg/given-ambiguous-default-2.scala:18:23 -------------------------------------------------- +18 |def f: Unit = summon[C] // error: Ambiguous given instances + | ^ + |No best given instance of type C was found for parameter x of method summon in object Predef. + |I found: + | + | given_C(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A], this.given_C$default$2) + | + |But both given instance a1 and given instance a2 match type A. diff --git a/tests/neg/given-ambiguous-default-2.scala b/tests/neg/given-ambiguous-default-2.scala new file mode 100644 index 000000000000..9e639b66f3d1 --- /dev/null +++ b/tests/neg/given-ambiguous-default-2.scala @@ -0,0 +1,18 @@ +/** This test checks that provided given instances take precedence over default + * given arguments, even when there are multiple default arguments. Before the + * fix for issue #19414, this code would compile without errors. + * + * See also: + * - tests/neg/given-ambiguous-default-1.scala + * - tests/neg/19414.scala + * - tests/neg/19414-desugared.scala + */ + +class A +class B +class C +given a1: A = ??? +given a2: A = ??? +given (using a: A = A(), b: B = B()): C = ??? + +def f: Unit = summon[C] // error: Ambiguous given instances