Tweaks to given priority

compiler/src/dotty/tools/dotc/printing/Formatting.scala

@@ -2,8 +2,6 @@ package dotty.tools
 package dotc
 package printing
 
-import scala.language.unsafeNulls
-
 import scala.collection.mutable
 
 import core.*
@@ -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:
@@ -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
@@ -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 =>

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -2988,7 +2988,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.

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

@@ -1830,15 +1830,13 @@ trait Applications extends Compatibility {
  isAsGood(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
  }
  case _ => // (3)
- def compareValues(tp1: Type, tp2: Type)(using Context) =
- isAsGoodValueType(tp1, tp2, alt1.symbol.is(Implicit), alt2.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(isAsGoodValueType(tp1, instantiateWithTypeVars(tp2)))
  case _ => // 3b)
- compareValues(tp1, tp2)
+ isAsGoodValueType(tp1, tp2)
  }
 
  /** Test whether value type `tp1` is as good as value type `tp2`.
@@ -1867,16 +1865,15 @@ trait Applications extends Compatibility {
  * - From Scala 3.6, `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 an implicit and the other is a given (or an extension), the implicit loses.
+ * (when `preferGeneral` is true).
  *
  * - 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.
  *
  * 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, alt1IsImplicit: Boolean, alt2IsImplicit: Boolean)(using Context): Boolean =
+ def isAsGoodValueType(tp1: Type, tp2: Type)(using Context): Boolean =
  val oldResolution = ctx.mode.is(Mode.OldImplicitResolution)
  if !preferGeneral || Feature.migrateTo3 && oldResolution then
  // Normal specificity test for overloading resolution (where `preferGeneral` is false)
@@ -1892,10 +1889,7 @@ trait Applications extends Compatibility {
  val tp1p = prepare(tp1)
  val tp2p = prepare(tp2)
 
- if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`)
- || oldResolution
- || alt1IsImplicit && alt2IsImplicit
- then
+ if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) || oldResolution 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.
@@ -1909,9 +1903,8 @@ trait Applications extends Compatibility {
  case _ => mapOver(t)
  (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2)
  else
- // New rules: better means generalize, givens (and extensions) always beat implicits
- if alt1IsImplicit != alt2IsImplicit then alt2IsImplicit
- else (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
+ // New rules: better means generalize
+ (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
  end isAsGoodValueType
 
  /** Widen the result type of synthetic given methods from the implementation class to the
@@ -1970,8 +1963,11 @@ trait Applications extends Compatibility {
  else if winsPrefix1 then 1
  else -1
 
+ val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
+ val implicitPair = alt1.symbol.is(Implicit) && alt2.symbol.is(Implicit)
+ val newGivenRules = preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)
+
  def compareWithTypes(tp1: Type, tp2: Type) =
- val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
  val winsType1 = isAsGood(alt1, tp1, alt2, tp2)
  val winsType2 = isAsGood(alt2, tp2, alt1, tp1)
 
@@ -1982,13 +1978,22 @@ 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.
+ // But don't do that for implicit/implicit pairs. It's better to leave
+ // them ambiguous so that the logic in Implicits/compareAlternatives kicks in
+ // which resolves them with the old rules.
+ def drawOrOwner =
+ if newGivenRules && !implicitPair 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
@@ -1999,13 +2004,20 @@ trait Applications extends Compatibility {
  val strippedType1 = stripImplicit(fullType1)
  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
+ var result = compareWithTypes(strippedType1, strippedType2)
+ if result != 0 then result
+ else if ownerScore != 0 && newGivenRules && implicitPair then
+ 0 // for implicit/implicit pairs fail fast
+ // so that we retry in compareAlternatives with old rules.
+ // Note: This is problematic since it fails the transitity
+ // requirement, i.e compareAlternatives is not a partial order
+ // anymore. But it might be needed to keep the number of failing
+ // projects smaller
+ else if strippedType1 eq fullType1 then
+ if strippedType2 eq fullType2 then 0 // no implicits either side: its' a draw
  else 1 // prefer 1st 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
+ else if strippedType2 eq fullType2 then -1 // prefer 2nd alternative with no implicits
+ else compareWithTypes(fullType1, fullType2) // continue by comparing implicit parameters
  }
  end compare
 

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

@@ -549,6 +549,11 @@ object Implicits:
  /** An ambiguous implicits failure */
  class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
 
+ private[Implicits] var priorityChangeWarnings: List[Message] = Nil
+
+ def priorityChangeWarningNote(using Context): String =
+ priorityChangeWarnings.map(msg => s"\n\nNote: $msg").mkString
+
  def msg(using Context): Message =
  var str1 = err.refStr(alt1.ref)
  var str2 = err.refStr(alt2.ref)
@@ -1330,10 +1335,13 @@ trait Implicits:
  if alt1.ref eq alt2.ref then 0
  else if alt1.level != alt2.level then alt1.level - alt2.level
  else
- var cmp = comp(using searchContext())
+ lazy val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution))
+ val cmp = comp(using searchContext()) match
+ // if we get an ambiguity with new rules for a pair of old-style implicits, fall back to old rules
+ case 0 if alt1.ref.symbol.is(Implicit) && alt2.ref.symbol.is(Implicit) => prev
+ case cmp => cmp
  val sv = Feature.sourceVersion
  if isWarnPriorityChangeVersion(sv) then
