Skip to content

Early return from resolveOverloaded in case arguments are erroneous #10985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 13, 2021
Merged

Early return from resolveOverloaded in case arguments are erroneous #10985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
51b496a
Early return from resolveOverloaded in case arguments are erroneous
odersky Jan 4, 2021
792da41
Update check file
odersky Jan 5, 2021
22c6b60
Use new error detector: hasErrors
odersky Jan 5, 2021
a3798a2
Fix handling of default arguments in parameter-dependent using clauses
odersky Jan 5, 2021
ab70523
Abort implicit search for types with errors
odersky Jan 5, 2021
084d13b
Clean up terminology
odersky Jan 6, 2021
1ecc48b
Fix rebase breakage
odersky Jan 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,28 @@ object Types {
/** Is this type produced as a repair for an error? */
final def isError(using Context): Boolean = stripTypeVar.isInstanceOf[ErrorType]

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

/** Is this type unusable for implicit search or overloading resolution
* since it has embedded errors that can match anything? This is weaker and more
* ad-hoc than isErroneous. The main differences are that we always consider aliases
* (since these are relevant for inference or resolution) but never consider prefixes
* (since these often do not constrain the search space anyway).
*/
def unusableForInference(using Context): Boolean = widenDealias match
case AppliedType(tycon, args) => tycon.unusableForInference || args.exists(_.unusableForInference)
case RefinedType(parent, _, rinfo) => parent.unusableForInference || rinfo.unusableForInference
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a case for RecType?

case TypeBounds(lo, hi) => lo.unusableForInference || hi.unusableForInference
case tp: AndOrType => tp.tp1.unusableForInference || tp.tp2.unusableForInference
case tp: LambdaType => tp.resultType.unusableForInference || tp.paramInfos.exists(_.unusableForInference)
case WildcardType(optBounds) => optBounds.unusableForInference
case _: ErrorType => true
case _ => false

/** Does the type carry an annotation that is an instance of `cls`? */
@tailrec final def hasAnnotation(cls: ClassSymbol)(using Context): Boolean = stripTypeVar match {
case AnnotatedType(tp, annot) => (annot matches cls) || (tp hasAnnotation cls)
Expand Down
267 changes: 135 additions & 132 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,105 @@ object Applications {
overwriteType(app.tpe)
// ExtMethodApply always has wildcard type in order not to prompt any further adaptations
// such as eta expansion before the method is fully applied.

/** Find reference to default parameter getter for parameter #n in current
* parameter list, or NoType if none was found
*/
def findDefaultGetter(fn: Tree, n: Int, testOnly: Boolean)(using Context): Tree =
if fn.symbol.isTerm then
val meth = fn.symbol.asTerm
val receiver: Tree = methPart(fn) match {
case Select(receiver, _) => receiver
case mr => mr.tpe.normalizedPrefix match {
case mr: TermRef => ref(mr)
case mr =>
if testOnly then
// In this case it is safe to skolemize now; we will produce a stable prefix for the actual call.
ref(mr.narrow)
else
EmptyTree
}
}
val getterPrefix =
if (meth.is(Synthetic) && meth.name == nme.apply) nme.CONSTRUCTOR else meth.name
def getterName = DefaultGetterName(getterPrefix, n + numArgs(fn))
if !meth.hasDefaultParams then
EmptyTree
else if (receiver.isEmpty) {
def findGetter(cx: Context): Tree =
if (cx eq NoContext) EmptyTree
else if (cx.scope != cx.outer.scope &&
cx.denotNamed(meth.name).hasAltWith(_.symbol == meth)) {
val denot = cx.denotNamed(getterName)
if (denot.exists) ref(TermRef(cx.owner.thisType, getterName, denot))
else findGetter(cx.outer)
}
else findGetter(cx.outer)
findGetter(ctx)
}
else {
def selectGetter(qual: Tree): Tree = {
val getterDenot = qual.tpe.member(getterName)
if (getterDenot.exists) qual.select(TermRef(qual.tpe, getterName, getterDenot))
else EmptyTree
}
if (!meth.isClassConstructor)
selectGetter(receiver)
else {
// default getters for class constructors are found in the companion object
val cls = meth.owner
val companion = cls.companionModule
if (companion.isTerm) {
val prefix = receiver.tpe.baseType(cls).normalizedPrefix
if (prefix.exists) selectGetter(ref(TermRef(prefix, companion.asTerm)))
else EmptyTree
}
else EmptyTree
}
}
else EmptyTree // structural applies don't have symbols or defaults
end findDefaultGetter

/** Splice new method reference `meth` into existing application `app` */
private def spliceMeth(meth: Tree, app: Tree)(using Context): Tree = app match {
case Apply(fn, args) =>
spliceMeth(meth, fn).appliedToArgs(args)
case TypeApply(fn, targs) =>
// Note: It is important that the type arguments `targs` are passed in new trees
// instead of being spliced in literally. Otherwise, a type argument to a default
// method could be constructed as the definition site of the type variable for
// that default constructor. This would interpolate type variables too early,
// causing lots of tests (among them tasty_unpickleScala2) to fail.
//
// The test case is in i1757.scala. Here we have a variable `s` and a method `cpy`
// defined like this:
//
// var s
// def cpy[X](b: List[Int] = b): B[X] = new B[X](b)
//
// The call `s.cpy()` then gets expanded to
//
// { val $1$: B[Int] = this.s
// $1$.cpy[X']($1$.cpy$default$1[X']
// }
//
// A type variable gets interpolated if it does not appear in the type
// of the current tree and the current tree contains the variable's "definition".
// Previously, the polymorphic function tree to which the variable was first added
// was taken as the variable's definition. But that fails here because that
// tree was `s.cpy` but got transformed into `$1$.cpy`. We now take the type argument
// [X'] of the variable as its definition tree, which is more robust. But then
// it's crucial that the type tree is not copied directly as argument to
// `cpy$default$1`. If it was, the variable `X'` would already be interpolated
// when typing the default argument, which is too early.
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
case _ => meth
}

def defaultArgument(fn: Tree, n: Int, testOnly: Boolean)(using Context): Tree =
val getter = findDefaultGetter(fn, n, testOnly)
if getter.isEmpty then getter
else spliceMeth(getter.withSpan(fn.span), fn)
}

trait Applications extends Compatibility {
Expand Down Expand Up @@ -354,7 +453,9 @@ trait Applications extends Compatibility {
def success: Boolean = ok

protected def methodType: MethodType = methType.asInstanceOf[MethodType]
private def methString: String = i"${err.refStr(methRef)}: ${methType.show}"
private def methString: String =
def infoStr = if methType.isErroneous then "" else i": $methType"
i"${err.refStr(methRef)}$infoStr"

/** Re-order arguments to correctly align named arguments */
def reorder[T >: Untyped](args: List[Trees.Tree[T]]): List[Trees.Tree[T]] = {
Expand Down Expand Up @@ -412,98 +513,6 @@ trait Applications extends Compatibility {
handlePositional(methodType.paramNames, args)
}

/** Splice new method reference into existing application */
def spliceMeth(meth: Tree, app: Tree): Tree = app match {
case Apply(fn, args) =>
spliceMeth(meth, fn).appliedToTermArgs(args)
case TypeApply(fn, targs) =>
// Note: It is important that the type arguments `targs` are passed in new trees
// instead of being spliced in literally. Otherwise, a type argument to a default
// method could be constructed as the definition site of the type variable for
// that default constructor. This would interpolate type variables too early,
// causing lots of tests (among them tasty_unpickleScala2) to fail.
//
// The test case is in i1757.scala. Here we have a variable `s` and a method `cpy`
// defined like this:
//
// var s
// def cpy[X](b: List[Int] = b): B[X] = new B[X](b)
//
// The call `s.cpy()` then gets expanded to
//
// { val $1$: B[Int] = this.s
// $1$.cpy[X']($1$.cpy$default$1[X']
// }
//
// A type variable gets interpolated if it does not appear in the type
// of the current tree and the current tree contains the variable's "definition".
// Previously, the polymorphic function tree to which the variable was first added
// was taken as the variable's definition. But that fails here because that
// tree was `s.cpy` but got transformed into `$1$.cpy`. We now take the type argument
// [X'] of the variable as its definition tree, which is more robust. But then
// it's crucial that the type tree is not copied directly as argument to
// `cpy$default$1`. If it was, the variable `X'` would already be interpolated
// when typing the default argument, which is too early.
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
case _ => meth
}

/** Find reference to default parameter getter for parameter #n in current
* parameter list, or NoType if none was found
*/
def findDefaultGetter(n: Int)(using Context): Tree = {
val meth = methRef.symbol.asTerm
val receiver: Tree = methPart(normalizedFun) match {
case Select(receiver, _) => receiver
case mr => mr.tpe.normalizedPrefix match {
case mr: TermRef => ref(mr)
case mr =>
if (this.isInstanceOf[TestApplication[?]])
// In this case it is safe to skolemize now; we will produce a stable prefix for the actual call.
ref(mr.narrow)
else
EmptyTree
}
}
val getterPrefix =
if (meth.is(Synthetic) && meth.name == nme.apply) nme.CONSTRUCTOR else meth.name
def getterName = DefaultGetterName(getterPrefix, n)
if (!meth.hasDefaultParams)
EmptyTree
else if (receiver.isEmpty) {
def findGetter(cx: Context): Tree =
if (cx eq NoContext) EmptyTree
else if (cx.scope != cx.outer.scope &&
cx.denotNamed(meth.name).hasAltWith(_.symbol == meth)) {
val denot = cx.denotNamed(getterName)
if (denot.exists) ref(TermRef(cx.owner.thisType, getterName, denot))
else findGetter(cx.outer)
}
else findGetter(cx.outer)
findGetter(ctx)
}
else {
def selectGetter(qual: Tree): Tree = {
val getterDenot = qual.tpe.member(getterName)
if (getterDenot.exists) qual.select(TermRef(qual.tpe, getterName, getterDenot))
else EmptyTree
}
if (!meth.isClassConstructor)
selectGetter(receiver)
else {
// default getters for class constructors are found in the companion object
val cls = meth.owner
val companion = cls.companionModule
if (companion.isTerm) {
val prefix = receiver.tpe.baseType(cls).normalizedPrefix
if (prefix.exists) selectGetter(ref(TermRef(prefix, companion.asTerm)))
else EmptyTree
}
else EmptyTree
}
}
}

/** Is `sym` a constructor of a Java-defined annotation? */
def isJavaAnnotConstr(sym: Symbol): Boolean =
sym.is(JavaDefined) && sym.isConstructor && sym.owner.derivesFrom(defn.AnnotationClass)
Expand Down Expand Up @@ -554,18 +563,7 @@ trait Applications extends Compatibility {
else
EmptyTree
}
else {
val getter =
if (sym.exists) // `sym` doesn't exist for structural calls
findDefaultGetter(n + numArgs(normalizedFun))
else
EmptyTree

if (!getter.isEmpty)
spliceMeth(getter.withSpan(normalizedFun.span), normalizedFun)
else
EmptyTree
}
else defaultArgument(normalizedFun, n, this.isInstanceOf[TestApplication[?]])

def implicitArg = implicitArgTree(formal, appPos.span)

Expand Down Expand Up @@ -1927,35 +1925,40 @@ trait Applications extends Compatibility {
case _ => false

record("resolveOverloaded.narrowedApplicable", candidates.length)
val found = narrowMostSpecific(candidates)
if (found.length <= 1) found
if pt.unusableForInference then
// `pt` might have become erroneous by typing arguments of FunProtos.
// If `pt` is erroneous, don't try to go further; report the error in `pt` instead.
candidates
else
val deepPt = pt.deepenProto
deepPt match
case pt @ FunProto(_, resType: FunOrPolyProto) =>
// try to narrow further with snd argument list
resolveMapped(candidates, skipParamClause(pt.typedArgs().tpes), resType)
case _ =>
// prefer alternatives that need no eta expansion
val noCurried = alts.filter(!resultIsMethod(_))
val noCurriedCount = noCurried.length
if noCurriedCount == 1 then
noCurried
else if noCurriedCount > 1 && noCurriedCount < alts.length then
resolveOverloaded1(noCurried, pt)
else
// prefer alternatves that match without default parameters
val noDefaults = alts.filter(!_.symbol.hasDefaultParams)
val noDefaultsCount = noDefaults.length
if noDefaultsCount == 1 then
noDefaults
else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
resolveOverloaded1(noDefaults, pt)
else if deepPt ne pt then
// try again with a deeper known expected type
resolveOverloaded1(alts, deepPt)
val found = narrowMostSpecific(candidates)
if found.length <= 1 then found
else
val deepPt = pt.deepenProto
deepPt match
case pt @ FunProto(_, resType: FunOrPolyProto) =>
// try to narrow further with snd argument list
resolveMapped(candidates, skipParamClause(pt.typedArgs().tpes), resType)
case _ =>
// prefer alternatives that need no eta expansion
val noCurried = alts.filter(!resultIsMethod(_))
val noCurriedCount = noCurried.length
if noCurriedCount == 1 then
noCurried
else if noCurriedCount > 1 && noCurriedCount < alts.length then
resolveOverloaded1(noCurried, pt)
else
candidates
// prefer alternatves that match without default parameters
val noDefaults = alts.filter(!_.symbol.hasDefaultParams)
val noDefaultsCount = noDefaults.length
if noDefaultsCount == 1 then
noDefaults
else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
resolveOverloaded1(noDefaults, pt)
else if deepPt ne pt then
// try again with a deeper known expected type
resolveOverloaded1(alts, deepPt)
else
candidates
}
end resolveOverloaded1

Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,11 @@ trait Implicits:
assert(ctx.phase.allowsImplicitSearch,
if (argument.isEmpty) i"missing implicit parameter of type $pt after typer"
else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}")

if pt.unusableForInference
|| !argument.isEmpty && argument.tpe.unusableForInference
then return NoMatchingImplicitsFailure

val result0 =
try ImplicitSearch(pt, argument, span).bestImplicit
catch case ce: CyclicReference =>
Expand Down
21 changes: 21 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ object ProtoTypes {
if ((name eq this.name) && (memberProto eq this.memberProto) && (compat eq this.compat)) this
else SelectionProto(name, memberProto, compat, privateOK)

override def isErroneous(using Context): Boolean =
memberProto.isErroneous

override def unusableForInference(using Context): Boolean =
memberProto.unusableForInference

def map(tm: TypeMap)(using Context): SelectionProto = derivedSelectionProto(name, tm(memberProto), compat)
def fold[T](x: T, ta: TypeAccumulator[T])(using Context): T = ta(x, memberProto)

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

override def unusableForInference(using Context): Boolean =
state.typedArgs.exists(_.tpe.unusableForInference)

override def toString: String = s"FunProto(${args mkString ","} => $resultType)"

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

override def isErroneous(using Context): Boolean =
argType.isErroneous || resType.isErroneous

override def unusableForInference(using Context): Boolean =
argType.unusableForInference || resType.unusableForInference

def map(tm: TypeMap)(using Context): ViewProto = derivedViewProto(tm(argType), tm(resultType))

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

override def isErroneous(using Context): Boolean =
targs.exists(_.tpe.isErroneous)

override def unusableForInference(using Context): Boolean =
targs.exists(_.tpe.unusableForInference)

def map(tm: TypeMap)(using Context): PolyProto =
derivedPolyProto(targs, tm(resultType))

Expand Down
Loading