Skip to content

Commit 798987a

Browse files
sjrdsjrd
authored
Restrict implicit args to using (#22458)
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). Fixes #22439
2 parents a59d3e1 + c37dc8b commit 798987a

11 files changed

+179
-95
lines changed

‎compiler/src/dotty/tools/dotc/transform/init/Objects.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ class Objects(using Context @constructorOnly):
15711571
val trace2 = Trace.trace.add(pat)
15721572
pat match
15731573
case Alternative(pats) =>
1574-
val (types, values) = pats.map(evalPattern(scrutinee, _)).unzip()
1574+
val (types, values) = pats.map(evalPattern(scrutinee, _)).unzip
15751575
val orType = types.fold(defn.NothingType)(OrType(_, _, false))
15761576
(orType, values.join)
15771577

‎compiler/src/dotty/tools/dotc/typer/Applications.scala

+4-3
Original file line numberDiff line numberDiff line change
@@ -757,13 +757,14 @@ trait Applications extends Compatibility {
757757
}
758758
else defaultArgument(normalizedFun, n, testOnly)
759759

760-
def implicitArg = implicitArgTree(formal, appPos.span)
761-
762760
if !defaultArg.isEmpty then
763761
defaultArg.tpe.widen match
764762
case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1)
765763
case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1)
766-
else if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled) then
764+
else if (methodType.isContextualMethod || applyKind == ApplyKind.Using && methodType.isImplicitMethod)
765+
&& ctx.mode.is(Mode.ImplicitsEnabled)
766+
then
767+
val implicitArg = implicitArgTree(formal, appPos.span)
767768
matchArgs(args1, addTyped(treeToArg(implicitArg)), n + 1)
768769
else
769770
missingArg(n)

‎compiler/src/dotty/tools/dotc/typer/Implicits.scala

+2
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,8 @@ object Implicits:
570570
i"""
571571
|Note that implicit $what cannot be applied because they are ambiguous;
572572
|$explanation""" :: Nil
573+
574+
def asNested = if nested then this else AmbiguousImplicits(alt1, alt2, expectedType, argument, nested = true)
573575
end AmbiguousImplicits
574576

