From 6331820ed516aaff35c779756cce8f2b8e04d17e Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 25 Oct 2024 19:01:25 +0100 Subject: [PATCH] Fix regression in overload resolution picking eta-expanded method --- .../src/dotty/tools/dotc/core/Contexts.scala | 6 +-- .../src/dotty/tools/dotc/core/Types.scala | 11 +++++ .../dotty/tools/dotc/typer/Applications.scala | 43 +++++++++++-------- .../src/dotty/tools/dotc/typer/Typer.scala | 13 +----- tests/pos/i21727.min.scala | 21 +++++++++ tests/pos/i21727.scala | 18 ++++++++ 6 files changed, 79 insertions(+), 33 deletions(-) create mode 100644 tests/pos/i21727.min.scala create mode 100644 tests/pos/i21727.scala diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index d69c7408d0b1..6d0f0b6e5521 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -802,11 +802,11 @@ object Contexts { final def retractMode(mode: Mode): c.type = c.setMode(c.mode &~ mode) } - /** Run `op` with a pool-allocated context that has an ExporeTyperState. */ + /** Run `op` with a pool-allocated context that has an ExploreTyperState. */ inline def explore[T](inline op: Context ?=> T)(using Context): T = exploreInFreshCtx(op) - /** Run `op` with a pool-allocated FreshContext that has an ExporeTyperState. */ + /** Run `op` with a pool-allocated FreshContext that has an ExploreTyperState. */ inline def exploreInFreshCtx[T](inline op: FreshContext ?=> T)(using Context): T = val pool = ctx.base.exploreContextPool val nestedCtx = pool.next() @@ -931,7 +931,7 @@ object Contexts { FreshContext(ctx.base).init(ctx, ctx) private var inUse: Int = 0 - private var pool = new mutable.ArrayBuffer[FreshContext] + private val pool = new mutable.ArrayBuffer[FreshContext] def next()(using Context): FreshContext = val base = ctx.base diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 8a9d44cb8d25..2271a4a1c22a 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1630,6 +1630,17 @@ object Types extends TypeUtils { NoType } + /** Follow proxies and approximate type paramrefs by their upper bound + * in the current constraint in order to figure out robustly + * whether an expected type is some sort of function type. + */ + def underlyingApplied(using Context): Type = this.stripTypeVar match + case tp: RefinedType => tp + case tp: AppliedType => tp + case tp: TypeParamRef => TypeComparer.bounds(tp).hi.underlyingApplied + case tp: TypeProxy => tp.superType.underlyingApplied + case _ => this + /** The iterator of underlying types as long as type is a TypeProxy. * Useful for diagnostics */ diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 17be2acc7378..79a151ea6879 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1803,7 +1803,8 @@ trait Applications extends Compatibility { * an alternative that takes more implicit parameters wins over one * that takes fewer. */ - def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) { + def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false, needsEta: Boolean = false)(using Context): Int = + trace(i"compare($alt1, $alt2)", overload) { record("resolveOverloaded.compare") val scheme = val oldResolution = ctx.mode.is(Mode.OldImplicitResolution) @@ -1818,9 +1819,11 @@ trait Applications extends Compatibility { /** Is alternative `alt1` with type `tp1` as good as alternative * `alt2` with type `tp2` ? * - * 1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as good as `alt2` - * if `alt1` is nullary or `alt2` is applicable to arguments (p1, ..., pn) of - * types T1,...,Tn. If the last parameter `pn` has a vararg type T*, then + * 1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as good as `alt2` if: + * a. `alt1` is nullary, or + * b. `alt2` is applicable to arguments (p1, ..., pn) of types T1,...,Tn, or + * c. eta-expanded `alt1` is as good as `alt2` (when needing eta-expansion). + * When testing method applicability (1b), if the last parameter `pn` has a vararg type T*, then * `alt1` must be applicable to arbitrary numbers of `T` parameters (which * implies that it must be a varargs method as well). * 2. A polymorphic member of type [a1 >: L1 <: U1, ..., an >: Ln <: Un]T is as @@ -1828,20 +1831,22 @@ trait Applications extends Compatibility { * assumption that for i = 1,...,n each ai is an abstract type name bounded * from below by Li and from above by Ui. * 3. A member of any other type `tp1` is: - * a. always as good as a method or a polymorphic method. - * b. as good as a member of any other type `tp2` if `asGoodValueType(tp1, tp2) = true` + * a. as good as an eta-expanded `tp2` method (when needing eta-expansion) + * b. always as good as a (not eta-expanded) method or a polymorphic method. + * c. as good as a member of any other type `tp2` if `asGoodValueType(tp1, tp2) = true` */ def isAsGood(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsGood $tp1 $tp2", overload) { tp1 match case tp1: MethodType => // (1) - tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType] + tp1.paramInfos.isEmpty && tp2.isInstanceOf[MethodOrPoly] // (1a) || { if tp1.isVarArgsMethod then tp2.isVarArgsMethod - && isApplicableMethodRef(alt2, tp1.paramInfos.map(_.repeatedToSingle), WildcardType, ArgMatch.Compatible) + && isApplicableMethodRef(alt2, tp1.paramInfos.map(_.repeatedToSingle), WildcardType, ArgMatch.Compatible) // (1b) else - isApplicableMethodRef(alt2, tp1.paramInfos, WildcardType, ArgMatch.Compatible) + isApplicableMethodRef(alt2, tp1.paramInfos, WildcardType, ArgMatch.Compatible) // (1b) } + || needsEta && !tp2.isInstanceOf[MethodOrPoly] && isAsGood(alt1, tp1.toFunctionType(), alt2, tp2) // (1c) case tp1: PolyType => // (2) inContext(ctx.fresh.setExploreTyperState()) { // Fully define the PolyType parameters so that the infos of the @@ -1859,11 +1864,12 @@ trait Applications extends Compatibility { def compareValues(tp2: Type)(using Context) = isAsGoodValueType(tp1, tp2, alt1.symbol.is(Implicit)) tp2 match - case tp2: MethodType => true // (3a) - case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a) - case tp2: PolyType => // (3b) + case tp2: MethodType if needsEta => isAsGood(alt1, tp1, alt2, tp2.toFunctionType()) // (3a) + case tp2: MethodType => true // (3b) + case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3b) + case tp2: PolyType => // (3c) explore(compareValues(instantiateWithTypeVars(tp2))) - case _ => // 3b) + case _ => // (3c) compareValues(tp2) } @@ -2037,13 +2043,13 @@ trait Applications extends Compatibility { } end compare - def narrowMostSpecific(alts: List[TermRef])(using Context): List[TermRef] = { + def narrowMostSpecific(alts: List[TermRef], needsEta: Boolean)(using Context): List[TermRef] = { record("narrowMostSpecific") alts match { case Nil => alts case _ :: Nil => alts case alt1 :: alt2 :: Nil => - compare(alt1, alt2) match { + compare(alt1, alt2, needsEta = needsEta) match { case 1 => alt1 :: Nil case -1 => alt2 :: Nil case 0 => alts @@ -2051,7 +2057,7 @@ trait Applications extends Compatibility { case alt :: alts1 => def survivors(previous: List[TermRef], alts: List[TermRef]): List[TermRef] = alts match { case alt :: alts1 => - compare(previous.head, alt) match { + compare(previous.head, alt, needsEta = needsEta) match { case 1 => survivors(previous, alts1) case -1 => survivors(alt :: previous.tail, alts1) case 0 => survivors(alt :: previous, alts1) @@ -2061,7 +2067,7 @@ trait Applications extends Compatibility { val best :: rest = survivors(alt :: Nil, alts1): @unchecked def asGood(alts: List[TermRef]): List[TermRef] = alts match { case alt :: alts1 => - if (compare(alt, best) < 0) asGood(alts1) else alt :: asGood(alts1) + if (compare(alt, best, needsEta = needsEta) < 0) asGood(alts1) else alt :: asGood(alts1) case nil => Nil } @@ -2374,7 +2380,8 @@ trait Applications extends Compatibility { // If `pt` is erroneous, don't try to go further; report the error in `pt` instead. candidates else - val found = narrowMostSpecific(candidates) + val needsEta = defn.isFunctionNType(pt.underlyingApplied) + val found = narrowMostSpecific(candidates, needsEta) if found.length <= 1 then found else val deepPt = pt.deepenProto diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3810bc66841e..b12293488f0e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -4471,17 +4471,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer tree } - // Follow proxies and approximate type paramrefs by their upper bound - // in the current constraint in order to figure out robustly - // whether an expected type is some sort of function type. - def underlyingApplied(tp: Type): Type = tp.stripTypeVar match { - case tp: RefinedType => tp - case tp: AppliedType => tp - case tp: TypeParamRef => underlyingApplied(TypeComparer.bounds(tp).hi) - case tp: TypeProxy => underlyingApplied(tp.superType) - case _ => tp - } - // If the expected type is a selection of an extension method, deepen it // to also propagate the argument type (which is the receiver we have // typechecked already). This is needed for i8311.scala. Doing so @@ -4494,7 +4483,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case _ => pt def adaptNoArgs(wtp: Type): Tree = { - val ptNorm = underlyingApplied(pt) + val ptNorm = pt.underlyingApplied def functionExpected = defn.isFunctionNType(ptNorm) def needsEta = pt.revealIgnored match case _: SingletonType | _: FunOrPolyProto => false diff --git a/tests/pos/i21727.min.scala b/tests/pos/i21727.min.scala new file mode 100644 index 000000000000..0bc830f28cfa --- /dev/null +++ b/tests/pos/i21727.min.scala @@ -0,0 +1,21 @@ +import scala.language.implicitConversions + +type UUID = String +object MyId: + def fromUUID[F[_]: Functor: UUIDGen]: F[String] = + toFunctorOps(UUIDGen[F].randomUUID).map(fromUUID) // error + private def fromUUID(id: UUID): String = ??? + +object UUIDGen: + def apply[F[_]](implicit ev: UUIDGen[F]): UUIDGen[F] = ev +trait UUIDGen[F[_]]: + def randomUUID: F[UUID] + +trait Functor[F[_]] +implicit def toFunctorOps[F[_], A](target: F[A])(implicit tc: Functor[F]): Ops[F, A] { type TypeClassType = Functor[F]} = + new Ops[F, A] { type TypeClassType = Functor[F] } + +trait Ops[F[_], A] { + type TypeClassType <: Functor[F] + def map[B](f: A => B): F[B] = ??? +} diff --git a/tests/pos/i21727.scala b/tests/pos/i21727.scala new file mode 100644 index 000000000000..1f028cd17fc9 --- /dev/null +++ b/tests/pos/i21727.scala @@ -0,0 +1,18 @@ +trait ExMap1[K1, +V1] extends PartialFunction[K1, V1] +trait ExMap2[K2, +V2] extends PartialFunction[K2, V2] + +trait Gen[L[_]] { def make: L[Unit] } + +trait Functor[M[_]]: + def map[A, B](ma: M[A])(f: A => B): M[B] +object Functor: + implicit def inst1[K3]: Functor[[V3] =>> ExMap1[K3, V3]] = ??? + implicit def inst2[K4]: Functor[[V4] =>> ExMap2[K4, V4]] = ??? + +class Test: + def foo(x: Unit): String = x.toString() + def foo[F[_]](using F: Functor[F], G: Gen[F]): F[String] = + val res1: F[String] = F.map[Unit, String](G.make)(foo) // was: error + val res2: F[String] = F.map[Unit, String](G.make)(foo(_)) // was: ok + + ??? : F[String]