Skip to content

Backport "Fix healAmbiguous to compareAlternatives with disambiguate = true" to 3.5.0 #21344

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.

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
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/config/SourceVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ enum SourceVersion:
case `3.4-migration`, `3.4`
case `3.5-migration`, `3.5`
case `3.6-migration`, `3.6`
case `3.7-migration`, `3.7`
// !!! Keep in sync with scala.runtime.stdlibPatches.language !!!
case `future-migration`, `future`

Expand Down
17 changes: 7 additions & 10 deletions compiler/src/dotty/tools/dotc/printing/Formatting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package dotty.tools
package dotc
package printing

import scala.language.unsafeNulls

import scala.collection.mutable

import core.*
Expand Down Expand Up @@ -52,7 +50,11 @@ object Formatting {
object ShowAny extends Show[Any]:
def show(x: Any): Shown = x

class ShowImplicits3:
class ShowImplicits4:
given [X: Show]: Show[X | Null] with
def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))

class ShowImplicits3 extends ShowImplicits4:
given Show[Product] = ShowAny

class ShowImplicits2 extends ShowImplicits3:
Expand All @@ -77,15 +79,10 @@ object Formatting {
given [K: Show, V: Show]: Show[Map[K, V]] with
def show(x: Map[K, V]) =
CtxShow(x.map((k, v) => s"${toStr(k)} => ${toStr(v)}"))
end given

given [H: Show, T <: Tuple: Show]: Show[H *: T] with
def show(x: H *: T) =
CtxShow(toStr(x.head) *: toShown(x.tail).asInstanceOf[Tuple])
end given

given [X: Show]: Show[X | Null] with
def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))

