Skip to content

Commit 626b24a

Browse files
authored
Merge pull request #10985 from dotty-staging/fix-#9344
Early return from resolveOverloaded in case arguments are erroneous
2 parents a5ff84a + 1ecc48b commit 626b24a

File tree

9 files changed

+230
-155
lines changed

9 files changed

+230
-155
lines changed

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,10 +333,28 @@ object Types {
333333
/** Is this type produced as a repair for an error? */
334334
final def isError(using Context): Boolean = stripTypeVar.isInstanceOf[ErrorType]
335335

336-
/** Is some part of the widened version of this type produced as a repair for an error? */
336+
/** Is some part of the widened version of this type produced as a repair for an error?
337+
*
338+
*/
337339
def isErroneous(using Context): Boolean =
338340
widen.existsPart(_.isError, forceLazy = false)
339341

342+
/** Is this type unusable for implicit search or overloading resolution
343+
* since it has embedded errors that can match anything? This is weaker and more
344+
* ad-hoc than isErroneous. The main differences are that we always consider aliases
345+
* (since these are relevant for inference or resolution) but never consider prefixes
346+
* (since these often do not constrain the search space anyway).
347+
*/
348+
def unusableForInference(using Context): Boolean = widenDealias match
349+
case AppliedType(tycon, args) => tycon.unusableForInference || args.exists(_.unusableForInference)
350+
case RefinedType(parent, _, rinfo) => parent.unusableForInference || rinfo.unusableForInference
351+
case TypeBounds(lo, hi) => lo.unusableForInference || hi.unusableForInference
352+
case tp: AndOrType => tp.tp1.unusableForInference || tp.tp2.unusableForInference
353+
case tp: LambdaType => tp.resultType.unusableForInference || tp.paramInfos.exists(_.unusableForInference)
354+
case WildcardType(optBounds) => optBounds.unusableForInference
355+
case _: ErrorType => true
356+
case _ => false
357+
340358
/** Does the type carry an annotation that is an instance of `cls`? */
341359
@tailrec final def hasAnnotation(cls: ClassSymbol)(using Context): Boolean = stripTypeVar match {
342360
case AnnotatedType(tp, annot) => (annot matches cls) || (tp hasAnnotation cls)

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

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

217316
trait Applications extends Compatibility {
@@ -354,7 +453,9 @@ trait Applications extends Compatibility {
354453
def success: Boolean = ok
355454

356455
protected def methodType: MethodType = methType.asInstanceOf[MethodType]
357-
private def methString: String = i"${err.refStr(methRef)}: ${methType.show}"
456+
private def methString: String =
457+
def infoStr = if methType.isErroneous then "" else i": $methType"
458+
i"${err.refStr(methRef)}$infoStr"
358459

359460
/** Re-order arguments to correctly align named arguments */
360461
def reorder[T >: Untyped](args: List[Trees.Tree[T]]): List[Trees.Tree[T]] = {
@@ -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

@@ -1927,35 +1925,40 @@ trait Applications extends Compatibility {
19271925
case _ => false
19281926

19291927
record("resolveOverloaded.narrowedApplicable", candidates.length)
1930-
val found = narrowMostSpecific(candidates)
1931-
if (found.length <= 1) found
1928+
if pt.unusableForInference then
1929+
// `pt` might have become erroneous by typing arguments of FunProtos.
1930+
// If `pt` is erroneous, don't try to go further; report the error in `pt` instead.
1931+
candidates
19321932
else
1933-
val deepPt = pt.deepenProto
1934-
deepPt match
1935-
case pt @ FunProto(_, resType: FunOrPolyProto) =>
1936-
// try to narrow further with snd argument list
1937-
resolveMapped(candidates, skipParamClause(pt.typedArgs().tpes), resType)
1938-
case _ =>
1939-
// prefer alternatives that need no eta expansion
1940-
val noCurried = alts.filter(!resultIsMethod(_))
1941-
val noCurriedCount = noCurried.length
1942-
if noCurriedCount == 1 then
1943-
noCurried
1944-
else if noCurriedCount > 1 && noCurriedCount < alts.length then
1945-
resolveOverloaded1(noCurried, pt)
1946-
else
1947-
// prefer alternatves that match without default parameters
1948-
val noDefaults = alts.filter(!_.symbol.hasDefaultParams)
1949-
val noDefaultsCount = noDefaults.length
1950-
if noDefaultsCount == 1 then
1951-
noDefaults
1952-
else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
1953-
resolveOverloaded1(noDefaults, pt)
1954-
else if deepPt ne pt then
1955-
// try again with a deeper known expected type
1956-
resolveOverloaded1(alts, deepPt)
1933+
val found = narrowMostSpecific(candidates)
1934+
if found.length <= 1 then found
1935+
else
1936+
val deepPt = pt.deepenProto
1937+
deepPt match
1938+
case pt @ FunProto(_, resType: FunOrPolyProto) =>
1939+
// try to narrow further with snd argument list
1940+
resolveMapped(candidates, skipParamClause(pt.typedArgs().tpes), resType)
1941+
case _ =>
1942+
// prefer alternatives that need no eta expansion
1943+
val noCurried = alts.filter(!resultIsMethod(_))
1944+
val noCurriedCount = noCurried.length
1945+
if noCurriedCount == 1 then
1946+
noCurried
1947+
else if noCurriedCount > 1 && noCurriedCount < alts.length then
1948+
resolveOverloaded1(noCurried, pt)
19571949
else
1958-
candidates
1950+
// prefer alternatves that match without default parameters
1951+
val noDefaults = alts.filter(!_.symbol.hasDefaultParams)
1952+
val noDefaultsCount = noDefaults.length
1953+
if noDefaultsCount == 1 then
1954+
noDefaults
1955+
else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
1956+
resolveOverloaded1(noDefaults, pt)
1957+
else if deepPt ne pt then
1958+
// try again with a deeper known expected type
1959+
resolveOverloaded1(alts, deepPt)
1960+
else
1961+
candidates
19591962
}
19601963
end resolveOverloaded1
19611964

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,11 @@ trait Implicits:
954954
assert(ctx.phase.allowsImplicitSearch,
955955
if (argument.isEmpty) i"missing implicit parameter of type $pt after typer"
956956
else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}")
957+
958+
if pt.unusableForInference
959+
|| !argument.isEmpty && argument.tpe.unusableForInference
960+
then return NoMatchingImplicitsFailure
961+
957962
val result0 =
958963
try ImplicitSearch(pt, argument, span).bestImplicit
959964
catch case ce: CyclicReference =>

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ object ProtoTypes {
193193
if ((name eq this.name) && (memberProto eq this.memberProto) && (compat eq this.compat)) this
194194
else SelectionProto(name, memberProto, compat, privateOK)
195195

196+
override def isErroneous(using Context): Boolean =
197+
memberProto.isErroneous
198+
199+
override def unusableForInference(using Context): Boolean =
200+
memberProto.unusableForInference
201+
196202
def map(tm: TypeMap)(using Context): SelectionProto = derivedSelectionProto(name, tm(memberProto), compat)
197203
def fold[T](x: T, ta: TypeAccumulator[T])(using Context): T = ta(x, memberProto)
198204

@@ -403,6 +409,9 @@ object ProtoTypes {
403409
override def isErroneous(using Context): Boolean =
404410
state.typedArgs.tpes.exists(_.isErroneous)
405411

412+
override def unusableForInference(using Context): Boolean =
413+
state.typedArgs.exists(_.tpe.unusableForInference)
414+
406415
override def toString: String = s"FunProto(${args mkString ","} => $resultType)"
407416

408417
def map(tm: TypeMap)(using Context): FunProto =
@@ -453,6 +462,12 @@ object ProtoTypes {
453462
if ((argType eq this.argType) && (resultType eq this.resultType)) this
454463
else ViewProto(argType, resultType)
455464

465+
override def isErroneous(using Context): Boolean =
466+
argType.isErroneous || resType.isErroneous
467+
468+
override def unusableForInference(using Context): Boolean =
469+
argType.unusableForInference || resType.unusableForInference
470+
456471
def map(tm: TypeMap)(using Context): ViewProto = derivedViewProto(tm(argType), tm(resultType))
457472

458473
def fold[T](x: T, ta: TypeAccumulator[T])(using Context): T =
@@ -496,6 +511,12 @@ object ProtoTypes {
496511
if ((targs eq this.targs) && (resType eq this.resType)) this
497512
else PolyProto(targs, resType)
498513

514+
override def isErroneous(using Context): Boolean =
515+
targs.exists(_.tpe.isErroneous)
516+
517+
override def unusableForInference(using Context): Boolean =
518+
targs.exists(_.tpe.unusableForInference)
519+
499520
def map(tm: TypeMap)(using Context): PolyProto =
500521
derivedPolyProto(targs, tm(resultType))
501522

0 commit comments

Comments
 (0)