From bdfb12e17b72c1bd1e307f65abf955d69d992179 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 10:11:39 +0200 Subject: [PATCH 1/7] Tweaks to given priority - Don't treat givens as higher priority than implicits. The previous logic was faulty anyway. - Drop special handling of NotGiven in prioritization. The previous logic pretended to do so, but was ineffective. - Don't use old priority in general for implicit/implicit pairs. This would make upgrading to givens a constant struggle. But do use it as a tie breaker in the case of ambiguities. - Also use owner score as a tie breaker if after all other tests we still have an ambiguity. --- community-build/community-projects/PPrint | 2 +- .../scala-collection-compat | 2 +- .../tools/dotc/printing/Formatting.scala | 8 +-- .../dotty/tools/dotc/typer/Applications.scala | 57 +++++++++++-------- .../dotty/tools/dotc/typer/Implicits.scala | 7 ++- .../tools/dotc/StringFormatterTest.scala | 1 + tests/neg/i2974.scala | 28 +++++++++ tests/pos/given-priority.scala | 24 ++++++++ tests/pos/i2974.scala | 12 ---- tests/pos/scala-uri.scala | 29 ++++++++++ tests/run/enrich-gentraversable.check | 2 +- tests/run/enrich-gentraversable.scala | 3 +- tests/run/implicits_poly.check | 2 +- tests/warn/i15503a.scala | 8 +-- 14 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 tests/neg/i2974.scala create mode 100644 tests/pos/given-priority.scala delete mode 100644 tests/pos/i2974.scala create mode 100644 tests/pos/scala-uri.scala diff --git a/community-build/community-projects/PPrint b/community-build/community-projects/PPrint index 34a777f687bc..2203dc6081f5 160000 --- a/community-build/community-projects/PPrint +++ b/community-build/community-projects/PPrint @@ -1 +1 @@ -Subproject commit 34a777f687bc851953e682f99edcae9d2875babc +Subproject commit 2203dc6081f5e8fa89f552b155724b0a8fdcec03 diff --git a/community-build/community-projects/scala-collection-compat b/community-build/community-projects/scala-collection-compat index 2bf3fea914b2..a81b5bc7661a 160000 --- a/community-build/community-projects/scala-collection-compat +++ b/community-build/community-projects/scala-collection-compat @@ -1 +1 @@ -Subproject commit 2bf3fea914b2f13e4805b3e7b519bdf0e595e4c9 +Subproject commit a81b5bc7661a22d2a91328e1a1ff1a006c1e5336 diff --git a/compiler/src/dotty/tools/dotc/printing/Formatting.scala b/compiler/src/dotty/tools/dotc/printing/Formatting.scala index 6f1c32beb822..58616f25f3b3 100644 --- a/compiler/src/dotty/tools/dotc/printing/Formatting.scala +++ b/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.* @@ -77,12 +75,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)) @@ -148,8 +144,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 => diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 42765cd6c0bf..b6c683e3d6f1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/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`. @@ -1868,7 +1866,6 @@ trait Applications extends Compatibility { * 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. * * - 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. @@ -1876,7 +1873,7 @@ trait Applications extends Compatibility { * 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,9 @@ trait Applications extends Compatibility { else if winsPrefix1 then 1 else -1 + val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) + 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,15 +1976,27 @@ 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 + 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 + // For 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. On the other hand, we do + // want to exhaust all other possibilities before using owner score as a tie breaker. + // For instance, pos/scala-uri.scala depends on that. + def drawOrOwner = + if preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) then + //println(i"disambi compare($alt1, $alt2)? $ownerScore") + ownerScore + else 0 + if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1 else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1 else @@ -2000,11 +2006,12 @@ 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 drawOrOwner // 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 if strippedType2 eq fullType2 then -1 // prefer 2nd alternative with no implicits else compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters } end compare diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index dac0c0e78448..6e008957aafa 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1330,10 +1330,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 diff --git a/compiler/test/dotty/tools/dotc/StringFormatterTest.scala b/compiler/test/dotty/tools/dotc/StringFormatterTest.scala index 4dfc08cc7e9b..b0ff8b8fc03e 100644 --- a/compiler/test/dotty/tools/dotc/StringFormatterTest.scala +++ b/compiler/test/dotty/tools/dotc/StringFormatterTest.scala @@ -23,6 +23,7 @@ class StringFormatterTest extends AbstractStringFormatterTest: @Test def flagsTup = check("(,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 = "" diff --git a/tests/neg/i2974.scala b/tests/neg/i2974.scala new file mode 100644 index 000000000000..a77b4a61da45 --- /dev/null +++ b/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 +} diff --git a/tests/pos/given-priority.scala b/tests/pos/given-priority.scala new file mode 100644 index 000000000000..048e063eff35 --- /dev/null +++ b/tests/pos/given-priority.scala @@ -0,0 +1,24 @@ +/* These tests show various mechanisms available for implicit prioritization. + */ +import language.`3.6` + +class A // The type for which we infer terms below +class B extends A + +/* First, two schemes that require a pre-planned architecture for how and + * where given instances are defined. + * + * Traditional scheme: prioritize with location in class hierarchy + */ +class LowPriorityImplicits: + given g1: A() + +object NormalImplicits extends LowPriorityImplicits: + given g2: B() + +def test1 = + import NormalImplicits.given + val x = summon[A] + val _: B = x + val y = summon[B] + val _: B = y diff --git a/tests/pos/i2974.scala b/tests/pos/i2974.scala deleted file mode 100644 index 75c6a24a41bb..000000000000 --- a/tests/pos/i2974.scala +++ /dev/null @@ -1,12 +0,0 @@ -trait Foo[-T] - -trait Bar[-T] extends Foo[T] - -object Test { - implicit val fa: Foo[Any] = ??? - implicit val ba: Bar[Int] = ??? - - def test: Unit = { - implicitly[Foo[Int]] - } -} diff --git a/tests/pos/scala-uri.scala b/tests/pos/scala-uri.scala new file mode 100644 index 000000000000..1adc50c8a6ab --- /dev/null +++ b/tests/pos/scala-uri.scala @@ -0,0 +1,29 @@ +import scala.language.implicitConversions + +trait QueryKey[A] +object QueryKey extends QueryKeyInstances +sealed trait QueryKeyInstances: + implicit val stringQueryKey: QueryKey[String] = ??? + +trait QueryValue[-A] +object QueryValue extends QueryValueInstances +sealed trait QueryValueInstances1: + implicit final val stringQueryValue: QueryValue[String] = ??? + implicit final val noneQueryValue: QueryValue[None.type] = ??? + +sealed trait QueryValueInstances extends QueryValueInstances1: + implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? + +trait QueryKeyValue[A] +object QueryKeyValue: + implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? + + +@main def Test = summon[QueryKeyValue[(String, None.type)]] + +/*(using + QueryKeyValue.tuple2QueryKeyValue[String, None.type]( + QueryKey.stringQueryKey, + QueryValue.optionQueryValue[A]))*/ + + // error \ No newline at end of file diff --git a/tests/run/enrich-gentraversable.check b/tests/run/enrich-gentraversable.check index 7a5611a13a08..dd33eda94c7a 100644 --- a/tests/run/enrich-gentraversable.check +++ b/tests/run/enrich-gentraversable.check @@ -4,5 +4,5 @@ List(2, 4) HW ArraySeq(72, 108, 108, 32, 114, 108, 100) Map(bar -> 2) -TreeMap(bar -> 2) +Map(bar -> 2) Map(bar -> 2) diff --git a/tests/run/enrich-gentraversable.scala b/tests/run/enrich-gentraversable.scala index 887a6aea7983..170861fc861e 100644 --- a/tests/run/enrich-gentraversable.scala +++ b/tests/run/enrich-gentraversable.scala @@ -47,7 +47,8 @@ object Test extends App { val tm = TreeMap(1 -> "foo", 2 -> "bar") val tmm = tm.filterMap { case (k, v) => if (k % 2 == 0) Some(v -> k) else None } - typed[TreeMap[String, Int]](tmm) + typed[Map[String, Int]](tmm) // was TreeMap, now Map, + // since `BuildFrom` is covariant in `That` and `TreeMap[String, Int] <:< Map[String, Int]` println(tmm) val mv = m.view diff --git a/tests/run/implicits_poly.check b/tests/run/implicits_poly.check index 0b148ee47940..d7e3e4927e18 100644 --- a/tests/run/implicits_poly.check +++ b/tests/run/implicits_poly.check @@ -1 +1 @@ -barFoo +fooFoo diff --git a/tests/warn/i15503a.scala b/tests/warn/i15503a.scala index df8691c21a13..972ffe878d15 100644 --- a/tests/warn/i15503a.scala +++ b/tests/warn/i15503a.scala @@ -149,8 +149,8 @@ object GivenImportOrderAtoB: object A { implicit val x: X = new X } object B { implicit val y: Y = new Y } class C { - import A._ // warn - import B._ // OK + import A._ // OK + import B._ // warn def t = implicitly[X] } @@ -160,8 +160,8 @@ object GivenImportOrderBtoA: object A { implicit val x: X = new X } object B { implicit val y: Y = new Y } class C { - import B._ // OK - import A._ // warn + import B._ // warn + import A._ // OK def t = implicitly[X] } /* END : tests on given import order */ From 3cf4adaad5d506228714339a6fd7899e90640c22 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 12:09:29 +0200 Subject: [PATCH 2/7] Add priority change warnings to ambiguous implicits error messages Previously warnings were produced but not shown since at the same position we already have an ambiguity error. We now add the note to the error message. --- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../dotty/tools/dotc/typer/Implicits.scala | 24 ++++++++++++-- tests/neg/given-triangle.check | 8 +++++ tests/neg/i21303/JavaEnum.java | 1 + tests/neg/i21303/Test.scala | 33 +++++++++++++++++++ tests/pos/i21303/JavaEnum.java | 1 + tests/pos/i21303/Test.scala | 32 ++++++++++++++++++ tests/warn/i21036a.check | 6 +++- tests/warn/i21036b.check | 6 +++- 9 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i21303/JavaEnum.java create mode 100644 tests/neg/i21303/Test.scala create mode 100644 tests/pos/i21303/JavaEnum.java create mode 100644 tests/pos/i21303/Test.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 1d906130d4e4..5d5842e44e15 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/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. diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 6e008957aafa..0130804e0a61 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -549,6 +549,12 @@ 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 priorityChangeWarning: Message | Null = null + + def priorityChangeWarningNote(using Context): String = + if priorityChangeWarning != null then s"\n\nNote: $priorityChangeWarning" + else "" + def msg(using Context): Message = var str1 = err.refStr(alt1.ref) var str2 = err.refStr(alt2.ref) @@ -1348,13 +1354,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 @@ -1615,6 +1629,12 @@ trait Implicits: val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil) for (critical, msg) <- priorityChangeWarnings do if result.found.exists(critical.contains(_)) then + result match + case result: SearchFailure => + result.reason match + case ambi: AmbiguousImplicits => ambi.priorityChangeWarning = msg + case _ => + case _ => report.warning(msg, srcPos) result end searchImplicit diff --git a/tests/neg/given-triangle.check b/tests/neg/given-triangle.check index f548df0078de..73d5aea12dc4 100644 --- a/tests/neg/given-triangle.check +++ b/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 diff --git a/tests/neg/i21303/JavaEnum.java b/tests/neg/i21303/JavaEnum.java new file mode 100644 index 000000000000..e626d5070626 --- /dev/null +++ b/tests/neg/i21303/JavaEnum.java @@ -0,0 +1 @@ +public enum JavaEnum { ABC, DEF, GHI } diff --git a/tests/neg/i21303/Test.scala b/tests/neg/i21303/Test.scala new file mode 100644 index 000000000000..fa8058140067 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/pos/i21303/JavaEnum.java b/tests/pos/i21303/JavaEnum.java new file mode 100644 index 000000000000..e626d5070626 --- /dev/null +++ b/tests/pos/i21303/JavaEnum.java @@ -0,0 +1 @@ +public enum JavaEnum { ABC, DEF, GHI } diff --git a/tests/pos/i21303/Test.scala b/tests/pos/i21303/Test.scala new file mode 100644 index 000000000000..fe3efa6e38f3 --- /dev/null +++ b/tests/pos/i21303/Test.scala @@ -0,0 +1,32 @@ +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]: TSType[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]] \ No newline at end of file diff --git a/tests/warn/i21036a.check b/tests/warn/i21036a.check index 673c01374ef3..876a81ad8a83 100644 --- a/tests/warn/i21036a.check +++ b/tests/warn/i21036a.check @@ -1,6 +1,10 @@ -- Warning: tests/warn/i21036a.scala:7:17 ------------------------------------------------------------------------------ 7 |val y = summon[A] // warn | ^ - | Given search preference for A between alternatives (b : B) and (a : A) will change + | Given search preference for A between alternatives + | (b : B) + | and + | (a : A) + | will change. | Current choice : the first alternative | New choice from Scala 3.6: the second alternative diff --git a/tests/warn/i21036b.check b/tests/warn/i21036b.check index ff7fdfd7a87c..11bb38727d77 100644 --- a/tests/warn/i21036b.check +++ b/tests/warn/i21036b.check @@ -1,6 +1,10 @@ -- Warning: tests/warn/i21036b.scala:7:17 ------------------------------------------------------------------------------ 7 |val y = summon[A] // warn | ^ - | Change in given search preference for A between alternatives (b : B) and (a : A) + | Given search preference for A between alternatives + | (b : B) + | and + | (a : A) + | has changed. | Previous choice : the first alternative | New choice from Scala 3.6: the second alternative From 2795a50ff9b64605d410248f05ac3b5307406b43 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 12:33:38 +0200 Subject: [PATCH 3/7] Another test variation of #21303 --- tests/pos/i21303a/JavaEnum.java | 1 + tests/pos/i21303a/Test.scala | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/pos/i21303a/JavaEnum.java create mode 100644 tests/pos/i21303a/Test.scala diff --git a/tests/pos/i21303a/JavaEnum.java b/tests/pos/i21303a/JavaEnum.java new file mode 100644 index 000000000000..e626d5070626 --- /dev/null +++ b/tests/pos/i21303a/JavaEnum.java @@ -0,0 +1 @@ +public enum JavaEnum { ABC, DEF, GHI } diff --git a/tests/pos/i21303a/Test.scala b/tests/pos/i21303a/Test.scala new file mode 100644 index 000000000000..83a598b5f17f --- /dev/null +++ b/tests/pos/i21303a/Test.scala @@ -0,0 +1,35 @@ +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]: TSType[E] = ??? + given javaEnumTSNamedType[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]] + summon[TSNamedType[JavaEnum]] From bba0db34727ee48648603d617f33e74be8288cb0 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 15:09:23 +0200 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Eugene Flesselle --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index b6c683e3d6f1..ef9947e96903 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1865,7 +1865,7 @@ 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. + * (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. @@ -2011,7 +2011,7 @@ trait Applications extends Compatibility { if strippedType2 eq fullType2 then drawOrOwner // 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 then -1 // prefer 2nd alternative with no implicits else compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters } end compare From 462b2934bee0539a6cd2ca5419958c4e0783c3e0 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 16:21:54 +0200 Subject: [PATCH 5/7] Apply suggestions from code review --- .../dotty/tools/dotc/typer/Applications.scala | 20 ++++++------- .../dotty/tools/dotc/typer/Implicits.scala | 29 ++++++++++++------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index ef9947e96903..da7a39d94e0e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1991,11 +1991,10 @@ trait Applications extends Compatibility { // break that by changing implicit priority of types. On the other hand, we do // want to exhaust all other possibilities before using owner score as a tie breaker. // For instance, pos/scala-uri.scala depends on that. - def drawOrOwner = - if preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) then - //println(i"disambi compare($alt1, $alt2)? $ownerScore") - ownerScore - else 0 + def disambiguateWithOwner(result: Int) = + if result == 0 && preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) + then ownerScore + else result if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1 else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1 @@ -2005,14 +2004,15 @@ trait Applications extends Compatibility { val strippedType1 = stripImplicit(fullType1) val strippedType2 = stripImplicit(fullType2) - val result = compareWithTypes(strippedType1, strippedType2) - if result != 0 then result - else if strippedType1 eq fullType1 then + var result = compareWithTypes(strippedType1, strippedType2) + if result != 0 then return result + if strippedType1 eq fullType1 then if strippedType2 eq fullType2 - then drawOrOwner // no implicits either side: its' a draw + then disambiguateWithOwner(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 compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters + else disambiguateWithOwner( + compareWithTypes(fullType1, fullType2)) // continue by comparing implicit parameters } end compare diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 0130804e0a61..e57459bd11e1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -549,11 +549,10 @@ 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 priorityChangeWarning: Message | Null = null + private[Implicits] var priorityChangeWarnings: List[Message] = Nil def priorityChangeWarningNote(using Context): String = - if priorityChangeWarning != null then s"\n\nNote: $priorityChangeWarning" - else "" + priorityChangeWarnings.map(msg => s"\n\nNote: $msg").mkString def msg(using Context): Message = var str1 = err.refStr(alt1.ref) @@ -1627,15 +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 - result match - case result: SearchFailure => - result.reason match - case ambi: AmbiguousImplicits => ambi.priorityChangeWarning = msg - case _ => + + // 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 _ => - report.warning(msg, srcPos) + case _ => + for msg <- shownWarnings do + report.warning(msg, srcPos) + result end searchImplicit From 3d9ce8eb42c777e5e030c447343d2ef018e4ec1c Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 31 Jul 2024 19:21:08 +0200 Subject: [PATCH 6/7] Disambiguate by owner score before considering further implicit arguments This makes slick-migration-api-example work and makes the original scala-uri.scala fail. See the comment in neg/scala-uri.scala for why this is so. It also needs a change in the givens in dotty.tools.reporting.Formatting. The motivation for honoring owner score over the others is that it is often used for explicit prioritization, so we should take it into account more than other secondary criteria. --- .../tools/dotc/printing/Formatting.scala | 9 ++--- .../dotty/tools/dotc/typer/Applications.scala | 36 ++++++++----------- tests/neg/scala-uri.check | 14 ++++++++ tests/neg/scala-uri.scala | 30 ++++++++++++++++ tests/pos/scala-uri.scala | 9 +---- tests/pos/slick-migration-api-example.scala | 23 ++++++++++++ 6 files changed, 88 insertions(+), 33 deletions(-) create mode 100644 tests/neg/scala-uri.check create mode 100644 tests/neg/scala-uri.scala create mode 100644 tests/pos/slick-migration-api-example.scala diff --git a/compiler/src/dotty/tools/dotc/printing/Formatting.scala b/compiler/src/dotty/tools/dotc/printing/Formatting.scala index 58616f25f3b3..43cac17e6318 100644 --- a/compiler/src/dotty/tools/dotc/printing/Formatting.scala +++ b/compiler/src/dotty/tools/dotc/printing/Formatting.scala @@ -50,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: @@ -80,9 +84,6 @@ object Formatting { def show(x: H *: T) = CtxShow(toStr(x.head) *: toShown(x.tail).asInstanceOf[Tuple]) - 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 diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index da7a39d94e0e..ef98c33d5221 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1865,7 +1865,7 @@ 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). + * (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. @@ -1963,9 +1963,8 @@ trait Applications extends Compatibility { else if winsPrefix1 then 1 else -1 - val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) - 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) @@ -1977,25 +1976,22 @@ trait Applications extends Compatibility { // (prefer the one that is not a method, but that's arbitrary). if alt1.widenExpr =:= alt2 then -1 else 1 else + // For 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 preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) + then ownerScore + else 0 ownerScore match - case 1 => if winsType1 || !winsType2 then 1 else 0 - case -1 => if winsType2 || !winsType1 then -1 else 0 + 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 - // For 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. On the other hand, we do - // want to exhaust all other possibilities before using owner score as a tie breaker. - // For instance, pos/scala-uri.scala depends on that. - def disambiguateWithOwner(result: Int) = - if result == 0 && preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) - then ownerScore - else result - if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1 else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1 else @@ -2005,14 +2001,12 @@ trait Applications extends Compatibility { val strippedType2 = stripImplicit(fullType2) var result = compareWithTypes(strippedType1, strippedType2) - if result != 0 then return result - if strippedType1 eq fullType1 then - if strippedType2 eq fullType2 - then disambiguateWithOwner(0) // no implicits either side: its' a draw + if result != 0 then result + 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 then -1 // prefer 2nd alternative with no implicits - else disambiguateWithOwner( - compareWithTypes(fullType1, fullType2)) // continue by comparing implicit parameters + else compareWithTypes(fullType1, fullType2) // continue by comparing implicit parameters } end compare diff --git a/tests/neg/scala-uri.check b/tests/neg/scala-uri.check new file mode 100644 index 000000000000..0e6544c186d8 --- /dev/null +++ b/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 value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A] */ + | summon[QueryValue[A]] + | ) + | ) + | + |But both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A]. diff --git a/tests/neg/scala-uri.scala b/tests/neg/scala-uri.scala new file mode 100644 index 000000000000..a169cd92f0a0 --- /dev/null +++ b/tests/neg/scala-uri.scala @@ -0,0 +1,30 @@ +import scala.language.implicitConversions + +trait QueryKey[A] +object QueryKey extends QueryKeyInstances +sealed trait QueryKeyInstances: + implicit val stringQueryKey: QueryKey[String] = ??? + +trait QueryValue[-A] +object QueryValue extends QueryValueInstances +sealed trait QueryValueInstances1: + implicit final val stringQueryValue: QueryValue[String] = ??? + implicit final val noneQueryValue: QueryValue[None.type] = ??? + // The noneQueryValue makes no sense at this priority. Since QueryValue + // is contravariant, QueryValue[None.type] is always better than QueryValue[Option[A]] + // no matter whether it's old or new resolution. So taking both owner and type + // score into account, it's always a draw. With the new disambiguation, we prefer + // the optionQueryValue[A], which gives an ambiguity down the road, because we don't + // know what the wrapped type A is. Previously, we preferred QueryValue[None.type] + // because it is unconditional. The solution is to put QueryValue[None.type] in the + // same trait as QueryValue[Option[A]], as is shown in pos/scala-uri.scala. + +sealed trait QueryValueInstances extends QueryValueInstances1: + implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? + +trait QueryKeyValue[A] +object QueryKeyValue: + implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? + + +@main def Test = summon[QueryKeyValue[(String, None.type)]] // error diff --git a/tests/pos/scala-uri.scala b/tests/pos/scala-uri.scala index 1adc50c8a6ab..42fa44854fb4 100644 --- a/tests/pos/scala-uri.scala +++ b/tests/pos/scala-uri.scala @@ -9,10 +9,10 @@ trait QueryValue[-A] object QueryValue extends QueryValueInstances sealed trait QueryValueInstances1: implicit final val stringQueryValue: QueryValue[String] = ??? - implicit final val noneQueryValue: QueryValue[None.type] = ??? sealed trait QueryValueInstances extends QueryValueInstances1: implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? + implicit final val noneQueryValue: QueryValue[None.type] = ??? trait QueryKeyValue[A] object QueryKeyValue: @@ -20,10 +20,3 @@ object QueryKeyValue: @main def Test = summon[QueryKeyValue[(String, None.type)]] - -/*(using - QueryKeyValue.tuple2QueryKeyValue[String, None.type]( - QueryKey.stringQueryKey, - QueryValue.optionQueryValue[A]))*/ - - // error \ No newline at end of file diff --git a/tests/pos/slick-migration-api-example.scala b/tests/pos/slick-migration-api-example.scala new file mode 100644 index 000000000000..3b6f1b4a82f4 --- /dev/null +++ b/tests/pos/slick-migration-api-example.scala @@ -0,0 +1,23 @@ +trait Migration +object Migration: + implicit class MigrationConcat[M <: Migration](m: M): + def &[N <: Migration, O](n: N)(implicit ccm: CanConcatMigrations[M, N, O]): O = ??? + +trait ReversibleMigration extends Migration +trait MigrationSeq extends Migration +trait ReversibleMigrationSeq extends MigrationSeq with ReversibleMigration + +trait ToReversible[-A <: Migration] +object ToReversible: + implicit val reversible: ToReversible[ReversibleMigration] = ??? +class CanConcatMigrations[-A, -B, +C] +trait CanConcatMigrationsLow: + implicit def default[A <: Migration, B <: Migration]: CanConcatMigrations[A, B, MigrationSeq] = ??? +object CanConcatMigrations extends CanConcatMigrationsLow: + implicit def reversible[A <: Migration, B <: Migration](implicit reverseA: ToReversible[A], + reverseB: ToReversible[B]): CanConcatMigrations[A, B, ReversibleMigrationSeq] = ??? + +@main def Test = + val rm: ReversibleMigration = ??? + val rms = rm & rm & rm + summon[rms.type <:< ReversibleMigrationSeq] // error Cannot prove that (rms : slick.migration.api.MigrationSeq) <:< slick.migration.api.ReversibleMigrationSeq. \ No newline at end of file From 62acaf14b7c008fb13138cbdd041dc31341175be Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 2 Aug 2024 20:24:05 +0200 Subject: [PATCH 7/7] A tweak of a tweak about owner scope disambiguation --- .../dotty/tools/dotc/typer/Applications.scala | 21 ++++-- tests/neg/scala-uri.check | 4 +- tests/neg/scala-uri.scala | 10 +-- tests/pos/i21320.scala | 73 +++++++++++++++++++ tests/pos/scala-uri.scala | 4 +- 5 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 tests/pos/i21320.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index ef98c33d5221..1d7df0b9d85a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1963,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) @@ -1976,13 +1979,14 @@ trait Applications extends Compatibility { // (prefer the one that is not a method, but that's arbitrary). if alt1.widenExpr =:= alt2 then -1 else 1 else - // For implicit resolution, take ownerscore as more significant than type resolution + // 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 preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) - then ownerScore - else 0 + 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 @@ -2002,6 +2006,13 @@ trait Applications extends Compatibility { 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 diff --git a/tests/neg/scala-uri.check b/tests/neg/scala-uri.check index 0e6544c186d8..91bcd7ab6a6c 100644 --- a/tests/neg/scala-uri.check +++ b/tests/neg/scala-uri.check @@ -6,9 +6,9 @@ | | QueryKeyValue.tuple2QueryKeyValue[String, None.type](QueryKey.stringQueryKey, | QueryValue.optionQueryValue[A]( - | /* ambiguous: both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[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 value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A]. + |But both given instance stringQueryValue in trait QueryValueInstances1 and given instance noneQueryValue in trait QueryValueInstances1 match type QueryValue[A]. diff --git a/tests/neg/scala-uri.scala b/tests/neg/scala-uri.scala index a169cd92f0a0..3820f8cf5613 100644 --- a/tests/neg/scala-uri.scala +++ b/tests/neg/scala-uri.scala @@ -3,13 +3,13 @@ import scala.language.implicitConversions trait QueryKey[A] object QueryKey extends QueryKeyInstances sealed trait QueryKeyInstances: - implicit val stringQueryKey: QueryKey[String] = ??? + given stringQueryKey: QueryKey[String] = ??? trait QueryValue[-A] object QueryValue extends QueryValueInstances sealed trait QueryValueInstances1: - implicit final val stringQueryValue: QueryValue[String] = ??? - implicit final val noneQueryValue: QueryValue[None.type] = ??? + given stringQueryValue: QueryValue[String] = ??? + given noneQueryValue: QueryValue[None.type] = ??? // The noneQueryValue makes no sense at this priority. Since QueryValue // is contravariant, QueryValue[None.type] is always better than QueryValue[Option[A]] // no matter whether it's old or new resolution. So taking both owner and type @@ -20,11 +20,11 @@ sealed trait QueryValueInstances1: // same trait as QueryValue[Option[A]], as is shown in pos/scala-uri.scala. sealed trait QueryValueInstances extends QueryValueInstances1: - implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? + given optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? trait QueryKeyValue[A] object QueryKeyValue: - implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? + given tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? @main def Test = summon[QueryKeyValue[(String, None.type)]] // error diff --git a/tests/pos/i21320.scala b/tests/pos/i21320.scala new file mode 100644 index 000000000000..0af346028482 --- /dev/null +++ b/tests/pos/i21320.scala @@ -0,0 +1,73 @@ +import scala.deriving.* +import scala.compiletime.* + +trait ConfigMonoid[T]: + def zero: T + def orElse(main: T, defaults: T): T + +object ConfigMonoid: + given option[T]: ConfigMonoid[Option[T]] = ??? + + inline def zeroTuple[C <: Tuple]: Tuple = + inline erasedValue[C] match + case _: EmptyTuple => EmptyTuple + case _: (t *: ts) => + summonInline[ConfigMonoid[t]].zero *: zeroTuple[ts] + + inline def valueTuple[C <: Tuple, T](index: Int, main: T, defaults: T): Tuple = + inline erasedValue[C] match + case _: EmptyTuple => EmptyTuple + case _: (t *: ts) => + def get(v: T) = v.asInstanceOf[Product].productElement(index).asInstanceOf[t] + summonInline[ConfigMonoid[t]].orElse(get(main), get(defaults)) *: valueTuple[ts, T]( + index + 1, + main, + defaults + ) + + inline given derive[T](using m: Mirror.ProductOf[T]): ConfigMonoid[T] = + new ConfigMonoid[T]: + def zero: T = m.fromProduct(zeroTuple[m.MirroredElemTypes]) + def orElse(main: T, defaults: T): T = m.fromProduct(valueTuple[m.MirroredElemTypes, T](0, main, defaults)) + + + +final case class PublishOptions( + v1: Option[String] = None, + v2: Option[String] = None, + v3: Option[String] = None, + v4: Option[String] = None, + v5: Option[String] = None, + v6: Option[String] = None, + v7: Option[String] = None, + v8: Option[String] = None, + v9: Option[String] = None, + ci: PublishContextualOptions = PublishContextualOptions(), +) +object PublishOptions: + implicit val monoid: ConfigMonoid[PublishOptions] = ConfigMonoid.derive + +final case class PublishContextualOptions( + v1: Option[String] = None, + v2: Option[String] = None, + v3: Option[String] = None, + v4: Option[String] = None, + v5: Option[String] = None, + v6: Option[String] = None, + v7: Option[String] = None, + v8: Option[String] = None, + v9: Option[String] = None, + v10: Option[String] = None, + v11: Option[String] = None, + v12: Option[String] = None, + v13: Option[String] = None, + v14: Option[String] = None, + v15: Option[String] = None, + v16: Option[String] = None, + v17: Option[String] = None, + v18: Option[String] = None, + v19: Option[String] = None, + v20: Option[String] = None +) +object PublishContextualOptions: + implicit val monoid: ConfigMonoid[PublishContextualOptions] = ConfigMonoid.derive \ No newline at end of file diff --git a/tests/pos/scala-uri.scala b/tests/pos/scala-uri.scala index 42fa44854fb4..75ea2fc70d8a 100644 --- a/tests/pos/scala-uri.scala +++ b/tests/pos/scala-uri.scala @@ -1,3 +1,4 @@ +// This works for implicit/implicit pairs but not for givens, see neg version. import scala.language.implicitConversions trait QueryKey[A] @@ -9,14 +10,13 @@ trait QueryValue[-A] object QueryValue extends QueryValueInstances sealed trait QueryValueInstances1: implicit final val stringQueryValue: QueryValue[String] = ??? + implicit final val noneQueryValue: QueryValue[None.type] = ??? sealed trait QueryValueInstances extends QueryValueInstances1: implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? - implicit final val noneQueryValue: QueryValue[None.type] = ??? trait QueryKeyValue[A] object QueryKeyValue: implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? - @main def Test = summon[QueryKeyValue[(String, None.type)]]