Don't try default args when there are ambiguous implicits

Before this commit, the logic in `class Typer` / `def adapt1` / `def addImplicitArgs` was to try default arguments if *any* argument had an implicit search error that was not an ambiguous implicits error.

This commit changes the logic to try default arguments only if *all* arguments' implicit search errors are not ambiguous implicits error.

The rational is that if there is any ambiguous implicits error, it's not worth trying default arguments because this cannot solve the ambiguity. Instead, we report the first ambiguous implicits error directly.

Author: mbovel

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

@@ -3849,22 +3849,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         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)
+        val firstAmbiguous = args.tpes.find(_.isInstanceOf[AmbiguousImplicits])
+        def firstError = args.tpes.find(_.isError)
+        val propFail = firstAmbiguous.orElse(firstError).getOrElse(NoType)
 
         def issueErrors(): Tree = {
           def paramSymWithMethodTree(paramName: TermName) =
@@ -3897,9 +3884,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           // need to be reset.
           ctx.typerState.resetTo(saved)
 
-          // 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 method has default params and there are no "Ambiguous implicits"
+          // error, fall back to regular application where all inferred
+          // implicits are passed as named args.
+          // If there are "Ambiguous implicits" errors, these take precedence
+          // over the default params (see issue #19414 and related tests).
+          if hasDefaultParams && firstAmbiguous.isEmpty then
             val namedArgs = wtp.paramNames.lazyZip(args).flatMap { (pname, arg) =>
               if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil
             }

tests/neg/19414-desugared.check

@@ -0,0 +1,4 @@
+-- [E172] Type Error: tests/neg/19414-desugared.scala:34:34 ------------------------------------------------------------
+34 |  summon[BodySerializer[JsObject]] // error: Ambiguous given instances
+   |                                  ^
+   |Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of parameter x of method summon in object Predef

tests/neg/19414-desugared.scala

@@ -0,0 +1,34 @@
+/** 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 fail with a "No given
+  * instance of type BodySerializer[JsObject]".
+  *
+  * See also:
+  *   - tests/neg/19414.scala
+  *   - tests/neg/given-ambiguous-default-1.scala
+  *   - tests/neg/given-ambiguous-default-2.scala
+  */
+
+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 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

tests/neg/19414.check

@@ -0,0 +1,4 @@
+-- [E172] Type Error: tests/neg/19414.scala:27:34 ----------------------------------------------------------------------
+27 |  summon[BodySerializer[JsObject]] // error: Ambiguous given instances
+   |                                  ^
+   |Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of parameter x of method summon in object Predef

tests/neg/19414.scala

@@ -0,0 +1,27 @@
+/** 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 fail with a "No given
+  * instance of type BodySerializer[JsObject]".
+  *
+  * See also:
+  *   - tests/neg/19414-desugared.scala
+  *   - tests/neg/given-ambiguous-default-1.scala
+  *   - tests/neg/given-ambiguous-default-2.scala
+  */
+
+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

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

@@ -0,0 +1,4 @@
+-- [E172] Type Error: tests/neg/given-ambiguous-default-1.scala:19:23 --------------------------------------------------
+19 |def f: Unit = summon[B] // error: Ambiguous given instances
+   |                       ^
+   |Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter x of method summon in object Predef

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

@@ -0,0 +1,19 @@
+/** This test checks that provided 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

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

@@ -0,0 +1,4 @@
+-- [E172] Type Error: tests/neg/given-ambiguous-default-2.scala:19:23 --------------------------------------------------
+19 |def f: Unit = summon[C] // error: Ambiguous given instances
+   |                       ^
+   |Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter x of method summon in object Predef

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

@@ -0,0 +1,19 @@
+/** 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