- val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution))
  if disambiguate && cmp != prev then
  def warn(msg: Message) =
  val critical = alt1.ref :: alt2.ref :: Nil
@@ -1345,13 +1353,21 @@ trait Implicits:
  case _ => "none - it's ambiguous"
  if sv.stable == SourceVersion.`3.5` then
  warn(
- em"""Given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} will change
+ em"""Given search preference for $pt between alternatives
+ | ${alt1.ref}
+ |and
+ | ${alt2.ref}
+ |will change.
  |Current choice : ${choice(prev)}
  |New choice from Scala 3.6: ${choice(cmp)}""")
  prev
  else
  warn(
- em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref}
+ em"""Given search preference for $pt between alternatives
+ | ${alt1.ref}
+ |and
+ | ${alt2.ref}
+ |has changed.
  |Previous choice : ${choice(prev)}
  |New choice from Scala 3.6: ${choice(cmp)}""")
  cmp
@@ -1610,9 +1626,23 @@ trait Implicits:
  throw ex
 
  val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil)
- for (critical, msg) <- priorityChangeWarnings do
- if result.found.exists(critical.contains(_)) then
- report.warning(msg, srcPos)
+
+ // Issue all priority change warnings that can affect the result
+ val shownWarnings = priorityChangeWarnings.toList.collect:
+ case (critical, msg) if result.found.exists(critical.contains(_)) =>
+ msg
+ result match
+ case result: SearchFailure =>
+ result.reason match
+ case ambi: AmbiguousImplicits =>
+ // Make warnings part of error message because otherwise they are suppressed when
+ // the error is emitted.
+ ambi.priorityChangeWarnings = shownWarnings
+ case _ =>
+ case _ =>
+ for msg <- shownWarnings do
+ report.warning(msg, srcPos)
+
  result
  end searchImplicit
 

compiler/test/dotty/tools/dotc/StringFormatterTest.scala

@@ -23,6 +23,7 @@ class StringFormatterTest extends AbstractStringFormatterTest:
  @Test def flagsTup = check("(<static>,final)", i"${(JavaStatic, Final)}")
  @Test def seqOfTup2 = check("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %")
  @Test def seqOfTup3 = check("(Foo,given, (right is approximated))", i"${Seq((Foo, Given, TypeComparer.ApproxState.None.addHigh))}%, %")
+ @Test def tupleNull = check("(1,null)", i"${(1, null: String | Null)}")
 
  class StorePrinter extends Printer:
  var string: String = "<never set>"

tests/neg/given-triangle.check

@@ -2,3 +2,11 @@
 15 |@main def Test = f // error
  | ^
  |Ambiguous given instances: both given instance given_B and given instance given_C match type A of parameter a of method f
+ |
+ |Note: Given search preference for A between alternatives
+ | (given_A : A)
+ |and
+ | (given_B : B)
+ |will change.
+ |Current choice : the second alternative
+ |New choice from Scala 3.6: the first alternative

tests/neg/i21303/JavaEnum.java

@@ -0,0 +1 @@
+public enum JavaEnum { ABC, DEF, GHI }

tests/neg/i21303/Test.scala

@@ -0,0 +1,33 @@
+//> using options -source 3.6-migration
+import scala.deriving.Mirror
+import scala.compiletime.*
+import scala.reflect.ClassTag
+import scala.annotation.implicitNotFound
+
+
+trait TSType[T]
+object TSType extends DefaultTSTypes with TSTypeMacros
+
+trait TSNamedType[T] extends TSType[T]
+
+trait DefaultTSTypes extends JavaTSTypes
+trait JavaTSTypes {
+ given javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSNamedType[E] = ???
+}
+object DefaultTSTypes extends DefaultTSTypes
+trait TSTypeMacros {
+ inline given [T: Mirror.Of]: TSType[T] = derived[T]
+ inline def derived[T](using m: Mirror.Of[T]): TSType[T] = {
+ val elemInstances = summonAll[m.MirroredElemTypes]
+ ???
+ }
+
+ private inline def summonAll[T <: Tuple]: List[TSType[_]] = {
+ inline erasedValue[T] match {
+ case _: EmptyTuple => Nil
+ case _: (t *: ts) => summonInline[TSType[t]] :: summonAll[ts]
+ }
+ }
+}
+
+@main def Test = summon[TSType[JavaEnum]] // error

tests/neg/i2974.scala

@@ -0,0 +1,28 @@
+
+trait Foo[-T]
+trait Bar[-T] extends Foo[T]
+
+object Test {
+
+ locally:
+ implicit val fa: Foo[Int] = ???
+ implicit val ba: Bar[Int] = ???
+ summon[Foo[Int]] // ok
+
+ locally:
+ implicit val fa: Foo[Int] = ???
+ implicit val ba: Bar[Any] = ???
+ summon[Foo[Int]] // ok
+
+ locally:
+ given fa: Foo[Any] = ???
+ given ba: Bar[Int] = ???
+ summon[Foo[Int]] // error: now ambiguous,
+ // was resolving to `ba` when using intermediate rules:
+ // better means specialize, but map all type arguments downwards
+
+ locally:
+ implicit val fa: Foo[Any] = ???
+ implicit val ba: Bar[Int] = ???
+ summon[Foo[Int]] // is OK since we fall back to old rules for old-style implicits as a tie breaker
+}

tests/neg/scala-uri.check

@@ -0,0 +1,14 @@
+-- [E172] Type Error: tests/neg/scala-uri.scala:30:59 ------------------------------------------------------------------
+30 |@main def Test = summon[QueryKeyValue[(String, None.type)]] // error
+ | ^
+ |No best given instance of type QueryKeyValue[(String, None.type)] was found for parameter x of method summon in object Predef.
+ |I found:
+ |
+ | QueryKeyValue.tuple2QueryKeyValue[String, None.type](QueryKey.stringQueryKey,
+ | QueryValue.optionQueryValue[A](
+ | /* ambiguous: both given instance stringQueryValue in trait QueryValueInstances1 and given instance noneQueryValue in trait QueryValueInstances1 match type QueryValue[A] */
+ | summon[QueryValue[A]]
+ | )
+ | )
+ |
+ |But both given instance stringQueryValue in trait QueryValueInstances1 and given instance noneQueryValue in trait QueryValueInstances1 match type QueryValue[A].