Skip to content

Commit 3e08108

Browse files
som-snytttgodzik
authored andcommitted
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).
1 parent db3107c commit 3e08108

File tree

10 files changed

+175
-90
lines changed

10 files changed

+175
-90
lines changed

compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ object CheckUnused:
525525
def checkPrivate(sym: Symbol, pos: SrcPos) =
526526
if ctx.settings.WunusedHas.privates
527527
&& !sym.isPrimaryConstructor
528-
&& sym.is(Private, butNot = SelfName | Synthetic | CaseAccessor)
528+
&& !sym.isOneOf(SelfName | Synthetic | CaseAccessor)
529529
&& !sym.name.is(BodyRetainerName)
530530
&& !sym.isSerializationSupport
531531
&& !(sym.is(Mutable) && sym.isSetter && sym.owner.is(Trait)) // tracks sym.underlyingSymbol sibling getter

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -673,13 +673,14 @@ trait Applications extends Compatibility {
673673
}
674674
else defaultArgument(normalizedFun, n, testOnly)
675675

676-
def implicitArg = implicitArgTree(formal, appPos.span)
677-
678676
if !defaultArg.isEmpty then
679677
defaultArg.tpe.widen match
680678
case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1)
681679
case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1)
682-
else if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled) then
680+
else if (methodType.isContextualMethod || applyKind == ApplyKind.Using && methodType.isImplicitMethod)
681+
&& ctx.mode.is(Mode.ImplicitsEnabled)
682+
then
683+
val implicitArg = implicitArgTree(formal, appPos.span)
683684
matchArgs(args1, addTyped(treeToArg(implicitArg)), n + 1)
684685
else
685686
missingArg(n)

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ object Implicits:
514514
override def toString = s"TooUnspecific"
515515

516516
/** An ambiguous implicits failure */
517-
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType {
517+
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
518518

519519
def msg(using Context): Message =
520520
var str1 = err.refStr(alt1.ref)
@@ -531,7 +531,10 @@ object Implicits:
531531
i"""
532532
|Note that implicit $what cannot be applied because they are ambiguous;
533533
|$explanation"""
534-
}
534+
535+
def asNested = if nested then this else AmbiguousImplicits(alt1, alt2, expectedType, argument, nested = true)
536+
537+
end AmbiguousImplicits
535538

536539
class MismatchedImplicit(ref: TermRef,
537540
val expectedType: Type,

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

Lines changed: 72 additions & 63 deletions
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}
@@ -53,7 +52,9 @@ import config.Config
5352
import transform.CheckUnused.OriginalName
5453

5554
import scala.annotation.constructorOnly
55+
import scala.annotation.{unchecked as _, *}
5656
import dotty.tools.dotc.rewrites.Rewrites
57+
import dotty.tools.dotc.util.chaining.*
5758

