From f43543f454b8cdb5c8309bc52522ee7c2ff2a61c Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 23 Jan 2025 21:55:02 -0800 Subject: [PATCH] Restrict implicit args to using Typer#adaptNoArgsImplicitMethod populates implicit args when an arg list is missing. To remedy missing implicits, it tries a named application `using` args it did find. Then Applications#tryDefault supplies a default arg if available. A previous fix to allow tryDefault to supply implicit args for `implicit` params is now restricted to explicit `using`; typer now adds `using` for `implicit` when it needs to try defaults. This commit restores propagatedFailure and the previous condition that default params are tried if there is an error that is not an ambiguity. An additional restriction is that default params must be useful: there must be a param which has a default arg to be added (because it's not a named arg). --- .../dotty/tools/dotc/typer/Applications.scala | 7 +- .../dotty/tools/dotc/typer/Implicits.scala | 2 + .../src/dotty/tools/dotc/typer/Typer.scala | 143 ++++++++++-------- tests/neg/given-ambiguous-default-1.check | 8 +- tests/neg/i18123.check | 6 +- tests/neg/i19594.check | 28 +++- tests/neg/i19594.scala | 15 +- tests/neg/i22439.check | 22 +++ tests/neg/i22439.scala | 17 +++ tests/run/extra-implicits.scala | 16 +- 10 files changed, 170 insertions(+), 94 deletions(-) create mode 100644 tests/neg/i22439.check create mode 100644 tests/neg/i22439.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 96590cb84544..058cd2de332c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -757,13 +757,14 @@ trait Applications extends Compatibility { } else defaultArgument(normalizedFun, n, testOnly) - def implicitArg = implicitArgTree(formal, appPos.span) - if !defaultArg.isEmpty then defaultArg.tpe.widen match case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1) case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1) - else if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled) then + else if (methodType.isContextualMethod || applyKind == ApplyKind.Using && methodType.isImplicitMethod) + && ctx.mode.is(Mode.ImplicitsEnabled) + then + val implicitArg = implicitArgTree(formal, appPos.span) matchArgs(args1, addTyped(treeToArg(implicitArg)), n + 1) else missingArg(n) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 9d273ebca866..009fc2d19e8f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -570,6 +570,8 @@ object Implicits: i""" |Note that implicit $what cannot be applied because they are ambiguous; |$explanation""" :: Nil + + def asNested = if nested then this else AmbiguousImplicits(alt1, alt2, expectedType, argument, nested = true) end AmbiguousImplicits class MismatchedImplicit(ref: TermRef, diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index fd35471da07e..18d61a158f16 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -36,7 +36,6 @@ import util.{Property, SimpleIdentityMap, SrcPos} import Applications.{tupleComponentTypes, wrapDefs, defaultArgument} import collection.mutable -import annotation.tailrec import Implicits.* import util.Stats.record import config.Printers.{gadts, typr} @@ -52,7 +51,8 @@ import config.Config import config.MigrationVersion import transform.CheckUnused.OriginalName -import scala.annotation.constructorOnly +import scala.annotation.{unchecked as _, *} +import dotty.tools.dotc.util.chaining.* object Typer { @@ -4197,6 +4197,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def addImplicitArgs(using Context) = def hasDefaultParams = methPart(tree).symbol.hasDefaultParams + def findDefaultArgument(argIndex: Int): Tree = + def appPart(t: Tree): Tree = t match + case Block(_, expr) => appPart(expr) + case Inlined(_, _, expr) => appPart(expr) + case t => t + defaultArgument(appPart(tree), n = argIndex, testOnly = false) def implicitArgs(formals: List[Type], argIndex: Int, pt: Type): List[Tree] = formals match case Nil => Nil case formal :: formals1 => @@ -4218,13 +4224,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer then implicitArgs(formals, argIndex, pt1) else arg :: implicitArgs(formals1, argIndex + 1, pt1) case failed: SearchFailureType => - lazy val defaultArg = - def appPart(t: Tree): Tree = t match - case Block(stats, expr) => appPart(expr) - case Inlined(_, _, expr) => appPart(expr) - case _ => t - defaultArgument(appPart(tree), argIndex, testOnly = false) - .showing(i"default argument: for $formal, $tree, $argIndex = $result", typr) + lazy val defaultArg = findDefaultArgument(argIndex) + .showing(i"default argument: for $formal, $tree, $argIndex = $result", typr) if !hasDefaultParams || defaultArg.isEmpty then // no need to search further, the adapt fails in any case // the reason why we continue inferring arguments in case of an AmbiguousImplicits @@ -4246,44 +4247,44 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer arg :: inferArgsAfter(arg) end implicitArgs - /** Reports errors for arguments of `appTree` that have a - * `SearchFailureType`. - */ - def issueErrors(fun: Tree, args: List[Tree]): Tree = - // Prefer other errors over ambiguities. If nested in outer searches a missing - // implicit can be healed by simply dropping this alternative and trying something - // else. But an ambiguity is sticky and propagates outwards. If we have both - // a missing implicit on one argument and an ambiguity on another the whole - // branch should be classified as a missing implicit. - val firstNonAmbiguous = args.tpes.find(tp => tp.isError && !tp.isInstanceOf[AmbiguousImplicits]) - def firstError = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType) - def firstFailure = firstNonAmbiguous.getOrElse(firstError) - 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 - 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, paramStr, paramSymWithMethodCallTree), - tree.srcPos.endPos - ) - case _ => - } - - res + // Pick a failure type to propagate, if any. + // Prefer other errors over ambiguities. If nested in outer searches a missing + // implicit can be healed by simply dropping this alternative and trying something + // else. But an ambiguity is sticky and propagates outwards. If we have both + // a missing implicit on one argument and an ambiguity on another the whole + // branch should be classified as a missing implicit. + def propagatedFailure(args: List[Tree]): Type = args match + case arg :: args => arg.tpe match + case ambi: AmbiguousImplicits => propagatedFailure(args) match + case NoType | (_: AmbiguousImplicits) => ambi + case failed => failed + case failed: SearchFailureType => failed + case _ => propagatedFailure(args) + case Nil => NoType + + /** Reports errors for arguments of `appTree` that have a `SearchFailureType`. + */ + def issueErrors(fun: Tree, args: List[Tree], failureType: Type): Tree = + val errorType = failureType match + case ai: AmbiguousImplicits => ai.asNested + case tp => tp + untpd.Apply(fun, args) + .withType(errorType) + .tap: res => + wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach: (paramName, formal, arg) => + 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)) + val msg = missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree) + report.error(msg, tree.srcPos.endPos) + case _ => val args = implicitArgs(wtp.paramInfos, 0, pt) - if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) { + val failureType = propagatedFailure(args) + if failureType.exists then // 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 @@ -4292,32 +4293,40 @@ 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 then + if hasDefaultParams && !failureType.isInstanceOf[AmbiguousImplicits] then // Only keep the arguments that don't have an error type, or that - // have an `AmbiguousImplicits` error type. The later ensures that a + // have an `AmbiguousImplicits` error type. The latter 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") - 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) - } + wtp.paramNames.lazyZip(args).collect: + case (pname, arg) if !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits] => + untpd.NamedArg(pname, untpd.TypedSplice(arg)) + .toList + val usingDefaultArgs = + wtp.paramNames.zipWithIndex + .exists((n, i) => !namedArgs.exists(_.name == n) && !findDefaultArgument(i).isEmpty) + + if !usingDefaultArgs then + issueErrors(tree, args, failureType) + else + val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs) + // old-style implicit needs to be marked using so that implicits are searched + val needsUsing = wtp.isImplicitMethod || 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") + // 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. + typed(app, pt, locked) match + case retyped @ Apply(tree, args) if retyped.tpe.isError => + propagatedFailure(args) match + case sft: SearchFailureType => issueErrors(tree, args, sft) + case _ => issueErrors(tree, args, retyped.tpe) + case retyped => retyped + else issueErrors(tree, args, failureType) else inContext(origCtx): // Reset context in case it was set to a supercall context before. diff --git a/tests/neg/given-ambiguous-default-1.check b/tests/neg/given-ambiguous-default-1.check index 1a5006c23055..97ef6126909b 100644 --- a/tests/neg/given-ambiguous-default-1.check +++ b/tests/neg/given-ambiguous-default-1.check @@ -1,9 +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: + | 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]) + | 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. + | But both given instance a1 and given instance a2 match type A. diff --git a/tests/neg/i18123.check b/tests/neg/i18123.check index d784c4d12673..a36ed8822de8 100644 --- a/tests/neg/i18123.check +++ b/tests/neg/i18123.check @@ -1,7 +1,7 @@ --- [E172] Type Error: tests/neg/i18123.scala:25:33 --------------------------------------------------------------------- +-- [E172] Type Error: tests/neg/i18123.scala:25:48 --------------------------------------------------------------------- 25 | (charClassIntersection.rep() | classItem.rep()) // error - | ^^^^^^^^^^^^^^^ - |No given instance of type pkg.Implicits.Repeater[pkg.RegexTree, V] was found. + | ^ + |No given instance of type pkg.Implicits.Repeater[pkg.RegexTree, V] was found for parameter repeater of method rep in package pkg. |I found: | | pkg.Implicits.Repeater.GenericRepeaterImplicit[T] diff --git a/tests/neg/i19594.check b/tests/neg/i19594.check index bb9ff3fc68af..732721c544ce 100644 --- a/tests/neg/i19594.check +++ b/tests/neg/i19594.check @@ -1,8 +1,20 @@ --- [E172] Type Error: tests/neg/i19594.scala:12:14 --------------------------------------------------------------------- -12 | assertEquals(true, 1, "values are not the same") // error - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | Can you see me?! --- [E172] Type Error: tests/neg/i19594.scala:13:14 --------------------------------------------------------------------- -13 | assertEquals(true, 1) // error - | ^^^^^^^^^^^^^^^^^^^^^ - | Can you see me?! +-- [E172] Type Error: tests/neg/i19594.scala:15:50 --------------------------------------------------------------------- +15 | assertEquals(true, 1, "values are not the same") // error + | ^ + | Can you see me?! +-- [E172] Type Error: tests/neg/i19594.scala:16:23 --------------------------------------------------------------------- +16 | assertEquals(true, 1) // error + | ^ + | Can you see me?! +-- [E172] Type Error: tests/neg/i19594.scala:20:12 --------------------------------------------------------------------- +20 | f(true, 1) // error + | ^ + | Can you see me?! +-- [E172] Type Error: tests/neg/i19594.scala:23:39 --------------------------------------------------------------------- +23 | g(true, 1, "values are not the same") // error + | ^ + | Can you see me?! +-- [E172] Type Error: tests/neg/i19594.scala:26:3 ---------------------------------------------------------------------- +26 | h(true, 1) // error + | ^^^^^^^^^^ + | No given instance of type String was found diff --git a/tests/neg/i19594.scala b/tests/neg/i19594.scala index a559da8d9250..6f5453dcdf23 100644 --- a/tests/neg/i19594.scala +++ b/tests/neg/i19594.scala @@ -1,7 +1,10 @@ import scala.annotation.implicitNotFound @implicitNotFound("Can you see me?!") -trait Compare[A, B] +trait Compare[-A, -B] + +object Compare: + val any: Compare[Any, Any] = new Compare {} object example extends App: @@ -11,3 +14,13 @@ object example extends App: assertEquals(true, 1, "values are not the same") // error assertEquals(true, 1) // error + +object updated: + def f[A, B](a: A, b: B)(using Compare[A, B]) = () + f(true, 1) // error + + def g[A, B](a: A, b: B, clue: => Any)(implicit comp: Compare[A, B]) = () + g(true, 1, "values are not the same") // error + + def h[A, B](a: A, b: B)(using c: Compare[A, B] = Compare.any, s: String) = () + h(true, 1) // error diff --git a/tests/neg/i22439.check b/tests/neg/i22439.check new file mode 100644 index 000000000000..d4ef5c4c7507 --- /dev/null +++ b/tests/neg/i22439.check @@ -0,0 +1,22 @@ +-- [E171] Type Error: tests/neg/i22439.scala:7:3 ----------------------------------------------------------------------- +7 | f() // error f() missing arg + | ^^^ + | missing argument for parameter i of method f: (implicit i: Int, j: Int): Int +-- [E050] Type Error: tests/neg/i22439.scala:8:2 ----------------------------------------------------------------------- +8 | g() // error g(given_Int, given_Int)() doesn't take more params + | ^ + | method g does not take more parameters + | + | longer explanation available when compiling with `-explain` +-- [E171] Type Error: tests/neg/i22439.scala:11:3 ---------------------------------------------------------------------- +11 | f(j = 27) // error missing argument for parameter i of method f + | ^^^^^^^^^ + | missing argument for parameter i of method f: (implicit i: Int, j: Int): Int +-- [E172] Type Error: tests/neg/i22439.scala:16:3 ---------------------------------------------------------------------- +16 | h // error + | ^ + | No given instance of type String was found for parameter s of method h +-- [E172] Type Error: tests/neg/i22439.scala:17:3 ---------------------------------------------------------------------- +17 | h(using i = 17) // error + | ^^^^^^^^^^^^^^^ + | No given instance of type String was found diff --git a/tests/neg/i22439.scala b/tests/neg/i22439.scala new file mode 100644 index 000000000000..0e5182f6487b --- /dev/null +++ b/tests/neg/i22439.scala @@ -0,0 +1,17 @@ + +@main def test() = println: + given Int = 42 + def f(implicit i: Int, j: Int) = i + j + def g(using i: Int, j: Int) = i + j + val x: Int = f + f() // error f() missing arg + g() // error g(given_Int, given_Int)() doesn't take more params + f // ok implicits + g // ok implicits + f(j = 27) // error missing argument for parameter i of method f + f(using j = 27) // ok, explicit supplemented by implicit + g(using j = 27) // ok, explicit supplemented by implicit + + def h(using i: Int, s: String) = s * i + h // error + h(using i = 17) // error diff --git a/tests/run/extra-implicits.scala b/tests/run/extra-implicits.scala index 62ff862c709f..45978341556c 100644 --- a/tests/run/extra-implicits.scala +++ b/tests/run/extra-implicits.scala @@ -1,18 +1,18 @@ case class A(x: String) case class B(x: String) -given a1: A("default") -given b1: B("default") -val a2 = A("explicit") -val b2 = B("explicit") +given A("default") +given B("default") +val a = A("explicit") +val b = B("explicit") def f(using a: A, b: B): Unit = println(a) println(b) @main def Test = - f(using a2) - f(using a = a2) - f(using b = b2) - f(using b = b2, a = a2) + f(using a) + f(using a = a) + f(using b = b) + f(using b = b, a = a)