From 205272ce6b47e890a6f19639847bd1510a130871 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 3 Apr 2024 20:39:07 +0200 Subject: [PATCH 1/4] Refine overloading and implicit disambiguation We sometimes have two alternatives a.m and b.m with the same symbol but different prefixes. Previously these would always be ambiguous. We now try to disambiguate this so that the alternative with the more specific prefix wins. To determine this, we widen prefixes also going from module classes to their parents and then compare the resulting types. This might fix a problem in ScalaTest that popped up after #20054. --- .../dotty/tools/dotc/typer/Applications.scala | 32 ++++++++++++++++++- .../pos/implicit-prefix-disambiguation.scala | 14 ++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/pos/implicit-prefix-disambiguation.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 82f4c89ae203..cb119b92431b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1807,8 +1807,38 @@ trait Applications extends Compatibility { else tp } + def widenPrefix(alt: TermRef): Type = alt.prefix.widen match + case pre: (TypeRef | ThisType) if pre.typeSymbol.is(Module) => + pre.parents.reduceLeft(TypeComparer.andType(_, _)) + case wpre => wpre + + /** If two alternatives have the same symbol, we pick the one with the most + * specific prefix. To determine that, we widen the prefix types and also + * widen module classes to the intersection of their parent classes. Then + * if one of the resulting types is a more specific value type than the other, + * it wins. Example: + * + * trait A { given M = ... } + * trait B extends A + * object a extends A + * object b extends B + * + * In this case `b.M` would be regarded as more specific than `a.M`. + */ + def comparePrefixes(pre1: Type, pre2: Type) = + val winsPrefix1 = isAsSpecificValueType(pre1, pre2) + val winsPrefix2 = isAsSpecificValueType(pre2, pre1) + if winsPrefix1 == winsPrefix2 then 0 + else if winsPrefix1 then 1 + else -1 + def compareWithTypes(tp1: Type, tp2: Type) = { - val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) + val ownerScore = + val sym1 = alt1.symbol + val sym2 = alt2.symbol + if sym1 == sym2 then comparePrefixes(widenPrefix(alt1), widenPrefix(alt2)) + else compareOwner(sym1.maybeOwner, sym2.maybeOwner) + def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) diff --git a/tests/pos/implicit-prefix-disambiguation.scala b/tests/pos/implicit-prefix-disambiguation.scala new file mode 100644 index 000000000000..5059aa2db4eb --- /dev/null +++ b/tests/pos/implicit-prefix-disambiguation.scala @@ -0,0 +1,14 @@ +class I[X] + +trait A: + given I[B] = ??? +object A extends A + +trait B extends A +object B extends B + +//import B.given, A.given + +def Test = summon[I[B]] + + From 2e640c2b8624f99468cda2a69a30a008aa5c1ac4 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 3 Apr 2024 20:45:43 +0200 Subject: [PATCH 2/4] Make compareOwner symmetric compareOwner did certain tests for one side but not for the other, which made its outcome dependent on the order in which alternatives were presented. --- .../dotty/tools/dotc/typer/Applications.scala | 32 +++++++++++-------- .../dotty/tools/dotc/util/Signatures.scala | 4 +-- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index cb119b92431b..ceef89f8bfff 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1614,11 +1614,12 @@ trait Applications extends Compatibility { * Module classes also inherit the relationship from their companions. This means, * if no direct derivation exists between `sym1` and `sym2` also perform the following * tests: - * - If both sym1 and sym1 are module classes that have companion classes, - * and sym2 does not inherit implicit members from a base class (#), - * compare the companion classes. - * - If sym1 is a module class with a companion, and sym2 is a normal class or trait, - * compare the companion with sym2. + * - If both sym1 and sym2 are module classes that have companion classes, + * compare the companion classes. Return the result of that comparison, + * provided the module class with the larger companion class does not itself + * inherit implicit members from a base class (#), + * - If one sym is a module class with a companion, and the other is a normal class or trait, + * compare the companion with the other class or trait. * * Condition (#) is necessary to make `compareOwner(_, _) > 0` a transitive relation. * For instance: @@ -1642,17 +1643,22 @@ trait Applications extends Compatibility { * This means we get an ambiguity between `a` and `b` in all cases. */ def compareOwner(sym1: Symbol, sym2: Symbol)(using Context): Int = + def cls1 = sym1.companionClass + def cls2 = sym2.companionClass if sym1 == sym2 then 0 else if sym1.isSubClass(sym2) then 1 else if sym2.isSubClass(sym1) then -1 - else if sym1.is(Module) then - val cls1 = sym1.companionClass - if sym2.is(Module) then - if sym2.thisType.implicitMembers.forall(_.symbol.owner == sym2) then // test for (#) - compareOwner(cls1, sym2.companionClass) - else 0 - else compareOwner(cls1, sym2) - else 0 + else + if sym1.is(Module) && sym2.is(Module) then + val r = compareOwner(cls1, cls2) + if r == 0 then 0 + else + val larger = if r < 0 then sym1 else sym2 + if larger.thisType.implicitMembers.forall(_.symbol.owner == larger) then r + else 0 + else if sym1.is(Module) then compareOwner(cls1, sym2) + else if sym2.is(Module) then compareOwner(sym1, cls2) + else 0 /** Compare two alternatives of an overloaded call or an implicit search. * diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 0bd407261125..736633e0f6a7 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -495,8 +495,8 @@ object Signatures { case res => List(tpe) def isSyntheticEvidence(name: String) = - if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else - symbol.paramSymss.flatten.find(_.name.show == name).exists(_.flags.is(Flags.Implicit)) + name.startsWith(NameKinds.ContextBoundParamName.separator) + && symbol.paramSymss.flatten.find(_.name.show == name).exists(_.flags.is(Flags.Implicit)) def toTypeParam(tpe: PolyType): List[Param] = val evidenceParams = (tpe.paramNamess.flatten zip tpe.paramInfoss.flatten).flatMap: From 8eeee0bb4dbe156ee8b2384ab9a222aadecb9308 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 3 Apr 2024 21:45:53 +0200 Subject: [PATCH 3/4] Do prefix comparison only as a final tie breaker The alternatives with the same symbol could have nevertheless different types. We first want to disambiguate based on these types before we turn to prefixes as a final tie breaker. --- .../dotty/tools/dotc/typer/Applications.scala | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index ceef89f8bfff..10886f676732 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1839,11 +1839,7 @@ trait Applications extends Compatibility { else -1 def compareWithTypes(tp1: Type, tp2: Type) = { - val ownerScore = - val sym1 = alt1.symbol - val sym2 = alt2.symbol - if sym1 == sym2 then comparePrefixes(widenPrefix(alt1), widenPrefix(alt2)) - else compareOwner(sym1.maybeOwner, sym2.maybeOwner) + val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) @@ -1874,11 +1870,14 @@ trait Applications extends Compatibility { val strippedType2 = stripImplicit(fullType2) val result = compareWithTypes(strippedType1, strippedType2) - if (result != 0) result - else if (strippedType1 eq fullType1) - if (strippedType2 eq fullType2) 0 // no implicits either side: its' a draw + if result != 0 then result + else if strippedType1 eq fullType1 then + if strippedType2 eq fullType2 then + if alt1.symbol != alt2.symbol then 0 // no implicits either side: it's a draw ... + else comparePrefixes( // ... unless the symbol is the same, in which case + widenPrefix(alt1), widenPrefix(alt2)) // we compare prefixes else 1 // prefer 1st alternative with no implicits - else if (strippedType2 eq fullType2) -1 // prefer 2nd alternative with no implicits + else if strippedType2 eq fullType2 then -1 // prefer 2nd alternative with no implicits else compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters } end compare From 822eec652213b0ff02f9af078129bf22e6268aac Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Mon, 8 Apr 2024 16:27:33 +0200 Subject: [PATCH 4/4] Do prefix comparison also in comparison including implicit parameters --- .../dotty/tools/dotc/typer/Applications.scala | 38 +++++++++---------- .../pos/implicit-prefix-disambiguation.scala | 9 +++-- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 10886f676732..383f2fce636b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1831,18 +1831,20 @@ trait Applications extends Compatibility { * * In this case `b.M` would be regarded as more specific than `a.M`. */ - def comparePrefixes(pre1: Type, pre2: Type) = + def comparePrefixes = + val pre1 = widenPrefix(alt1) + val pre2 = widenPrefix(alt2) val winsPrefix1 = isAsSpecificValueType(pre1, pre2) val winsPrefix2 = isAsSpecificValueType(pre2, pre1) if winsPrefix1 == winsPrefix2 then 0 else if winsPrefix1 then 1 else -1 - def compareWithTypes(tp1: Type, tp2: Type) = { + def compareWithTypes(tp1: Type, tp2: Type) = val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) - def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) - def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) + val winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) + val winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) overload.println(i"compare($alt1, $alt2)? $tp1 $tp2 $ownerScore $winsType1 $winsType2") if winsType1 && winsType2 @@ -1851,15 +1853,14 @@ trait Applications extends Compatibility { // alternatives are the same after following ExprTypes, pick one of them // (prefer the one that is not a method, but that's arbitrary). if alt1.widenExpr =:= alt2 then -1 else 1 - else if ownerScore == 1 then - if winsType1 || !winsType2 then 1 else 0 - else if ownerScore == -1 then - if winsType2 || !winsType1 then -1 else 0 - else if winsType1 then - if winsType2 then 0 else 1 - else - if winsType2 then -1 else 0 - } + else ownerScore match + case 1 => if winsType1 || !winsType2 then 1 else 0 + case -1 => if winsType2 || !winsType1 then -1 else 0 + case 0 => + if winsType1 != winsType2 then if winsType1 then 1 else -1 + else if alt1.symbol == alt2.symbol then comparePrefixes + else 0 + end compareWithTypes if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1 else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1 @@ -1870,14 +1871,11 @@ trait Applications extends Compatibility { val strippedType2 = stripImplicit(fullType2) val result = compareWithTypes(strippedType1, strippedType2) - if result != 0 then result - else if strippedType1 eq fullType1 then - if strippedType2 eq fullType2 then - if alt1.symbol != alt2.symbol then 0 // no implicits either side: it's a draw ... - else comparePrefixes( // ... unless the symbol is the same, in which case - widenPrefix(alt1), widenPrefix(alt2)) // we compare prefixes + if (result != 0) result + else if (strippedType1 eq fullType1) + if (strippedType2 eq fullType2) 0 // no implicits either side: its' a draw else 1 // prefer 1st alternative with no implicits - else if strippedType2 eq fullType2 then -1 // prefer 2nd alternative with no implicits + else if (strippedType2 eq fullType2) -1 // prefer 2nd alternative with no implicits else compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters } end compare diff --git a/tests/pos/implicit-prefix-disambiguation.scala b/tests/pos/implicit-prefix-disambiguation.scala index 5059aa2db4eb..f7843e7f5831 100644 --- a/tests/pos/implicit-prefix-disambiguation.scala +++ b/tests/pos/implicit-prefix-disambiguation.scala @@ -1,7 +1,10 @@ + class I[X] +class J[X] trait A: given I[B] = ??? + given (using I[B]): J[B] = ??? object A extends A trait B extends A @@ -9,6 +12,6 @@ object B extends B //import B.given, A.given -def Test = summon[I[B]] - - +def Test = + summon[I[B]] + summon[J[B]]