Wrong error message for ambiguous given instances

[KuceraMartin]

Compiler version

3.3.1, 3.4.0-RC1-bin-20240109-91db06a-NIGHTLY

Minimized code

trait JsValue
trait JsObject extends JsValue

trait JsonWriter[T]
trait BodySerializer[-B]

class JsonPrinter

given JsonWriter[JsValue] = ???
given JsonWriter[JsObject] = ???

given[B : JsonWriter](using printer: JsonPrinter = new JsonPrinter): BodySerializer[B] = ???

def f: Unit =
  summon[BodySerializer[JsObject]]

Output

[error] No given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
[error] I found:
[error]
[error]     given_BodySerializer_B[B]()
[error]
[error] But given instance given_BodySerializer_B does not match type BodySerializer[JsObject].
[error]   summon[BodySerializer[JsObject]]
[error]                                   ^

Expectation

[error] Ambiguous given instances: both given instance given_JsonWriter_JsValue and given instance given_JsonWriter_JsObject match type JsonWriter[B] of parameter x of method summon in object Predef
[error]   summon[BodySerializer[JsObject]]
[error]                                   ^

note: the expectation is what I got when I removed the implicit JsonPrinter parameter from the given BodySerializer:

given[B : JsonWriter]: BodySerializer[B] = ???

[joroKr21]

Perhaps a related issue - when the instance below I found is a macro, it might have a custom error message.
It would be nice to show the custom error message instead of a type mismatch.

[mbovel]

If I add given JsonPrinter = new JsonPrinter, I also get the expected error message.

[scala-center-bot]

This issue was picked for the Issue Spree No. 42 of February 6th, 2024. @mbovel, @KuceraMartin, @nmcb will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[mbovel]

Note: it works if the default parameter is in a separate list:

given[B](using writer: JsonWriter[B])(using printer: JsonPrinter = new JsonPrinter): BodySerializer[B] = ???

[Jezisek-Ematiq]

The contextual bound desugars to an implicit rather than using which seems to cause a problem in Applications.scala:641 (it doesn't fall into the else if branch because it's not a contextual function)

[mbovel]

We tried:

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 7727c125d1..9d35f80f66 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -3899,7 +3899,7 @@ 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
+          if hasDefaultParams && !args.exists(_.tpe.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

which yields the following error message:

-- [E172] Type Error: tests/init/pos/19414.scala:14:46 -------------------------
14 |def f: Unit = summon[BodySerializer[JsObject]]
   |                                              ^
   |No given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
   |I found:
   |
   |    given_BodySerializer_B[B](
   |      /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */
   |        summon[Writer[B]],
   |    /* missing */summon[Printer])
   |
   |But no implicit values were found that match type Printer.
1 error found

Note: it says summon[Printer] is missing, which is wrong.

---

I also tried:

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 7727c125d1..32c9474891 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -3899,9 +3899,9 @@ 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
+          if hasDefaultParams then
             val namedArgs = wtp.paramNames.lazyZip(args).flatMap { (pname, arg) =>
-              if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil
+              if (arg.tpe.isError && !arg.tpe.isInstanceOf[AmbiguousImplicits]) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) 
:: Nil
             }

which yields the following error message:

-- [E172] Type Error: tests/init/pos/19414.scala:14:46 -------------------------
14 |def f: Unit = summon[BodySerializer[JsObject]]
   |                                              ^
   |No given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
   |I found:
   |
   |    given_BodySerializer_B[B](using
   |      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 given instance given_BodySerializer_B does not match type BodySerializer[JsObject].
   |
   |Note: given instance given_BodySerializer_B was not considered because it was not imported with `import given`.
1 error found

Note: the note is wrong.

[mbovel]

I don't understand why propagatedFailure has its current semantic:

https://github.com/lampepfl/dotty/blob/641ab7a4984cd76ff77fadaf822472a0a930e4a2/compiler/src/dotty/tools/dotc/typer/Typer.scala#L3853-L3867

It returns the first non-ambiguous error if there is one, or else the first error, or else NoType.

Written otherwise:

val firstNonAmbiguous = args.tpes.find(tp => tp.isError && !tp.isInstanceOf[AmbiguousImplicits])
def firstError = args.tpes.find(_.isError)
val propFail = firstNonAmbiguous.orElse(firstError).getOrElse(NoType)

Why isn't it the opposite; why doesn't it return the first ambiguous error if there is one?

val firstAmbiguous = args.tpes.find(_.isInstanceOf[AmbiguousImplicits])
def firstError = args.tpes.find(_.isError)
val propFail = firstAmbiguous.orElse(firstError).getOrElse(NoType)

Can adding default arguments avoid an ambiguous error? That would explain it. If not, we could probably report it directly.

[mbovel]

Something else: even when the expected message is shown Ambiguous given instances: ..., the parameters name is wrong: it should be evidence$1 of given_BodySerializer_B, not x of summon.