Skip to content

Commit a3798a2

Browse files
oderskyodersky
committed
Fix handling of default arguments in parameter-dependent using clauses
We had a problem in handling using clauses that were both parameter dependent and had default parameters. Example: trait A: type X f(using a: A = defaultA, b: a.X) If implicit search fails for `A`, we should search the second parameter with `defaultA.X` as type. We searched instead with an error type (i.e. the type of the missing argument for A). Sine error types are compatible with everything, any implicit candidate would match. In the original test i5427 we had just one closest nested candidate that was the right one, but once we add another candidate with a different type next to the first one the test fails.
1 parent 22c6b60 commit a3798a2

File tree

3 files changed

+147
-124
lines changed

3 files changed

+147
-124
lines changed

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

Lines changed: 102 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,107 @@ object Applications {
212212
overwriteType(app.tpe)
213213
// ExtMethodApply always has wildcard type in order not to prompt any further adaptations
214214
// such as eta expansion before the method is fully applied.
215+
}
216+
217+
/** Find reference to default parameter getter for parameter #n in current
218+
* parameter list, or NoType if none was found
219+
*/
220+
def findDefaultGetter(fn: Tree, n: Int, testOnly: Boolean)(using Context): Tree = {
221+
if fn.symbol.isTerm then
222+
val meth = fn.symbol.asTerm
223+
val receiver: Tree = methPart(fn) match {
224+
case Select(receiver, _) => receiver
225+
case mr => mr.tpe.normalizedPrefix match {
226+
case mr: TermRef => ref(mr)
227+
case mr =>
228+
if testOnly then
229+
// In this case it is safe to skolemize now; we will produce a stable prefix for the actual call.
230+
ref(mr.narrow)
231+
else
232+
EmptyTree
233+
}
234+
}
235+
val getterPrefix =
236+
if (meth.is(Synthetic) && meth.name == nme.apply) nme.CONSTRUCTOR else meth.name
237+
def getterName = DefaultGetterName(getterPrefix, n + numArgs(fn))
238+
if !meth.hasDefaultParams then
239+
EmptyTree
240+
else if (receiver.isEmpty) {
241+
def findGetter(cx: Context): Tree =
242+
if (cx eq NoContext) EmptyTree
243+
else if (cx.scope != cx.outer.scope &&
244+
cx.denotNamed(meth.name).hasAltWith(_.symbol == meth)) {
245+
val denot = cx.denotNamed(getterName)
246+
if (denot.exists) ref(TermRef(cx.owner.thisType, getterName, denot))
247+
else findGetter(cx.outer)
248+
}
249+
else findGetter(cx.outer)
250+
findGetter(ctx)
251+
}
252+
else {
253+
def selectGetter(qual: Tree): Tree = {
254+
val getterDenot = qual.tpe.member(getterName)
255+
if (getterDenot.exists) qual.select(TermRef(qual.tpe, getterName, getterDenot))
256+
else EmptyTree
257+
}
258+
if (!meth.isClassConstructor)
259+
selectGetter(receiver)
260+
else {
261+
// default getters for class constructors are found in the companion object
262+
val cls = meth.owner
263+
val companion = cls.companionModule
264+
if (companion.isTerm) {
265+
val prefix = receiver.tpe.baseType(cls).normalizedPrefix
266+
if (prefix.exists) selectGetter(ref(TermRef(prefix, companion.asTerm)))
267+
else EmptyTree
268+
}
269+
else EmptyTree
270+
}
271+
}
272+
else EmptyTree // structural applies don't have symbols or defaults
273+
}//.showing(i"find getter $fn, $n = $result")
274+
end findDefaultGetter
275+
276+
/** Splice new method reference `meth` into existing application `app` */
277+
private def spliceMeth(meth: Tree, app: Tree)(using Context): Tree = app match {
278+
case Apply(fn, args) =>
279+
spliceMeth(meth, fn).appliedToArgs(args)
280+
case TypeApply(fn, targs) =>
281+
// Note: It is important that the type arguments `targs` are passed in new trees
282+
// instead of being spliced in literally. Otherwise, a type argument to a default
283+
// method could be constructed as the definition site of the type variable for
284+
// that default constructor. This would interpolate type variables too early,
285+
// causing lots of tests (among them tasty_unpickleScala2) to fail.
286+
//
287+
// The test case is in i1757.scala. Here we have a variable `s` and a method `cpy`
288+
// defined like this:
289+
//
290+
// var s
291+
// def cpy[X](b: List[Int] = b): B[X] = new B[X](b)
292+
//
293+
// The call `s.cpy()` then gets expanded to
294+
//
295+
// { val $1$: B[Int] = this.s
296+
// $1$.cpy[X']($1$.cpy$default$1[X']
297+
// }
298+
//
299+
// A type variable gets interpolated if it does not appear in the type
300+
// of the current tree and the current tree contains the variable's "definition".
301+
// Previously, the polymorphic function tree to which the variable was first added
302+
// was taken as the variable's definition. But that fails here because that
303+
// tree was `s.cpy` but got transformed into `$1$.cpy`. We now take the type argument
304+
// [X'] of the variable as its definition tree, which is more robust. But then
305+
// it's crucial that the type tree is not copied directly as argument to
306+
// `cpy$default$1`. If it was, the variable `X'` would already be interpolated
307+
// when typing the default argument, which is too early.
308+
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
309+
case _ => meth
310+
}
311+
312+
def defaultArgument(fn: Tree, n: Int, testOnly: Boolean)(using Context): Tree =
313+
val getter = findDefaultGetter(fn, n, testOnly)
314+
if getter.isEmpty then getter
315+
else spliceMeth(getter.withSpan(fn.span), fn)
215316
}
216317