575577
class MismatchedImplicit(ref: TermRef,

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

+76-67
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import util.{Property, SimpleIdentityMap, SrcPos}
3636
import Applications.{tupleComponentTypes, wrapDefs, defaultArgument}
3737

3838
import collection.mutable
39-
import annotation.tailrec
4039
import Implicits.*
4140
import util.Stats.record
4241
import config.Printers.{gadts, typr}
@@ -52,7 +51,8 @@ import config.Config
5251
import config.MigrationVersion
5352
import transform.CheckUnused.OriginalName
5453

55-
import scala.annotation.constructorOnly
54+
import scala.annotation.{unchecked as _, *}
55+
import dotty.tools.dotc.util.chaining.*
5656

5757
object Typer {
5858

@@ -4239,6 +4239,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
42394239

42404240
def addImplicitArgs(using Context) =
42414241
def hasDefaultParams = methPart(tree).symbol.hasDefaultParams
4242+
def findDefaultArgument(argIndex: Int): Tree =
4243+
def appPart(t: Tree): Tree = t match
4244+
case Block(_, expr) => appPart(expr)
4245+
case Inlined(_, _, expr) => appPart(expr)
4246+
case t => t
4247+
defaultArgument(appPart(tree), n = argIndex, testOnly = false)
42424248
def implicitArgs(formals: List[Type], argIndex: Int, pt: Type): List[Tree] = formals match
42434249
case Nil => Nil
42444250
case formal :: formals1 =>
@@ -4260,13 +4266,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
42604266
then implicitArgs(formals, argIndex, pt1)
42614267
else arg :: implicitArgs(formals1, argIndex + 1, pt1)
42624268
case failed: SearchFailureType =>
4263-
lazy val defaultArg =
4264-
def appPart(t: Tree): Tree = t match
4265-
case Block(stats, expr) => appPart(expr)
4266-
case Inlined(_, _, expr) => appPart(expr)
4267-
case _ => t
4268-
defaultArgument(appPart(tree), argIndex, testOnly = false)
4269-
.showing(i"default argument: for $formal, $tree, $argIndex = $result", typr)
4269+
lazy val defaultArg = findDefaultArgument(argIndex)
4270+
.showing(i"default argument: for $formal, $tree, $argIndex = $result", typr)
42704271
if !hasDefaultParams || defaultArg.isEmpty then
42714272
// no need to search further, the adapt fails in any case
42724273
// the reason why we continue inferring arguments in case of an AmbiguousImplicits
@@ -4288,44 +4289,44 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
42884289
arg :: inferArgsAfter(arg)
42894290
end implicitArgs
42904291

4291-
/** Reports errors for arguments of `appTree` that have a
4292-
* `SearchFailureType`.
4293-
*/
4294-
def issueErrors(fun: Tree, args: List[Tree]): Tree =
4295-
// Prefer other errors over ambiguities. If nested in outer searches a missing
4296-
// implicit can be healed by simply dropping this alternative and trying something
4297-
// else. But an ambiguity is sticky and propagates outwards. If we have both
4298-
// a missing implicit on one argument and an ambiguity on another the whole
4299-
// branch should be classified as a missing implicit.
4300-
val firstNonAmbiguous = args.tpes.find(tp => tp.isError && !tp.isInstanceOf[AmbiguousImplicits])
4301-
def firstError = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType)
4302-
def firstFailure = firstNonAmbiguous.getOrElse(firstError)
4303-
val errorType =
4304-
firstFailure match
4305-
case tp: AmbiguousImplicits =>
4306-
AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true)
4307-
case tp =>
4308-
tp
4309-
val res = untpd.Apply(fun, args).withType(errorType)
4310-
4311-
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
4312-
arg.tpe match
4313-
case failure: SearchFailureType =>
4314-
val methodStr = err.refStr(methPart(fun).tpe)
4315-
val paramStr = implicitParamString(paramName, methodStr, fun)
4316-
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
4317-
val paramSymWithMethodCallTree = paramSym.map((_, res))
4318-
report.error(
4319-
missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree),
4320-
tree.srcPos.endPos
4321-
)
4322-
case _ =>
4323-
}
4324-
4325-
res
4292+
// Pick a failure type to propagate, if any.
4293+
// Prefer other errors over ambiguities. If nested in outer searches a missing
4294+
// implicit can be healed by simply dropping this alternative and trying something
4295+
// else. But an ambiguity is sticky and propagates outwards. If we have both
4296+
// a missing implicit on one argument and an ambiguity on another the whole
4297+
// branch should be classified as a missing implicit.
4298+
def propagatedFailure(args: List[Tree]): Type = args match
4299+
case arg :: args => arg.tpe match
4300+
case ambi: AmbiguousImplicits => propagatedFailure(args) match
4301+
case NoType | (_: AmbiguousImplicits) => ambi
4302+
case failed => failed
4303+
case failed: SearchFailureType => failed
4304+
case _ => propagatedFailure(args)
4305+
case Nil => NoType
4306+
4307+
/** Reports errors for arguments of `appTree` that have a `SearchFailureType`.
4308+
*/
4309+
def issueErrors(fun: Tree, args: List[Tree], failureType: Type): Tree =
4310+
val errorType = failureType match
4311+
case ai: AmbiguousImplicits => ai.asNested
4312+
case tp => tp
4313+
untpd.Apply(fun, args)
4314+
.withType(errorType)
4315+
.tap: res =>
4316+
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach: (paramName, formal, arg) =>
4317+
arg.tpe match
4318+
case failure: SearchFailureType =>
4319+
val methodStr = err.refStr(methPart(fun).tpe)
4320+
val paramStr = implicitParamString(paramName, methodStr, fun)
4321+
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
4322+
val paramSymWithMethodCallTree = paramSym.map((_, res))
4323+
val msg = missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree)
4324+
report.error(msg, tree.srcPos.endPos)
4325+
case _ =>
43264326

43274327
val args = implicitArgs(wtp.paramInfos, 0, pt)
4328-
if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
4328+
val failureType = propagatedFailure(args)
4329+
if failureType.exists then
43294330
// If there are several arguments, some arguments might already
43304331
// have influenced the context, binding variables, but later ones
43314332
// might fail. In that case the constraint and instantiated variables
@@ -4334,32 +4335,40 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
43344335