given Show[FlagSet] with
def show(x: FlagSet) = x.flagsString
Expand Down Expand Up @@ -148,8 +145,8 @@ object Formatting {
private def treatArg(arg: Shown, suffix: String)(using Context): (String, String) = arg.runCtxShow match {
case arg: Seq[?] if suffix.indexOf('%') == 0 && suffix.indexOf('%', 1) != -1 =>
val end = suffix.indexOf('%', 1)
val sep = StringContext.processEscapes(suffix.substring(1, end))
(arg.mkString(sep), suffix.substring(end + 1))
val sep = StringContext.processEscapes(suffix.substring(1, end).nn)
(arg.mkString(sep), suffix.substring(end + 1).nn)
case arg: Seq[?] =>
(arg.map(showArg).mkString("[", ", ", "]"), suffix)
case arg =>
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2955,7 +2955,7 @@ class MissingImplicitArgument(

/** Default error message for non-nested ambiguous implicits. */
def defaultAmbiguousImplicitMsg(ambi: AmbiguousImplicits) =
s"Ambiguous given instances: ${ambi.explanation}${location("of")}"
s"Ambiguous given instances: ${ambi.explanation}${location("of")}${ambi.priorityChangeWarningNote}"

/** Default error messages for non-ambiguous implicits, or nested ambiguous
* implicits.
Expand Down
79 changes: 47 additions & 32 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,17 @@ trait Applications extends Compatibility {
else if sym2.is(Module) then compareOwner(sym1, cls2)
else 0

enum CompareScheme:
case Old // Normal specificity test for overloading resolution (where `preferGeneral` is false)
// and in mode Scala3-migration when we compare with the old Scala 2 rules.

case Intermediate // Intermediate rules: better means specialize, but map all type arguments downwards
// These are enabled for 3.0-3.5, or if OldImplicitResolution
// is specified, and also for all comparisons between old-style implicits,

case New // New rules: better means generalize, givens (and extensions) always beat implicits
end CompareScheme

/** Compare two alternatives of an overloaded call or an implicit search.
*
* @param alt1, alt2 Non-overloaded references indicating the two choices
Expand All @@ -1774,6 +1785,15 @@ trait Applications extends Compatibility {
*/
def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) {
record("resolveOverloaded.compare")
val scheme =
val oldResolution = ctx.mode.is(Mode.OldImplicitResolution)
if !preferGeneral || Feature.migrateTo3 && oldResolution then
CompareScheme.Old
else if Feature.sourceVersion.isAtMost(SourceVersion.`3.5`)
|| oldResolution
|| alt1.symbol.is(Implicit) && alt2.symbol.is(Implicit)
then CompareScheme.Intermediate
else CompareScheme.New

/** Is alternative `alt1` with type `tp1` as good as alternative
* `alt2` with type `tp2` ?
Expand Down Expand Up @@ -1816,17 +1836,15 @@ trait Applications extends Compatibility {
isAsGood(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
}
case _ => // (3)
def isGiven(alt: TermRef) =
alt1.symbol.is(Given) && alt.symbol != defn.NotGivenClass
def compareValues(tp1: Type, tp2: Type)(using Context) =
isAsGoodValueType(tp1, tp2, isGiven(alt1), isGiven(alt2))
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)
explore(compareValues(tp1, instantiateWithTypeVars(tp2)))
explore(compareValues(instantiateWithTypeVars(tp2)))
case _ => // 3b)
compareValues(tp1, tp2)
compareValues(tp2)
}

/** Test whether value type `tp1` is as good as value type `tp2`.
Expand All @@ -1837,7 +1855,7 @@ trait Applications extends Compatibility {
* available in 3.0-migration if mode `Mode.OldImplicitResolution` is turned on as well.
* It is used to highlight differences between Scala 2 and 3 behavior.
*
* - In Scala 3.0-3.5, the behavior is as follows: `T <:p U` iff there is an impliit conversion
* - In Scala 3.0-3.6, the behavior is as follows: `T <:p U` iff there is an implicit conversion
* from `T` to `U`, or
*
* flip(T) <: flip(U)
Expand All @@ -1852,21 +1870,20 @@ trait Applications extends Compatibility {
* of parameters are not affected. So `T <: U` would imply `Set[Cmp[U]] <:p Set[Cmp[T]]`,
* as usual, because `Set` is non-variant.
*
* - From Scala 3.6, `T <:p U` means `T <: U` or `T` convertible to `U`
* - From Scala 3.7, `T <:p U` means `T <: U` or `T` convertible to `U`
* for overloading resolution (when `preferGeneral is false), and the opposite relation
* `U <: T` or `U convertible to `T` for implicit disambiguation between givens
* (when `preferGeneral` is true). For old-style implicit values, the 3.4 behavior is kept.
* If one of the alternatives is a given and the other is an implicit, the given wins.
* (when `preferGeneral` is true). For old-style implicit values, the 3.5 behavior is kept.
* If one of the alternatives is an implicit and the other is a given (or an extension), the implicit loses.
*
* - In Scala 3.5 and Scala 3.6-migration, we issue a warning if the result under
* Scala 3.6 differ wrt to the old behavior up to 3.5.
* - In Scala 3.6 and Scala 3.7-migration, we issue a warning if the result under
* Scala 3.7 differs wrt to the old behavior up to 3.6.
*
* Also and only for given resolution: If a compared type refers to a given or its module class, use
* the intersection of its parent classes instead.
*/
def isAsGoodValueType(tp1: Type, tp2: Type, alt1isGiven: Boolean, alt2isGiven: Boolean)(using Context): Boolean =
val oldResolution = ctx.mode.is(Mode.OldImplicitResolution)
if !preferGeneral || Feature.migrateTo3 && oldResolution then
def isAsGoodValueType(tp1: Type, tp2: Type, alt1IsImplicit: Boolean)(using Context): Boolean =
if scheme == CompareScheme.Old then
// Normal specificity test for overloading resolution (where `preferGeneral` is false)
// and in mode Scala3-migration when we compare with the old Scala 2 rules.
isCompatible(tp1, tp2)
Expand All @@ -1880,13 +1897,7 @@ trait Applications extends Compatibility {
val tp1p = prepare(tp1)
val tp2p = prepare(tp2)

if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`)
|| oldResolution
|| !alt1isGiven && !alt2isGiven
then
// Intermediate rules: better means specialize, but map all type arguments downwards
// These are enabled for 3.0-3.5, and for all comparisons between old-style implicits,
// and in 3.5 and 3.6-migration when we compare with previous rules.
if scheme == CompareScheme.Intermediate || alt1IsImplicit then
val flip = new TypeMap:
def apply(t: Type) = t match
case t @ AppliedType(tycon, args) =>
Expand All @@ -1897,9 +1908,7 @@ trait Applications extends Compatibility {
case _ => mapOver(t)
(flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2)
else
// New rules: better means generalize, givens always beat implicits
if alt1isGiven != alt2isGiven then alt1isGiven
else (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
(tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
end isAsGoodValueType

/** Widen the result type of synthetic given methods from the implementation class to the
Expand Down Expand Up @@ -1970,13 +1979,19 @@ 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 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
else
// For new implicit resolution, take ownerscore as more significant than type resolution
// Reason: People use owner hierarchies to explicitly prioritize, we should not
// break that by changing implicit priority of types.
def drawOrOwner =
if scheme == CompareScheme.New then ownerScore else 0
ownerScore match
case 1 => if winsType1 || !winsType2 then 1 else drawOrOwner
case -1 => if winsType2 || !winsType1 then -1 else drawOrOwner
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
Expand Down
Loading
Loading