From 894798e473d02d3e706b12ea5c74bc513d33867a Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Wed, 7 Feb 2024 14:10:13 +0100 Subject: [PATCH] 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. --- .../src/dotty/tools/dotc/typer/Typer.scala | 28 +++++---------- tests/neg/19414-desugared.check | 4 +++ tests/neg/19414-desugared.scala | 34 +++++++++++++++++++ tests/neg/19414.check | 4 +++ tests/neg/19414.scala | 27 +++++++++++++++ tests/neg/given-ambiguous-default-1.check | 4 +++ tests/neg/given-ambiguous-default-1.scala | 19 +++++++++++ tests/neg/given-ambiguous-default-2.check | 4 +++ tests/neg/given-ambiguous-default-2.scala | 19 +++++++++++ 9 files changed, 124 insertions(+), 19 deletions(-) create mode 100644 tests/neg/19414-desugared.check create mode 100644 tests/neg/19414-desugared.scala create mode 100644 tests/neg/19414.check create mode 100644 tests/neg/19414.scala create mode 100644 tests/neg/given-ambiguous-default-1.check create mode 100644 tests/neg/given-ambiguous-default-1.scala create mode 100644 tests/neg/given-ambiguous-default-2.check create mode 100644 tests/neg/given-ambiguous-default-2.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 7727c125d1e4..2daced94a181 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/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 } diff --git a/tests/neg/19414-desugared.check b/tests/neg/19414-desugared.check new file mode 100644 index 000000000000..4f777e91cae9 --- /dev/null +++ b/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 diff --git a/tests/neg/19414-desugared.scala b/tests/neg/19414-desugared.scala new file mode 100644 index 000000000000..c4803b9509ae --- /dev/null +++ b/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 diff --git a/tests/neg/19414.check b/tests/neg/19414.check new file mode 100644 index 000000000000..03e3fc1aaefe --- /dev/null +++ b/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 diff --git a/tests/neg/19414.scala b/tests/neg/19414.scala new file mode 100644 index 000000000000..c827879e8774 --- /dev/null +++ b/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 diff --git a/tests/neg/given-ambiguous-default-1.check b/tests/neg/given-ambiguous-default-1.check new file mode 100644 index 000000000000..a86df971c7ca --- /dev/null +++ b/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 diff --git a/tests/neg/given-ambiguous-default-1.scala b/tests/neg/given-ambiguous-default-1.scala new file mode 100644 index 000000000000..9d0297827cfa --- /dev/null +++ b/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 diff --git a/tests/neg/given-ambiguous-default-2.check b/tests/neg/given-ambiguous-default-2.check new file mode 100644 index 000000000000..0b1fb6c87d25 --- /dev/null +++ b/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 diff --git a/tests/neg/given-ambiguous-default-2.scala b/tests/neg/given-ambiguous-default-2.scala new file mode 100644 index 000000000000..33afd1d7b0a0 --- /dev/null +++ b/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