43354336
// If method has default params, fall back to regular application
43364337
// where all inferred implicits are passed as named args.
4337-
if hasDefaultParams then
4338+
if hasDefaultParams && !failureType.isInstanceOf[AmbiguousImplicits] then
43384339
// Only keep the arguments that don't have an error type, or that
4339-
// have an `AmbiguousImplicits` error type. The later ensures that a
4340+
// have an `AmbiguousImplicits` error type. The latter ensures that a
43404341
// default argument can't override an ambiguous implicit. See tests
43414342
// `given-ambiguous-default*` and `19414*`.
43424343
val namedArgs =
4343-
wtp.paramNames.lazyZip(args)
4344-
.filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits])
4345-
.map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg)))
4346-
4347-
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
4348-
val needsUsing = wtp.isContextualMethod || wtp.match
4349-
case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`)
4350-
case _ => false
4351-
if needsUsing then app.setApplyKind(ApplyKind.Using)
4352-
typr.println(i"try with default implicit args $app")
4353-
val retyped = typed(app, pt, locked)
4354-
4355-
// If the retyped tree still has an error type and is an `Apply`
4356-
// node, we can report the errors for each argument nicely.
4357-
// Otherwise, we don't report anything here.
4358-
retyped match
4359-
case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args)
4360-
case _ => retyped
4361-
else issueErrors(tree, args)
4362-
}
4344+
wtp.paramNames.lazyZip(args).collect:
4345+
case (pname, arg) if !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits] =>
4346+
untpd.NamedArg(pname, untpd.TypedSplice(arg))
4347+
.toList
4348+
val usingDefaultArgs =
4349+
wtp.paramNames.zipWithIndex
4350+
.exists((n, i) => !namedArgs.exists(_.name == n) && !findDefaultArgument(i).isEmpty)
4351+
4352+
if !usingDefaultArgs then
4353+
issueErrors(tree, args, failureType)
4354+
else
4355+
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
4356+
// old-style implicit needs to be marked using so that implicits are searched
4357+
val needsUsing = wtp.isImplicitMethod || wtp.match
4358+
case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`)
4359+
case _ => false
4360+
if needsUsing then app.setApplyKind(ApplyKind.Using)
4361+
typr.println(i"try with default implicit args $app")
4362+
// If the retyped tree still has an error type and is an `Apply`
4363+
// node, we can report the errors for each argument nicely.
4364+
// Otherwise, we don't report anything here.
4365+
typed(app, pt, locked) match
4366+
case retyped @ Apply(tree, args) if retyped.tpe.isError =>
4367+
propagatedFailure(args) match
4368+
case sft: SearchFailureType => issueErrors(tree, args, sft)
4369+
case _ => issueErrors(tree, args, retyped.tpe)
4370+
case retyped => retyped
4371+
else issueErrors(tree, args, failureType)
43634372
else
43644373
inContext(origCtx):
43654374
// Reset context in case it was set to a supercall context before.

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
-- [E172] Type Error: tests/neg/given-ambiguous-default-1.scala:18:23 --------------------------------------------------
22
18 |def f: Unit = summon[B] // error: Ambiguous given instances
33
| ^
4-
| No best given instance of type B was found for parameter x of method summon in object Predef.
5-
| I found:
4+
| No best given instance of type B was found for parameter x of method summon in object Predef.
5+
| I found:
66
|
7-
| given_B(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
7+
| given_B(/* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
88
|
9-
| But both given instance a1 and given instance a2 match type A.
9+
| But both given instance a1 and given instance a2 match type A.

‎tests/neg/i18123.check

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
-- [E172] Type Error: tests/neg/i18123.scala:25:33 ---------------------------------------------------------------------
1+
-- [E172] Type Error: tests/neg/i18123.scala:25:48 ---------------------------------------------------------------------
22
25 | (charClassIntersection.rep() | classItem.rep()) // error
3-
| ^^^^^^^^^^^^^^^
4-
|No given instance of type pkg.Implicits.Repeater[pkg.RegexTree, V] was found.
3+
| ^
4+
|No given instance of type pkg.Implicits.Repeater[pkg.RegexTree, V] was found for parameter repeater of method rep in package pkg.
55
|I found:
66
|
77
| pkg.Implicits.Repeater.GenericRepeaterImplicit[T]

‎tests/neg/i19594.check

+20-8
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
1-
-- [E172] Type Error: tests/neg/i19594.scala:12:14 ---------------------------------------------------------------------
2-
12 | assertEquals(true, 1, "values are not the same") // error
3-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4-
| Can you see me?!
5-
-- [E172] Type Error: tests/neg/i19594.scala:13:14 ---------------------------------------------------------------------
6-
13 | assertEquals(true, 1) // error
7-
| ^^^^^^^^^^^^^^^^^^^^^
8-
| Can you see me?!
1+
-- [E172] Type Error: tests/neg/i19594.scala:15:50 ---------------------------------------------------------------------
2+
15 | assertEquals(true, 1, "values are not the same") // error
3+
| ^
4+
| Can you see me?!
5+
-- [E172] Type Error: tests/neg/i19594.scala:16:23 ---------------------------------------------------------------------
6+
16 | assertEquals(true, 1) // error
7+
| ^
8+
| Can you see me?!
9+
-- [E172] Type Error: tests/neg/i19594.scala:20:12 ---------------------------------------------------------------------
10+
20 | f(true, 1) // error
11+
| ^
12+
| Can you see me?!
13+
-- [E172] Type Error: tests/neg/i19594.scala:23:39 ---------------------------------------------------------------------
14+
23 | g(true, 1, "values are not the same") // error
15+
| ^
16+
| Can you see me?!
17+
-- [E172] Type Error: tests/neg/i19594.scala:26:3 ----------------------------------------------------------------------
18+
26 | h(true, 1) // error
19+
| ^^^^^^^^^^
20+
| No given instance of type String was found

‎tests/neg/i19594.scala

+14-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import scala.annotation.implicitNotFound
22

33
@implicitNotFound("Can you see me?!")
4-
trait Compare[A, B]
4+
trait Compare[-A, -B]
5+
6+
object Compare:
7+
val any: Compare[Any, Any] = new Compare {}
58

69
object example extends App:
710

@@ -11,3 +14,13 @@ object example extends App:
1114

1215
assertEquals(true, 1, "values are not the same") // error
1316
assertEquals(true, 1) // error
17+
18+
object updated:
19+
def f[A, B](a: A, b: B)(using Compare[A, B]) = ()
20+
f(true, 1) // error
21+
22+
def g[A, B](a: A, b: B, clue: => Any)(implicit comp: Compare[A, B]) = ()
23+
g(true, 1, "values are not the same") // error
24+
25+
def h[A, B](a: A, b: B)(using c: Compare[A, B] = Compare.any, s: String) = ()
26+
h(true, 1) // error

‎tests/neg/i22439.check

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
-- [E171] Type Error: tests/neg/i22439.scala:7:3 -----------------------------------------------------------------------
2+
7 | f() // error f() missing arg
3+
| ^^^
4+
| missing argument for parameter i of method f: (implicit i: Int, j: Int): Int
5+
-- [E050] Type Error: tests/neg/i22439.scala:8:2 -----------------------------------------------------------------------
6+
8 | g() // error g(given_Int, given_Int)() doesn't take more params
7+
| ^
8+
| method g does not take more parameters
9+
|
10+
| longer explanation available when compiling with `-explain`
11+
-- [E171] Type Error: tests/neg/i22439.scala:11:3 ----------------------------------------------------------------------
12+
11 | f(j = 27) // error missing argument for parameter i of method f
13+
| ^^^^^^^^^
14+
| missing argument for parameter i of method f: (implicit i: Int, j: Int): Int
15+
-- [E172] Type Error: tests/neg/i22439.scala:16:3 ----------------------------------------------------------------------
16+
16 | h // error
17+
| ^
18+
| No given instance of type String was found for parameter s of method h
19+
-- [E172] Type Error: tests/neg/i22439.scala:17:3 ----------------------------------------------------------------------
20+
17 | h(using i = 17) // error
21+
| ^^^^^^^^^^^^^^^
22+
| No given instance of type String was found
23+
-- [E171] Type Error: tests/neg/i22439.scala:21:25 ---------------------------------------------------------------------
24+
21 | val (ws, zs) = vs.unzip() // error!
25+
| ^^^^^^^^^^
26+
|missing argument for parameter asPair of method unzip in trait StrictOptimizedIterableOps: (implicit asPair: ((Int, Int)) => (A1, A2)): (List[A1], List[A2])

‎tests/neg/i22439.scala

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
2+
@main def test() = println:
3+
given Int = 42
4+
def f(implicit i: Int, j: Int) = i + j
5+
def g(using i: Int, j: Int) = i + j
6+
val x: Int = f
7+
f() // error f() missing arg
8+
g() // error g(given_Int, given_Int)() doesn't take more params
9+
f // ok implicits
10+
g // ok implicits
11+
f(j = 27) // error missing argument for parameter i of method f
12+
f(using j = 27) // ok, explicit supplemented by implicit
13+
g(using j = 27) // ok, explicit supplemented by implicit
14+
15+
def h(using i: Int, s: String) = s * i
16+
h // error
17+
h(using i = 17) // error
18+
19+
val vs = List((42, 27))
20+
val (xs, ys) = vs.unzip
21+
val (ws, zs) = vs.unzip() // error!

‎tests/run/extra-implicits.scala

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11

22
case class A(x: String)
33
case class B(x: String)
4-
given a1: A("default")
5-
given b1: B("default")
6-
val a2 = A("explicit")
7-
val b2 = B("explicit")
4+
given A("default")
5+
given B("default")
6+
val a = A("explicit")
7+
val b = B("explicit")
88

99
def f(using a: A, b: B): Unit =
1010
println(a)
1111
println(b)
1212

1313
@main def Test =
14-
f(using a2)
15-
f(using a = a2)
16-
f(using b = b2)
17-
f(using b = b2, a = a2)
14+
f(using a)
15+
f(using a = a)
16+
f(using b = b)
17+
f(using b = b, a = a)
1818

0 commit comments

Comments
 (0)