217318
trait Applications extends Compatibility {
@@ -412,98 +513,6 @@ trait Applications extends Compatibility {
412513
handlePositional(methodType.paramNames, args)
413514
}
414515

415-
/** Splice new method reference into existing application */
416-
def spliceMeth(meth: Tree, app: Tree): Tree = app match {
417-
case Apply(fn, args) =>
418-
spliceMeth(meth, fn).appliedToTermArgs(args)
419-
case TypeApply(fn, targs) =>
420-
// Note: It is important that the type arguments `targs` are passed in new trees
421-
// instead of being spliced in literally. Otherwise, a type argument to a default
422-
// method could be constructed as the definition site of the type variable for
423-
// that default constructor. This would interpolate type variables too early,
424-
// causing lots of tests (among them tasty_unpickleScala2) to fail.
425-
//
426-
// The test case is in i1757.scala. Here we have a variable `s` and a method `cpy`
427-
// defined like this:
428-
//
429-
// var s
430-
// def cpy[X](b: List[Int] = b): B[X] = new B[X](b)
431-
//
432-
// The call `s.cpy()` then gets expanded to
433-
//
434-
// { val $1$: B[Int] = this.s
435-
// $1$.cpy[X']($1$.cpy$default$1[X']
436-
// }
437-
//
438-
// A type variable gets interpolated if it does not appear in the type
439-
// of the current tree and the current tree contains the variable's "definition".
440-
// Previously, the polymorphic function tree to which the variable was first added
441-
// was taken as the variable's definition. But that fails here because that
442-
// tree was `s.cpy` but got transformed into `$1$.cpy`. We now take the type argument
443-
// [X'] of the variable as its definition tree, which is more robust. But then
444-
// it's crucial that the type tree is not copied directly as argument to
445-
// `cpy$default$1`. If it was, the variable `X'` would already be interpolated
446-
// when typing the default argument, which is too early.
447-
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
448-
case _ => meth
449-
}
450-
451-
/** Find reference to default parameter getter for parameter #n in current
452-
* parameter list, or NoType if none was found
453-
*/
454-
def findDefaultGetter(n: Int)(using Context): Tree = {
455-
val meth = methRef.symbol.asTerm
456-
val receiver: Tree = methPart(normalizedFun) match {
457-
case Select(receiver, _) => receiver
458-
case mr => mr.tpe.normalizedPrefix match {
459-
case mr: TermRef => ref(mr)
460-
case mr =>
461-
if (this.isInstanceOf[TestApplication[?]])
462-
// In this case it is safe to skolemize now; we will produce a stable prefix for the actual call.
463-
ref(mr.narrow)
464-
else
465-
EmptyTree
466-
}
467-
}
468-
val getterPrefix =
469-
if (meth.is(Synthetic) && meth.name == nme.apply) nme.CONSTRUCTOR else meth.name
470-
def getterName = DefaultGetterName(getterPrefix, n)
471-
if (!meth.hasDefaultParams)
472-
EmptyTree
473-
else if (receiver.isEmpty) {
474-
def findGetter(cx: Context): Tree =
475-
if (cx eq NoContext) EmptyTree
476-
else if (cx.scope != cx.outer.scope &&
477-
cx.denotNamed(meth.name).hasAltWith(_.symbol == meth)) {
478-
val denot = cx.denotNamed(getterName)
479-
if (denot.exists) ref(TermRef(cx.owner.thisType, getterName, denot))
480-
else findGetter(cx.outer)
481-
}
482-
else findGetter(cx.outer)
483-
findGetter(ctx)
484-
}
485-
else {
486-
def selectGetter(qual: Tree): Tree = {
487-
val getterDenot = qual.tpe.member(getterName)
488-
if (getterDenot.exists) qual.select(TermRef(qual.tpe, getterName, getterDenot))
489-
else EmptyTree
490-
}
491-
if (!meth.isClassConstructor)
492-
selectGetter(receiver)
493-
else {
494-
// default getters for class constructors are found in the companion object
495-
val cls = meth.owner
496-
val companion = cls.companionModule
497-
if (companion.isTerm) {
498-
val prefix = receiver.tpe.baseType(cls).normalizedPrefix
499-
if (prefix.exists) selectGetter(ref(TermRef(prefix, companion.asTerm)))
500-
else EmptyTree
501-
}
502-
else EmptyTree
503-
}
504-
}
505-
}
506-
507516
/** Is `sym` a constructor of a Java-defined annotation? */
508517
def isJavaAnnotConstr(sym: Symbol): Boolean =
509518
sym.is(JavaDefined) && sym.isConstructor && sym.owner.derivesFrom(defn.AnnotationClass)
@@ -554,18 +563,7 @@ trait Applications extends Compatibility {
554563
else
555564
EmptyTree
556565
}
557-
else {
558-
val getter =
559-
if (sym.exists) // `sym` doesn't exist for structural calls
560-
findDefaultGetter(n + numArgs(normalizedFun))
561-
else
562-
EmptyTree
563-
564-
if (!getter.isEmpty)
565-
spliceMeth(getter.withSpan(normalizedFun.span), normalizedFun)
566-
else
567-
EmptyTree
568-
}
566+
else defaultArgument(normalizedFun, n, this.isInstanceOf[TestApplication[?]])
569567

570568
def implicitArg = implicitArgTree(formal, appPos.span)
571569

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

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import TypeComparer.CompareResult
3030
import util.Spans._
3131
import util.common._
3232
import util.{Property, SimpleIdentityMap, SrcPos}
33-
import Applications.{ExtMethodApply, productSelectorTypes, wrapDefs}
33+
import Applications.{ExtMethodApply, productSelectorTypes, wrapDefs, defaultArgument}
3434

3535
import collection.mutable
3636
import annotation.tailrec
@@ -3081,34 +3081,56 @@ class Typer extends Namer
30813081

30823082
def addImplicitArgs(using Context) = {
30833083
def hasDefaultParams = methPart(tree).symbol.hasDefaultParams
3084-
def implicitArgs(formals: List[Type], argIndex: Int, pt: Type): List[Tree] = formals match {
3084+
def implicitArgs(formals: List[Type], argIndex: Int, pt: Type): List[Tree] = formals match
30853085
case Nil => Nil
30863086
case formal :: formals1 =>
3087+
// If the implicit parameter list is dependent we must propagate inferred
3088+
// types through the remainder of the parameter list similarly to how it's
3089+
// done for non-implicit parameter lists in Applications#matchArgs#addTyped.
3090+
def inferArgsAfter(leading: Tree) =
3091+
val formals2 =
3092+
if wtp.isParamDependent && leading.tpe.exists then
3093+
formals1.mapconserve(f1 => safeSubstParam(f1, wtp.paramRefs(argIndex), leading.tpe))
3094+
else formals1
3095+
implicitArgs(formals2, argIndex + 1, pt)
3096+
30873097
val arg = inferImplicitArg(formal, tree.span.endPos)
3088-
arg.tpe match {
3098+
arg.tpe match
30893099
case failed: AmbiguousImplicits =>
30903100
val pt1 = pt.deepenProto
30913101
if (pt1 `ne` pt) && (pt1 ne sharpenedPt)
30923102
&& constrainResult(tree.symbol, wtp, pt1)
30933103
then implicitArgs(formals, argIndex, pt1)
30943104
else arg :: implicitArgs(formals1, argIndex + 1, pt1)
3095-
case failed: SearchFailureType if !hasDefaultParams =>
3096-
// no need to search further, the adapt fails in any case
3097-
// the reason why we continue inferring arguments in case of an AmbiguousImplicits
3098-
// is that we need to know whether there are further errors.
3099-
// If there are none, we have to propagate the ambiguity to the caller.
3100-
arg :: formals1.map(dummyArg)
3105+
case failed: SearchFailureType =>
3106+
lazy val defaultArg =
3107+
def appPart(t: Tree): Tree = t match
3108+
case Block(stats, expr) => appPart(expr)
3109+
case Inlined(_, _, expr) => appPart(expr)
3110+
case _ => t
3111+
defaultArgument(appPart(tree), argIndex, testOnly = false)
3112+
.showing(i"default argument: for $formal, $tree, $argIndex = $result", typr)
3113+
if !hasDefaultParams || defaultArg.isEmpty then
3114+
// no need to search further, the adapt fails in any case
3115+
// the reason why we continue inferring arguments in case of an AmbiguousImplicits
3116+
// is that we need to know whether there are further errors.
3117+
// If there are none, we have to propagate the ambiguity to the caller.
3118+
arg :: formals1.map(dummyArg)
3119+
else
3120+
// This is tricky. On the one hand, we need the defaultArg to
3121+
// correctly type subsequent formal parameters in the same using
3122+
// clause in case there are parameter dependencies. On the other hand,
3123+
// we cannot simply use `defaultArg` as inferred argument since some parts
3124+
// of it might need lifting out. What we do instead is to use `defaultArg` for
3125+
// computing parameter-dependent formals but leave the original erroneous
3126+
// `arg` in place. We come back to this later in the code conditioned by
3127+
// `if propFail.exists` where we re-type the whole using clause with named
3128+
// arguments for all implicits that were found.
3129+
arg :: inferArgsAfter(defaultArg)
31013130
case _ =>
3102-
// If the implicit parameter list is dependent we must propagate inferred
3103-
// types through the remainder of the parameter list similarly to how it's
3104-
// done for non-implicit parameter lists in Applications#matchArgs#addTyped.
3105-
val formals2 =
3106-
if (wtp.isParamDependent && arg.tpe.exists)
3107-
formals1.mapconserve(f1 => safeSubstParam(f1, wtp.paramRefs(argIndex), arg.tpe))
3108-
else formals1
3109-
arg :: implicitArgs(formals2, argIndex + 1, pt)
3110-
}
3111-
}
3131+
arg :: inferArgsAfter(arg)
3132+
end implicitArgs
3133+
31123134
val args = implicitArgs(wtp.paramInfos, 0, pt)
31133135

31143136
def propagatedFailure(args: List[Tree]): Type = args match {

‎tests/pos/i5427.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,14 @@ object Test2 {
2727

2828
object Test3 {
2929
def fooInt: Foo[Int] { type Out = String } = ???
30-
implicit def str: String = ???
30+
implicit def istr: String = ???
31+
implicit def iint: Int = ???
3132

3233
def test5[A](implicit f1: Foo[A] = fooInt, f2: f1.Out) = f2
3334

3435
val t5 = test5
36+
// used to succeed with just one local implicit `istr`
37+
// but failed if a competing implicit `iint` was added.
3538
t5: String
3639
}
3740

0 commit comments

Comments
 (0)