5859
object Typer {
5960

@@ -3842,6 +3843,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38423843

38433844
def addImplicitArgs(using Context) =
38443845
def hasDefaultParams = methPart(tree).symbol.hasDefaultParams
3846+
def findDefaultArgument(argIndex: Int): Tree =
3847+
def appPart(t: Tree): Tree = t match
3848+
case Block(_, expr) => appPart(expr)
3849+
case Inlined(_, _, expr) => appPart(expr)
3850+
case t => t
3851+
defaultArgument(appPart(tree), n = argIndex, testOnly = false)
38453852
def implicitArgs(formals: List[Type], argIndex: Int, pt: Type): List[Tree] = formals match
38463853
case Nil => Nil
38473854
case formal :: formals1 =>
@@ -3863,13 +3870,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38633870
then implicitArgs(formals, argIndex, pt1)
38643871
else arg :: implicitArgs(formals1, argIndex + 1, pt1)
38653872
case failed: SearchFailureType =>
3866-
lazy val defaultArg =
3867-
def appPart(t: Tree): Tree = t match
3868-
case Block(stats, expr) => appPart(expr)
3869-
case Inlined(_, _, expr) => appPart(expr)
3870-
case _ => t
3871-
defaultArgument(appPart(tree), argIndex, testOnly = false)
3872-
.showing(i"default argument: for $formal, $tree, $argIndex = $result", typr)
3873+
lazy val defaultArg = findDefaultArgument(argIndex)
3874+
.showing(i"default argument: for $formal, $tree, $argIndex = $result", typr)
38733875
if !hasDefaultParams || defaultArg.isEmpty then
38743876
// no need to search further, the adapt fails in any case
38753877
// the reason why we continue inferring arguments in case of an AmbiguousImplicits
@@ -3891,44 +3893,44 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38913893
arg :: inferArgsAfter(arg)
38923894
end implicitArgs
38933895

3894-
/** Reports errors for arguments of `appTree` that have a
3895-
* `SearchFailureType`.
3896-
*/
3897-
def issueErrors(fun: Tree, args: List[Tree]): Tree =
3898-
// Prefer other errors over ambiguities. If nested in outer searches a missing
3899-
// implicit can be healed by simply dropping this alternative and trying something
3900-
// else. But an ambiguity is sticky and propagates outwards. If we have both
3901-
// a missing implicit on one argument and an ambiguity on another the whole
3902-
// branch should be classified as a missing implicit.
3903-
val firstNonAmbiguous = args.tpes.find(tp => tp.isError && !tp.isInstanceOf[AmbiguousImplicits])
3904-
def firstError = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType)
3905-
def firstFailure = firstNonAmbiguous.getOrElse(firstError)
3906-
val errorType =
3907-
firstFailure match
3908-
case tp: AmbiguousImplicits =>
3909-
AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true)
3910-
case tp =>
3911-
tp
3912-
val res = untpd.Apply(fun, args).withType(errorType)
3913-
3914-
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
3915-
arg.tpe match
3916-
case failure: SearchFailureType =>
3917-
val methodStr = err.refStr(methPart(fun).tpe)
3918-
val paramStr = implicitParamString(paramName, methodStr, fun)
3919-
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
3920-
val paramSymWithMethodCallTree = paramSym.map((_, res))
3921-
report.error(
3922-
missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree),
3923-
tree.srcPos.endPos
3924-
)
3925-
case _ =>
3926-
}
3927-
3928-
res
3896+
// Pick a failure type to propagate, if any.
3897+
// Prefer other errors over ambiguities. If nested in outer searches a missing
3898+
// implicit can be healed by simply dropping this alternative and trying something
3899+
// else. But an ambiguity is sticky and propagates outwards. If we have both
3900+
// a missing implicit on one argument and an ambiguity on another the whole
3901+
// branch should be classified as a missing implicit.
3902+
def propagatedFailure(args: List[Tree]): Type = args match
3903+
case arg :: args => arg.tpe match
3904+
case ambi: AmbiguousImplicits => propagatedFailure(args) match
3905+
case NoType | (_: AmbiguousImplicits) => ambi
3906+
case failed => failed
3907+
case failed: SearchFailureType => failed
3908+
case _ => propagatedFailure(args)
3909+
case Nil => NoType
3910+
3911+
/** Reports errors for arguments of `appTree` that have a `SearchFailureType`.
3912+
*/
3913+
def issueErrors(fun: Tree, args: List[Tree], failureType: Type): Tree =
3914+
val errorType = failureType match
3915+
case ai: AmbiguousImplicits => ai.asNested
3916+
case tp => tp
3917+
untpd.Apply(fun, args)
3918+
.withType(errorType)
3919+
.tap: res =>
3920+
wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach: (paramName, formal, arg) =>
3921+
arg.tpe match
3922+
case failure: SearchFailureType =>
3923+
val methodStr = err.refStr(methPart(fun).tpe)
3924+
val paramStr = implicitParamString(paramName, methodStr, fun)
3925+
val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
3926+
val paramSymWithMethodCallTree = paramSym.map((_, res))
3927+
val msg = missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree)
3928+
report.error(msg, tree.srcPos.endPos)
3929+
case _ =>
39293930

