Skip to content

Commit

Permalink
Restrict implicit args to using
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
som-snytt committed Feb 2, 2025
1 parent 590691b commit 32c0f84
Show file tree
Hide file tree
Showing 10 changed files with 171 additions and 95 deletions.
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -755,13 +755,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)
Expand Down Expand Up @@ -1198,7 +1199,7 @@ trait Applications extends Compatibility {
//
// summonFrom {
// case given A[t] =>
// summonFrom
// summonFrom {
// case given `t` => ...
// }
// }
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
143 changes: 76 additions & 67 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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 {

Expand Down Expand Up @@ -4193,6 +4193,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 =>
Expand All @@ -4214,13 +4220,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
Expand All @@ -4242,44 +4243,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
Expand All @@ -4288,32 +4289,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.
Expand Down
8 changes: 4 additions & 4 deletions tests/neg/given-ambiguous-default-1.check
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 3 additions & 3 deletions tests/neg/i18123.check
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
28 changes: 20 additions & 8 deletions tests/neg/i19594.check
Original file line number Diff line number Diff line change
@@ -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
15 changes: 14 additions & 1 deletion tests/neg/i19594.scala
Original file line number Diff line number Diff line change
@@ -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:

Expand All @@ -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
22 changes: 22 additions & 0 deletions tests/neg/i22439.check
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions tests/neg/i22439.scala
Original file line number Diff line number Diff line change
@@ -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
16 changes: 8 additions & 8 deletions tests/run/extra-implicits.scala
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 32c0f84

Please sign in to comment.