39303931
val args = implicitArgs(wtp.paramInfos, 0, pt)
3931-
if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
3932+
val failureType = propagatedFailure(args)
3933+
if failureType.exists then
39323934
// If there are several arguments, some arguments might already
39333935
// have influenced the context, binding variables, but later ones
39343936
// might fail. In that case the constraint and instantiated variables
@@ -3937,29 +3939,36 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
39373939

39383940
// If method has default params, fall back to regular application
39393941
// where all inferred implicits are passed as named args.
3940-
if hasDefaultParams then
3942+
if hasDefaultParams && !failureType.isInstanceOf[AmbiguousImplicits] then
39413943
// Only keep the arguments that don't have an error type, or that
3942-
// have an `AmbiguousImplicits` error type. The later ensures that a
3944+
// have an `AmbiguousImplicits` error type. The latter ensures that a
39433945
// default argument can't override an ambiguous implicit. See tests
39443946
// `given-ambiguous-default*` and `19414*`.
39453947
val namedArgs =
3946-
wtp.paramNames.lazyZip(args)
3947-
.filter((_, arg) => !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits])
3948-
.map((pname, arg) => untpd.NamedArg(pname, untpd.TypedSplice(arg)))
3949-
3950-
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
3951-
if (wtp.isContextualMethod) app.setApplyKind(ApplyKind.Using)
3952-
typr.println(i"try with default implicit args $app")
3953-
val retyped = typed(app, pt, locked)
3954-
3955-
// If the retyped tree still has an error type and is an `Apply`
3956-
// node, we can report the errors for each argument nicely.
3957-
// Otherwise, we don't report anything here.
3958-
retyped match
3959-
case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args)
3960-
case _ => retyped
3961-
else issueErrors(tree, args)
3962-
}
3948+
wtp.paramNames.lazyZip(args).collect:
3949+
case (pname, arg) if !arg.tpe.isError || arg.tpe.isInstanceOf[AmbiguousImplicits] =>
3950+
untpd.NamedArg(pname, untpd.TypedSplice(arg))
3951+
.toList
3952+
val usingDefaultArgs =
3953+
wtp.paramNames.zipWithIndex
3954+
.exists((n, i) => !namedArgs.exists(_.name == n) && !findDefaultArgument(i).isEmpty)
3955+
3956+
if !usingDefaultArgs then
3957+
issueErrors(tree, args, failureType)
3958+
else
3959+
val app = cpy.Apply(tree)(untpd.TypedSplice(tree), namedArgs)
3960+
// old-style implicit needs to be marked using so that implicits are searched
3961+
typr.println(i"try with default implicit args $app")
3962+
// If the retyped tree still has an error type and is an `Apply`
3963+
// node, we can report the errors for each argument nicely.
3964+
// Otherwise, we don't report anything here.
3965+
typed(app, pt, locked) match
3966+
case retyped @ Apply(tree, args) if retyped.tpe.isError =>
3967+
propagatedFailure(args) match
3968+
case sft: SearchFailureType => issueErrors(tree, args, sft)
3969+
case _ => issueErrors(tree, args, retyped.tpe)
3970+
case retyped => retyped
3971+
else issueErrors(tree, args, failureType)
39633972
else
39643973
inContext(origCtx):
39653974
// Reset context in case it was set to a supercall context before.
Lines changed: 4 additions & 4 deletions
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/i19594.check

Lines changed: 20 additions & 8 deletions
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

Lines changed: 14 additions & 1 deletion
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

Lines changed: 26 additions & 0 deletions
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

Lines changed: 21 additions & 0 deletions
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

Lines changed: 8 additions & 8 deletions
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)