From 32f5200725b9f282e86a313ca65c4eee71fdeb27 Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Thu, 18 Jul 2024 18:35:43 +0200 Subject: [PATCH 1/9] Prefer extensions over conversions and implicits for member selection Before the changes, if `isAsGoodValueType` was called with an extension and a given conversion, it would prefer the conversion over the extension, because only the former yielded true in `isGiven`. Which contradicted the logic from searchImplicit which preferred extension over conversions for member selection. --- .../src/dotty/tools/dotc/typer/Applications.scala | 14 ++++++-------- tests/pos/i19715.scala | 3 ++- 2 files 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 c3369ac58e31..114372f1fa59 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1816,10 +1816,8 @@ 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)) + 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) @@ -1856,7 +1854,7 @@ 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 a given and the other is an implicit, the given wins. + * 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. @@ -1864,7 +1862,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, alt1isGiven: Boolean, alt2isGiven: Boolean)(using Context): Boolean = + def isAsGoodValueType(tp1: Type, tp2: Type, alt1IsImplicit: Boolean, alt2IsImplicit: Boolean)(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) @@ -1882,7 +1880,7 @@ trait Applications extends Compatibility { if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) || oldResolution - || !alt1isGiven && !alt2isGiven + || alt1IsImplicit && alt2IsImplicit 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, @@ -1897,8 +1895,8 @@ 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 + // New rules: better means generalize, givens (and extensions) always beat implicits + if alt1IsImplicit != alt2IsImplicit then alt2IsImplicit else (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1) end isAsGoodValueType diff --git a/tests/pos/i19715.scala b/tests/pos/i19715.scala index 91aeda5c1698..be5471ffa9b3 100644 --- a/tests/pos/i19715.scala +++ b/tests/pos/i19715.scala @@ -6,7 +6,8 @@ class NT(t: Tup): object NT: extension (x: NT) def app(n: Int): Boolean = true - given Conversion[NT, Tup] = _.toTup + given c1: Conversion[NT, Tup] = _.toTup + implicit def c2(t: NT): Tup = c1(t) def test = val nt = new NT(Tup()) From 1f554d54135460f0b4ce2e0f5ecd8ba3ade42718 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 11:28:18 +0200 Subject: [PATCH 2/9] A left-biased variant for implicit/given pairs We now use a left-biased scheme, as follows. From 3.6 on: - A given x: X is better than a given or implicit y: Y if y can be instantiated/widened to X. - An implicit x: X is better than a given or implicit y: Y if y can be instantiated to a supertype of X. - Use owner score for givens as a tie breaker if after all other tests we still have an ambiguity. This is not transitive, so we need a separate scheme to work around that. Other change: - Drop special handling of NotGiven in prioritization. The previous logic pretended to do so, but was ineffective. --- .../tools/dotc/printing/Formatting.scala | 17 ++--- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../dotty/tools/dotc/typer/Applications.scala | 65 +++++++++++------ .../dotty/tools/dotc/typer/Implicits.scala | 39 ++++++++-- .../tools/dotc/StringFormatterTest.scala | 1 + tests/neg/given-triangle.check | 8 ++ tests/neg/i21212.check | 4 + tests/neg/i21212.scala | 11 +++ tests/neg/i21303/JavaEnum.java | 1 + tests/neg/i21303/Test.scala | 33 +++++++++ tests/neg/i2974.scala | 16 ++++ tests/neg/scala-uri.check | 14 ++++ tests/neg/scala-uri.scala | 30 ++++++++ tests/pos/given-priority.scala | 24 ++++++ tests/pos/i21212.scala | 22 ++++++ tests/pos/i21303/JavaEnum.java | 1 + tests/pos/i21303/Test.scala | 32 ++++++++ tests/pos/i21303a/JavaEnum.java | 1 + tests/pos/i21303a/Test.scala | 35 +++++++++ tests/pos/i21320.scala | 73 +++++++++++++++++++ tests/pos/i2974.scala | 3 +- tests/pos/scala-uri.scala | 22 ++++++ tests/pos/slick-migration-api-example.scala | 23 ++++++ tests/warn/i21036a.check | 6 +- tests/warn/i21036b.check | 6 +- 25 files changed, 445 insertions(+), 44 deletions(-) create mode 100644 tests/neg/i21212.check create mode 100644 tests/neg/i21212.scala create mode 100644 tests/neg/i21303/JavaEnum.java create mode 100644 tests/neg/i21303/Test.scala create mode 100644 tests/neg/i2974.scala create mode 100644 tests/neg/scala-uri.check create mode 100644 tests/neg/scala-uri.scala create mode 100644 tests/pos/given-priority.scala create mode 100644 tests/pos/i21212.scala create mode 100644 tests/pos/i21303/JavaEnum.java create mode 100644 tests/pos/i21303/Test.scala create mode 100644 tests/pos/i21303a/JavaEnum.java create mode 100644 tests/pos/i21303a/Test.scala create mode 100644 tests/pos/i21320.scala create mode 100644 tests/pos/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 6f1c32beb822..43cac17e6318 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.* @@ -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 => diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d33b2c574318..3e60ab95859a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2985,7 +2985,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/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 114372f1fa59..2efe5282f025 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -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.4, 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 @@ -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.4`) + || 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` ? @@ -1816,15 +1836,15 @@ 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)) + 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`. @@ -1862,9 +1882,8 @@ 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 = - 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) @@ -1878,13 +1897,7 @@ trait Applications extends Compatibility { val tp1p = prepare(tp1) val tp2p = prepare(tp2) - if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) - || oldResolution - || alt1IsImplicit && alt2IsImplicit - 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) => @@ -1895,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 (and extensions) always beat implicits - if alt1IsImplicit != alt2IsImplicit then alt2IsImplicit - 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 @@ -1968,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 diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index dac0c0e78448..32a95ae501f0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/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,7 +1335,7 @@ 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()) + val cmp = comp(using searchContext()) val sv = Feature.sourceVersion if isWarnPriorityChangeVersion(sv) then val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution)) @@ -1345,13 +1350,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 +1623,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 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/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/i21212.check b/tests/neg/i21212.check new file mode 100644 index 000000000000..5d9fe7728cbc --- /dev/null +++ b/tests/neg/i21212.check @@ -0,0 +1,4 @@ +-- [E172] Type Error: tests/neg/i21212.scala:8:52 ---------------------------------------------------------------------- +8 | def test2(using a2: A)(implicit b2: B) = summon[A] // error: ambiguous + | ^ + |Ambiguous given instances: both parameter b2 and parameter a2 match type Minimization.A of parameter x of method summon in object Predef diff --git a/tests/neg/i21212.scala b/tests/neg/i21212.scala new file mode 100644 index 000000000000..389a82b19f1f --- /dev/null +++ b/tests/neg/i21212.scala @@ -0,0 +1,11 @@ +//> using options -source:3.6 +object Minimization: + + trait A + trait B extends A + + def test1(using a1: A)(using b1: B) = summon[A] // picks (most general) a1 + def test2(using a2: A)(implicit b2: B) = summon[A] // error: ambiguous + def test3(implicit a3: A, b3: B) = summon[A] // picks (most specific) b3 + +end Minimization 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/neg/i2974.scala b/tests/neg/i2974.scala new file mode 100644 index 000000000000..0bff2da1f3ba --- /dev/null +++ b/tests/neg/i2974.scala @@ -0,0 +1,16 @@ + +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]] // error: ambiguous +} diff --git a/tests/neg/scala-uri.check b/tests/neg/scala-uri.check new file mode 100644 index 000000000000..91bcd7ab6a6c --- /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 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]. diff --git a/tests/neg/scala-uri.scala b/tests/neg/scala-uri.scala new file mode 100644 index 000000000000..3820f8cf5613 --- /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: + given stringQueryKey: QueryKey[String] = ??? + +trait QueryValue[-A] +object QueryValue extends QueryValueInstances +sealed trait QueryValueInstances1: + 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 + // 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: + given optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ??? + +trait QueryKeyValue[A] +object QueryKeyValue: + given tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ??? + + +@main def Test = summon[QueryKeyValue[(String, None.type)]] // error 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/i21212.scala b/tests/pos/i21212.scala new file mode 100644 index 000000000000..1a1f2e35819a --- /dev/null +++ b/tests/pos/i21212.scala @@ -0,0 +1,22 @@ + +trait Functor[F[_]]: + def map[A, B](fa: F[A])(f: A => B): F[B] = ??? +trait Monad[F[_]] extends Functor[F] +trait MonadError[F[_], E] extends Monad[F]: + def raiseError[A](e: E): F[A] +trait Temporal[F[_]] extends MonadError[F, Throwable] + +trait FunctorOps[F[_], A]: + def map[B](f: A => B): F[B] = ??? +implicit def toFunctorOps[F[_], A](target: F[A])(implicit tc: Functor[F]): FunctorOps[F, A] = ??? + +class ContextBounds[F[_]: Temporal](using err: MonadError[F, Throwable]): + def useCase = err.raiseError(new RuntimeException()) + val bool: F[Boolean] = ??? + def fails = toFunctorOps(bool).map(_ => ()) // warns under -source:3.5, // error under -source:3.6 + +class UsingArguments[F[_]](using Temporal[F])(using err: MonadError[F, Throwable]): + def useCase = err.raiseError(new RuntimeException()) + val bool: F[Boolean] = ??? + def works = toFunctorOps(bool).map(_ => ()) // warns under -source:3.5 + 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/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]] diff --git a/tests/pos/i21320.scala b/tests/pos/i21320.scala new file mode 100644 index 000000000000..0a7e0d1941d1 --- /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: + given monoid: ConfigMonoid[PublishContextualOptions] = ConfigMonoid.derive \ No newline at end of file diff --git a/tests/pos/i2974.scala b/tests/pos/i2974.scala index 75c6a24a41bb..8f1c2e2d6d2f 100644 --- a/tests/pos/i2974.scala +++ b/tests/pos/i2974.scala @@ -7,6 +7,7 @@ object Test { implicit val ba: Bar[Int] = ??? def test: Unit = { - implicitly[Foo[Int]] + val x = summon[Foo[Int]] + val _: Bar[Int] = x } } diff --git a/tests/pos/scala-uri.scala b/tests/pos/scala-uri.scala new file mode 100644 index 000000000000..75ea2fc70d8a --- /dev/null +++ b/tests/pos/scala-uri.scala @@ -0,0 +1,22 @@ +// This works for implicit/implicit pairs but not for givens, see neg version. +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)]] 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 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 069afba906df3ab01471b80050538fcc1974b420 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 13:48:25 +0200 Subject: [PATCH 3/9] Compensate loss of transitivity We only have transitivity between givens or between implicits. To cope with that - We tank first all implicits, giving a best implicit search result. - Then we rank all givens startign with the implicit result. If there is a given that is better than the best implicit, the best given will be chosen. Otherwise we will stick with the best implicit. --- .../src/dotty/tools/dotc/typer/Implicits.scala | 18 +++++++++++++++--- tests/pos/given-owner-disambiguate.scala | 13 +++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 tests/pos/given-owner-disambiguate.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 32a95ae501f0..7aa88e1e582a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1381,8 +1381,6 @@ trait Implicits: def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match case alt1: SearchSuccess => var diff = compareAlternatives(alt1, alt2, disambiguate = true) - assert(diff <= 0 || isWarnPriorityChangeVersion(Feature.sourceVersion)) - // diff > 0 candidates should already have been eliminated in `rank` if diff == 0 && alt1.ref =:= alt2.ref then diff = 1 // See i12951 for a test where this happens else if diff == 0 && alt2.isExtension then @@ -1622,7 +1620,21 @@ trait Implicits: validateOrdering(ord) throw ex - val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + val sorted = sort(eligible) + val result = sorted match + case first :: rest => + val firstIsImplicit = first.ref.symbol.is(Implicit) + if rest.exists(_.ref.symbol.is(Implicit) != firstIsImplicit) then + // Mixture of implicits and givens + // Rank implicits first, then, if there is a given that it better than the best implicit(s) + // switch over to givens. + val (sortedImplicits, sortedGivens) = sorted.partition(_.ref.symbol.is(Implicit)) + val implicitResult = rank(sortedImplicits, NoMatchingImplicitsFailure, Nil) + rank(sortedGivens, implicitResult, Nil) + else + rank(sorted, NoMatchingImplicitsFailure, Nil) + case _ => + NoMatchingImplicitsFailure // Issue all priority change warnings that can affect the result val shownWarnings = priorityChangeWarnings.toList.collect: diff --git a/tests/pos/given-owner-disambiguate.scala b/tests/pos/given-owner-disambiguate.scala new file mode 100644 index 000000000000..f0a44ecc441a --- /dev/null +++ b/tests/pos/given-owner-disambiguate.scala @@ -0,0 +1,13 @@ +class General +class Specific extends General + +class LowPriority: + given a:General() + +object NormalPriority extends LowPriority: + given b:Specific() + +def run = + import NormalPriority.given + val x = summon[General] + val _: Specific = x // <- b was picked \ No newline at end of file From 69ca72b8d5de456ea418f34a7d5b9fe3317da657 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 18:14:37 +0200 Subject: [PATCH 4/9] Delay priority change until 3.7 Warnings from 3.6, change in 3.7. This is one version later than originally planned. --- .../tools/dotc/config/SourceVersion.scala | 1 + .../dotty/tools/dotc/typer/Applications.scala | 14 +++++++------- .../src/dotty/tools/dotc/typer/Implicits.scala | 18 +++++++++--------- .../scala/runtime/stdLibPatches/language.scala | 15 +++++++++++++++ tests/neg/given-triangle.check | 2 +- tests/neg/given-triangle.scala | 2 +- tests/neg/i15264.scala | 2 +- tests/neg/i21212.check | 4 ++-- tests/neg/i21212.scala | 3 ++- tests/neg/i21303/Test.scala | 2 +- tests/pos/source-import-3-7-migration.scala | 1 + tests/pos/source-import-3-7.scala | 1 + tests/run/given-triangle.scala | 2 +- tests/run/implicit-specifity.scala | 2 +- tests/run/implied-priority.scala | 2 +- tests/warn/i20420.scala | 2 +- tests/warn/i21036a.check | 2 +- tests/warn/i21036a.scala | 2 +- tests/warn/i21036b.check | 2 +- tests/warn/i21036b.scala | 2 +- 20 files changed, 50 insertions(+), 31 deletions(-) create mode 100644 tests/pos/source-import-3-7-migration.scala create mode 100644 tests/pos/source-import-3-7.scala diff --git a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala index 935b95003729..02140c3f4e3b 100644 --- a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala +++ b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala @@ -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` diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 2efe5282f025..9a5db44b15ca 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1753,7 +1753,7 @@ trait Applications extends Compatibility { // 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.4, or if OldImplicitResolution + // 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 @@ -1789,7 +1789,7 @@ trait Applications extends Compatibility { val oldResolution = ctx.mode.is(Mode.OldImplicitResolution) if !preferGeneral || Feature.migrateTo3 && oldResolution then CompareScheme.Old - else if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) + else if Feature.sourceVersion.isAtMost(SourceVersion.`3.5`) || oldResolution || alt1.symbol.is(Implicit) && alt2.symbol.is(Implicit) then CompareScheme.Intermediate @@ -1855,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) @@ -1870,14 +1870,14 @@ 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. + * (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. diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 7aa88e1e582a..ebdca078d345 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1305,13 +1305,13 @@ trait Implicits: /** Search a list of eligible implicit references */ private def searchImplicit(eligible: List[Candidate], contextual: Boolean): SearchResult = - // A map that associates a priority change warning (between -source 3.4 and 3.6) + // A map that associates a priority change warning (between -source 3.6 and 3.7) // with the candidate refs mentioned in the warning. We report the associated // message if one of the critical candidates is part of the result of the implicit search. val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], Message)]() def isWarnPriorityChangeVersion(sv: SourceVersion): Boolean = - sv.stable == SourceVersion.`3.5` || sv == SourceVersion.`3.6-migration` + sv.stable == SourceVersion.`3.6` || sv == SourceVersion.`3.7-migration` /** Compare `alt1` with `alt2` to determine which one should be chosen. * @@ -1319,12 +1319,12 @@ trait Implicits: * a number < 0 if `alt2` is preferred over `alt1` * 0 if neither alternative is preferred over the other * The behavior depends on the source version - * before 3.5: compare with preferGeneral = false - * 3.5: compare twice with preferGeneral = false and true, warning if result is different, + * before 3.6: compare with preferGeneral = false + * 3.6: compare twice with preferGeneral = false and true, warning if result is different, * return old result with preferGeneral = false - * 3.6-migration: compare twice with preferGeneral = false and true, warning if result is different, + * 3.7-migration: compare twice with preferGeneral = false and true, warning if result is different, * return new result with preferGeneral = true - * 3.6 and higher: compare with preferGeneral = true + * 3.7 and higher: compare with preferGeneral = true * * @param disambiguate The call is used to disambiguate two successes, not for ranking. * When ranking, we are always filtering out either > 0 or <= 0 results. @@ -1348,7 +1348,7 @@ trait Implicits: case -1 => "the second alternative" case 1 => "the first alternative" case _ => "none - it's ambiguous" - if sv.stable == SourceVersion.`3.5` then + if sv.stable == SourceVersion.`3.6` then warn( em"""Given search preference for $pt between alternatives | ${alt1.ref} @@ -1356,7 +1356,7 @@ trait Implicits: | ${alt2.ref} |will change. |Current choice : ${choice(prev)} - |New choice from Scala 3.6: ${choice(cmp)}""") + |New choice from Scala 3.7: ${choice(cmp)}""") prev else warn( @@ -1366,7 +1366,7 @@ trait Implicits: | ${alt2.ref} |has changed. |Previous choice : ${choice(prev)} - |New choice from Scala 3.6: ${choice(cmp)}""") + |New choice from Scala 3.7: ${choice(cmp)}""") cmp else cmp max prev // When ranking, we keep the better of cmp and prev, which ends up retaining a candidate diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala index e930b81923ff..b2e7525b8b7b 100644 --- a/library/src/scala/runtime/stdLibPatches/language.scala +++ b/library/src/scala/runtime/stdLibPatches/language.scala @@ -300,6 +300,21 @@ object language: @compileTimeOnly("`3.6` can only be used at compile time in import statements") object `3.6` + /** Set source version to 3.7-migration. + * + * @see [[https://docs.scala-lang.org/scala3/guides/migration/compatibility-intro.html]] + */ + @compileTimeOnly("`3.7-migration` can only be used at compile time in import statements") + object `3.7-migration` + + /** Set source version to 3.7 + * + * @see [[https://docs.scala-lang.org/scala3/guides/migration/compatibility-intro.html]] + */ + @compileTimeOnly("`3.7` can only be used at compile time in import statements") + object `3.7` + + // !!! Keep in sync with dotty.tools.dotc.config.SourceVersion !!! // Also add tests in `tests/pos/source-import-3-x.scala` and `tests/pos/source-import-3-x-migration.scala` diff --git a/tests/neg/given-triangle.check b/tests/neg/given-triangle.check index 73d5aea12dc4..147c54270afb 100644 --- a/tests/neg/given-triangle.check +++ b/tests/neg/given-triangle.check @@ -9,4 +9,4 @@ | (given_B : B) |will change. |Current choice : the second alternative - |New choice from Scala 3.6: the first alternative + |New choice from Scala 3.7: the first alternative diff --git a/tests/neg/given-triangle.scala b/tests/neg/given-triangle.scala index 16aca7c44dee..4842c5314f51 100644 --- a/tests/neg/given-triangle.scala +++ b/tests/neg/given-triangle.scala @@ -1,4 +1,4 @@ -//> using options -source 3.5 +//> using options -source 3.6 class A class B extends A class C extends A diff --git a/tests/neg/i15264.scala b/tests/neg/i15264.scala index 825e74701f73..d690eccf23f3 100644 --- a/tests/neg/i15264.scala +++ b/tests/neg/i15264.scala @@ -1,4 +1,4 @@ -import language.`3.6` +import language.`3.7` object priority: // lower number = higher priority class Prio0 extends Prio1 diff --git a/tests/neg/i21212.check b/tests/neg/i21212.check index 5d9fe7728cbc..06740af36d77 100644 --- a/tests/neg/i21212.check +++ b/tests/neg/i21212.check @@ -1,4 +1,4 @@ --- [E172] Type Error: tests/neg/i21212.scala:8:52 ---------------------------------------------------------------------- -8 | def test2(using a2: A)(implicit b2: B) = summon[A] // error: ambiguous +-- [E172] Type Error: tests/neg/i21212.scala:9:52 ---------------------------------------------------------------------- +9 | def test2(using a2: A)(implicit b2: B) = summon[A] // error: ambiguous | ^ |Ambiguous given instances: both parameter b2 and parameter a2 match type Minimization.A of parameter x of method summon in object Predef diff --git a/tests/neg/i21212.scala b/tests/neg/i21212.scala index 389a82b19f1f..3b030cefcdc7 100644 --- a/tests/neg/i21212.scala +++ b/tests/neg/i21212.scala @@ -1,4 +1,5 @@ -//> using options -source:3.6 +//> using options -source 3.7 + object Minimization: trait A diff --git a/tests/neg/i21303/Test.scala b/tests/neg/i21303/Test.scala index fa8058140067..25d43dac344e 100644 --- a/tests/neg/i21303/Test.scala +++ b/tests/neg/i21303/Test.scala @@ -1,4 +1,4 @@ -//> using options -source 3.6-migration +//> using options -source 3.7-migration import scala.deriving.Mirror import scala.compiletime.* import scala.reflect.ClassTag diff --git a/tests/pos/source-import-3-7-migration.scala b/tests/pos/source-import-3-7-migration.scala new file mode 100644 index 000000000000..2e80fcb0bab2 --- /dev/null +++ b/tests/pos/source-import-3-7-migration.scala @@ -0,0 +1 @@ +import language.`3.7-migration` \ No newline at end of file diff --git a/tests/pos/source-import-3-7.scala b/tests/pos/source-import-3-7.scala new file mode 100644 index 000000000000..7fa68fd496f6 --- /dev/null +++ b/tests/pos/source-import-3-7.scala @@ -0,0 +1 @@ +import language.`3.7` \ No newline at end of file diff --git a/tests/run/given-triangle.scala b/tests/run/given-triangle.scala index 0b483e87f28c..66339f44e43c 100644 --- a/tests/run/given-triangle.scala +++ b/tests/run/given-triangle.scala @@ -1,4 +1,4 @@ -import language.`3.6` +import language.`3.7` class A class B extends A diff --git a/tests/run/implicit-specifity.scala b/tests/run/implicit-specifity.scala index da90110c9866..9e59cf5f1869 100644 --- a/tests/run/implicit-specifity.scala +++ b/tests/run/implicit-specifity.scala @@ -1,4 +1,4 @@ -import language.`3.6` +import language.`3.7` case class Show[T](val i: Int) object Show { diff --git a/tests/run/implied-priority.scala b/tests/run/implied-priority.scala index 15f6a40a27ef..a9380e117875 100644 --- a/tests/run/implied-priority.scala +++ b/tests/run/implied-priority.scala @@ -1,6 +1,6 @@ /* These tests show various mechanisms available for implicit prioritization. */ -import language.`3.6` +import language.`3.7` class E[T](val str: String) // The type for which we infer terms below diff --git a/tests/warn/i20420.scala b/tests/warn/i20420.scala index d28270509f91..4c7585e32f48 100644 --- a/tests/warn/i20420.scala +++ b/tests/warn/i20420.scala @@ -1,4 +1,4 @@ -//> using options -source 3.5-migration +//> using options -source 3.6-migration final class StrictEqual[V] final class Less[V] diff --git a/tests/warn/i21036a.check b/tests/warn/i21036a.check index 876a81ad8a83..63d611a6e246 100644 --- a/tests/warn/i21036a.check +++ b/tests/warn/i21036a.check @@ -7,4 +7,4 @@ | (a : A) | will change. | Current choice : the first alternative - | New choice from Scala 3.6: the second alternative + | New choice from Scala 3.7: the second alternative diff --git a/tests/warn/i21036a.scala b/tests/warn/i21036a.scala index ab97429852d6..b7aba27ca95e 100644 --- a/tests/warn/i21036a.scala +++ b/tests/warn/i21036a.scala @@ -1,4 +1,4 @@ -//> using options -source 3.5 +//> using options -source 3.6 trait A trait B extends A given b: B = ??? diff --git a/tests/warn/i21036b.check b/tests/warn/i21036b.check index 11bb38727d77..dfa19a0e9bb1 100644 --- a/tests/warn/i21036b.check +++ b/tests/warn/i21036b.check @@ -7,4 +7,4 @@ | (a : A) | has changed. | Previous choice : the first alternative - | New choice from Scala 3.6: the second alternative + | New choice from Scala 3.7: the second alternative diff --git a/tests/warn/i21036b.scala b/tests/warn/i21036b.scala index 16dd72266613..c440f5d3c06d 100644 --- a/tests/warn/i21036b.scala +++ b/tests/warn/i21036b.scala @@ -1,4 +1,4 @@ -//> using options -source 3.6-migration +//> using options -source 3.7-migration trait A trait B extends A given b: B = ??? From 1ed92caa7cf97f75a9005e32776eea0359432799 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 6 Aug 2024 19:57:01 +0200 Subject: [PATCH 5/9] Fix ranking logic --- .../dotty/tools/dotc/typer/Implicits.scala | 31 +++++++--- tests/pos/i15264.scala | 1 + tests/warn/i15264.scala | 56 +++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 tests/warn/i15264.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ebdca078d345..b9f225f6a42a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1369,8 +1369,13 @@ trait Implicits: |New choice from Scala 3.7: ${choice(cmp)}""") cmp else cmp max prev - // When ranking, we keep the better of cmp and prev, which ends up retaining a candidate - // if it is retained in either version. + // When ranking, alt1 is always the new candidate and alt2 is the + // solution found previously. We keep the candidate if the outcome is 0 + // (ambiguous) or 1 (first wins). Or, when ranking in healImplicit we keep the + // candidate only if the outcome is 1. In both cases, keeping the better + // of `cmp` and `prev` means we keep candidates that could match + // in either scheme. This means that subsequent disambiguation + // comparisons will record a warning if cmp != prev. else cmp end compareAlternatives @@ -1416,7 +1421,15 @@ trait Implicits: if diff < 0 then alt2 else if diff > 0 then alt1 else SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument), span) - case _: SearchFailure => alt2 + case fail: SearchFailure => + fail.reason match + case ambi: AmbiguousImplicits => + if compareAlternatives(ambi.alt1, alt2) < 0 && + compareAlternatives(ambi.alt2, alt2) < 0 + then alt2 + else alt1 + case _ => + alt2 /** Try to find a best matching implicit term among all the candidates in `pending`. * @param pending The list of candidates that remain to be tested @@ -1621,7 +1634,7 @@ trait Implicits: throw ex val sorted = sort(eligible) - val result = sorted match + val res = sorted match case first :: rest => val firstIsImplicit = first.ref.symbol.is(Implicit) if rest.exists(_.ref.symbol.is(Implicit) != firstIsImplicit) then @@ -1638,11 +1651,11 @@ trait Implicits: // Issue all priority change warnings that can affect the result val shownWarnings = priorityChangeWarnings.toList.collect: - case (critical, msg) if result.found.exists(critical.contains(_)) => + case (critical, msg) if res.found.exists(critical.contains(_)) => msg - result match - case result: SearchFailure => - result.reason match + res match + case res: SearchFailure => + res.reason match case ambi: AmbiguousImplicits => // Make warnings part of error message because otherwise they are suppressed when // the error is emitted. @@ -1652,7 +1665,7 @@ trait Implicits: for msg <- shownWarnings do report.warning(msg, srcPos) - result + res end searchImplicit def isUnderSpecifiedArgument(tp: Type): Boolean = diff --git a/tests/pos/i15264.scala b/tests/pos/i15264.scala index 5be8436c12ba..18ca92df6cb1 100644 --- a/tests/pos/i15264.scala +++ b/tests/pos/i15264.scala @@ -1,3 +1,4 @@ +import language.`3.7` object priority: // lower number = higher priority class Prio0 extends Prio1 diff --git a/tests/warn/i15264.scala b/tests/warn/i15264.scala new file mode 100644 index 000000000000..9435c6364c08 --- /dev/null +++ b/tests/warn/i15264.scala @@ -0,0 +1,56 @@ +// Note: No check file for this test since the precise warning messages are non-deterministic +import language.`3.7-migration` +object priority: + // lower number = higher priority + class Prio0 extends Prio1 + object Prio0 { given Prio0() } + + class Prio1 extends Prio2 + object Prio1 { given Prio1() } + + class Prio2 + object Prio2 { given Prio2() } + +object repro: + // analogous to cats Eq, Hash, Order: + class A[V] + class B[V] extends A[V] + class C[V] extends A[V] + + class Q[V] + + object context: + // prios work here, which is cool + given[V](using priority.Prio0): C[V] = new C[V] + given[V](using priority.Prio1): B[V] = new B[V] + given[V](using priority.Prio2): A[V] = new A[V] + + object exports: + // so will these exports + export context.given + + // if you import these don't import from 'context' above + object qcontext: + // base defs, like what you would get from cats + given ga: A[Int] = new B[Int] // added so that we don't get an ambiguity in test2 + given gb: B[Int] = new B[Int] + given gc: C[Int] = new C[Int] + + // these seem like they should work but don't + given gcq[V](using p0: priority.Prio0)(using c: C[V]): C[Q[V]] = new C[Q[V]] + given gbq[V](using p1: priority.Prio1)(using b: B[V]): B[Q[V]] = new B[Q[V]] + given gaq[V](using p2: priority.Prio2)(using a: A[V]): A[Q[V]] = new A[Q[V]] + +object test1: + import repro.* + import repro.exports.given + + // these will work + val a = summon[A[Int]] // warn + + +object test2: + import repro.* + import repro.qcontext.given + + val a = summon[A[Q[Int]]] // warn From 5e90d91780dcddfe63c9b9e42c1c7c38812b94f5 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 6 Aug 2024 20:05:59 +0200 Subject: [PATCH 6/9] Make priority change warning messages stable Make the wording of a priority change warning message stable under different orders of eligibles. We now always report the previously chosen alternative first and the new one second. Note: We can still get ambiguities by fallging different pairs of alternatives depending on initial order. --- .../dotty/tools/dotc/typer/Implicits.scala | 66 +++++++++---------- tests/neg/given-triangle.check | 8 +-- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index b9f225f6a42a..a4fa989cc85c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1310,9 +1310,6 @@ trait Implicits: // message if one of the critical candidates is part of the result of the implicit search. val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], Message)]() - def isWarnPriorityChangeVersion(sv: SourceVersion): Boolean = - sv.stable == SourceVersion.`3.6` || sv == SourceVersion.`3.7-migration` - /** Compare `alt1` with `alt2` to determine which one should be chosen. * * @return a number > 0 if `alt1` is preferred over `alt2` @@ -1337,37 +1334,38 @@ trait Implicits: else val cmp = comp(using searchContext()) val sv = Feature.sourceVersion - if isWarnPriorityChangeVersion(sv) then + val isLastOldVersion = sv.stable == SourceVersion.`3.6` + val isMigratingVersion = sv == SourceVersion.`3.7-migration` + if isLastOldVersion || isMigratingVersion 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 - priorityChangeWarnings += ((critical, msg)) - implicits.println(i"PRIORITY CHANGE ${alt1.ref}, ${alt2.ref}, $disambiguate") - def choice(c: Int) = c match - case -1 => "the second alternative" - case 1 => "the first alternative" - case _ => "none - it's ambiguous" - if sv.stable == SourceVersion.`3.6` then - warn( - em"""Given search preference for $pt between alternatives - | ${alt1.ref} - |and - | ${alt2.ref} - |will change. - |Current choice : ${choice(prev)} - |New choice from Scala 3.7: ${choice(cmp)}""") - prev - else - warn( - em"""Given search preference for $pt between alternatives - | ${alt1.ref} - |and - | ${alt2.ref} - |has changed. - |Previous choice : ${choice(prev)} - |New choice from Scala 3.7: ${choice(cmp)}""") - cmp + implicits.println(i"PRIORITY CHANGE ${alt1.ref}, ${alt2.ref}") + val (loser, winner) = + prev match + case 1 => (alt1, alt2) + case -1 => (alt2, alt1) + case 0 => + cmp match + case 1 => (alt2, alt1) + case -1 => (alt1, alt2) + def choice(nth: String, c: Int) = + if c == 0 then "none - it's ambiguous" + else s"the $nth alternative" + val (change, whichChoice) = + if isLastOldVersion + then ("will change", "Current choice ") + else ("has changed", "Previous choice") + val msg = + em"""Given search preference for $pt between alternatives + | ${loser.ref} + |and + | ${winner.ref} + |$change. + |$whichChoice : ${choice("first", prev)} + |New choice from Scala 3.7: ${choice("second", cmp)}""" + val critical = alt1.ref :: alt2.ref :: Nil + priorityChangeWarnings += ((critical, msg)) + if isLastOldVersion then prev else cmp else cmp max prev // When ranking, alt1 is always the new candidate and alt2 is the // solution found previously. We keep the candidate if the outcome is 0 @@ -1424,8 +1422,8 @@ trait Implicits: case fail: SearchFailure => fail.reason match case ambi: AmbiguousImplicits => - if compareAlternatives(ambi.alt1, alt2) < 0 && - compareAlternatives(ambi.alt2, alt2) < 0 + if compareAlternatives(ambi.alt1, alt2, disambiguate = true) < 0 + && compareAlternatives(ambi.alt2, alt2, disambiguate = true) < 0 then alt2 else alt1 case _ => diff --git a/tests/neg/given-triangle.check b/tests/neg/given-triangle.check index 147c54270afb..f366c18e78f0 100644 --- a/tests/neg/given-triangle.check +++ b/tests/neg/given-triangle.check @@ -4,9 +4,9 @@ |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) + |and + | (given_A : A) |will change. - |Current choice : the second alternative - |New choice from Scala 3.7: the first alternative + |Current choice : the first alternative + |New choice from Scala 3.7: the second alternative From 127d64bc6e0f1cf826dc126eeffae0e252975c46 Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Tue, 6 Aug 2024 23:42:07 +0200 Subject: [PATCH 7/9] Fix `healAmbiguous` to `compareAlternatives` with `disambiguate = true` On the final result, compared with all the ambiguous candidates we are trying to recover from. We should still use `disambiguate = false` when filtering the `pending` candidates for the purpose of warnings, as in the other cases. Before the changes, it was possible for an ambiguous SearchFailure to be healed by a candidate which was considered better (possibly only) under a prioritization scheme different from the current one. As an optimization, we can avoid redoing compareAlternatives in versions which could have only used the new prioritization scheme to begin with. Also restores behaviour avoiding false positive warnings. Specifically, in cases where we could report a change in prioritization, despite having not yet done `tryImplicit` on the alternative, i.e. it was only compared as part of an early filtering See #21045 for related changes --- .../dotty/tools/dotc/typer/Implicits.scala | 49 ++++++++++--------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index a4fa989cc85c..5e9575903895 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1310,6 +1310,10 @@ trait Implicits: // message if one of the critical candidates is part of the result of the implicit search. val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], Message)]() + val sv = Feature.sourceVersion + val isLastOldVersion = sv.stable == SourceVersion.`3.6` + val isWarnPriorityChangeVersion = isLastOldVersion || sv == SourceVersion.`3.7-migration` + /** Compare `alt1` with `alt2` to determine which one should be chosen. * * @return a number > 0 if `alt1` is preferred over `alt2` @@ -1333,10 +1337,7 @@ trait Implicits: else if alt1.level != alt2.level then alt1.level - alt2.level else val cmp = comp(using searchContext()) - val sv = Feature.sourceVersion - val isLastOldVersion = sv.stable == SourceVersion.`3.6` - val isMigratingVersion = sv == SourceVersion.`3.7-migration` - if isLastOldVersion || isMigratingVersion then + if isWarnPriorityChangeVersion then val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution)) if disambiguate && cmp != prev then implicits.println(i"PRIORITY CHANGE ${alt1.ref}, ${alt2.ref}") @@ -1419,15 +1420,7 @@ trait Implicits: if diff < 0 then alt2 else if diff > 0 then alt1 else SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument), span) - case fail: SearchFailure => - fail.reason match - case ambi: AmbiguousImplicits => - if compareAlternatives(ambi.alt1, alt2, disambiguate = true) < 0 - && compareAlternatives(ambi.alt2, alt2, disambiguate = true) < 0 - then alt2 - else alt1 - case _ => - alt2 + case _: SearchFailure => alt2 /** Try to find a best matching implicit term among all the candidates in `pending`. * @param pending The list of candidates that remain to be tested @@ -1451,12 +1444,27 @@ trait Implicits: pending match { case cand :: remaining => /** To recover from an ambiguous implicit failure, we need to find a pending - * candidate that is strictly better than the failed candidate(s). + * candidate that is strictly better than the failed `ambiguous` candidate(s). * If no such candidate is found, we propagate the ambiguity. */ - def healAmbiguous(fail: SearchFailure, betterThanFailed: Candidate => Boolean) = - val newPending = remaining.filter(betterThanFailed) - rank(newPending, fail, Nil).recoverWith(_ => fail) + def healAmbiguous(fail: SearchFailure, ambiguous: List[RefAndLevel]) = + def betterThanAmbiguous(newCand: RefAndLevel, disambiguate: Boolean): Boolean = + ambiguous.forall(compareAlternatives(newCand, _, disambiguate) > 0) + + inline def betterByCurrentScheme(newCand: RefAndLevel): Boolean = + if isWarnPriorityChangeVersion then + // newCand may have only been kept in pending because it was better in the other priotization scheme. + // If that candidate produces a SearchSuccess, disambiguate will return it as the found SearchResult. + // We must now recheck it was really better than the ambigous candidates we are recovering from, + // under the rules of the current scheme, which are applied when disambiguate = true. + betterThanAmbiguous(newCand, disambiguate = true) + else true + + val newPending = remaining.filter(betterThanAmbiguous(_, disambiguate = false)) + rank(newPending, fail, Nil) match + case found: SearchSuccess if betterByCurrentScheme(found) => found + case _ => fail + end healAmbiguous negateIfNot(tryImplicit(cand, contextual)) match { case fail: SearchFailure => @@ -1471,8 +1479,7 @@ trait Implicits: else // The ambiguity happened in a nested search: to recover we // need a candidate better than `cand` - healAmbiguous(fail, newCand => - compareAlternatives(newCand, cand) > 0) + healAmbiguous(fail, cand :: Nil) else // keep only warnings that don't involve the failed candidate reference priorityChangeWarnings.filterInPlace: (critical, _) => @@ -1491,9 +1498,7 @@ trait Implicits: // The ambiguity happened in the current search: to recover we // need a candidate better than the two ambiguous alternatives. val ambi = fail.reason.asInstanceOf[AmbiguousImplicits] - healAmbiguous(fail, newCand => - compareAlternatives(newCand, ambi.alt1) > 0 && - compareAlternatives(newCand, ambi.alt2) > 0) + healAmbiguous(fail, ambi.alt1 :: ambi.alt2 :: Nil) } } case nil => From 78e8ec5ed456fafd4972529ead9209ad804893e4 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Wed, 7 Aug 2024 13:57:38 +0200 Subject: [PATCH 8/9] Adjust compilation tests to backported changes --- tests/neg/scala-uri.check | 4 ++-- tests/neg/scala-uri.scala | 1 + tests/pos/i13044.scala | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/neg/scala-uri.check b/tests/neg/scala-uri.check index 91bcd7ab6a6c..b6d52d6fffd0 100644 --- a/tests/neg/scala-uri.check +++ b/tests/neg/scala-uri.check @@ -1,5 +1,5 @@ --- [E172] Type Error: tests/neg/scala-uri.scala:30:59 ------------------------------------------------------------------ -30 |@main def Test = summon[QueryKeyValue[(String, None.type)]] // error +-- [E172] Type Error: tests/neg/scala-uri.scala:31:59 ------------------------------------------------------------------ +31 |@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: diff --git a/tests/neg/scala-uri.scala b/tests/neg/scala-uri.scala index 3820f8cf5613..f3bff269234f 100644 --- a/tests/neg/scala-uri.scala +++ b/tests/neg/scala-uri.scala @@ -1,3 +1,4 @@ +//> using options -source:3.6 import scala.language.implicitConversions trait QueryKey[A] diff --git a/tests/pos/i13044.scala b/tests/pos/i13044.scala index 4c9b8b914062..36299d9e8366 100644 --- a/tests/pos/i13044.scala +++ b/tests/pos/i13044.scala @@ -1,4 +1,4 @@ -//> using options -Xmax-inlines:33 +//> using options -Xmax-inlines:35 import scala.deriving.Mirror import scala.compiletime._ From 98e2668b6f0b2f25cd7435be356539458f29b1b8 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Wed, 7 Aug 2024 21:38:02 +0200 Subject: [PATCH 9/9] Revert "Compensate loss of transitivity" This reverts commit 8a41389dd4ea6c15f2089519ac5883c4c72a0c56. --- .../src/dotty/tools/dotc/typer/Implicits.scala | 17 ++--------------- tests/pos/given-owner-disambiguate.scala | 13 ------------- 2 files changed, 2 insertions(+), 28 deletions(-) delete mode 100644 tests/pos/given-owner-disambiguate.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 5e9575903895..e6c019860d08 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1385,6 +1385,7 @@ trait Implicits: def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match case alt1: SearchSuccess => var diff = compareAlternatives(alt1, alt2, disambiguate = true) + // diff > 0 candidates should already have been eliminated in `rank` if diff == 0 && alt1.ref =:= alt2.ref then diff = 1 // See i12951 for a test where this happens else if diff == 0 && alt2.isExtension then @@ -1636,21 +1637,7 @@ trait Implicits: validateOrdering(ord) throw ex - val sorted = sort(eligible) - val res = sorted match - case first :: rest => - val firstIsImplicit = first.ref.symbol.is(Implicit) - if rest.exists(_.ref.symbol.is(Implicit) != firstIsImplicit) then - // Mixture of implicits and givens - // Rank implicits first, then, if there is a given that it better than the best implicit(s) - // switch over to givens. - val (sortedImplicits, sortedGivens) = sorted.partition(_.ref.symbol.is(Implicit)) - val implicitResult = rank(sortedImplicits, NoMatchingImplicitsFailure, Nil) - rank(sortedGivens, implicitResult, Nil) - else - rank(sorted, NoMatchingImplicitsFailure, Nil) - case _ => - NoMatchingImplicitsFailure + val res = rank(sort(eligible), NoMatchingImplicitsFailure, Nil) // Issue all priority change warnings that can affect the result val shownWarnings = priorityChangeWarnings.toList.collect: diff --git a/tests/pos/given-owner-disambiguate.scala b/tests/pos/given-owner-disambiguate.scala deleted file mode 100644 index f0a44ecc441a..000000000000 --- a/tests/pos/given-owner-disambiguate.scala +++ /dev/null @@ -1,13 +0,0 @@ -class General -class Specific extends General - -class LowPriority: - given a:General() - -object NormalPriority extends LowPriority: - given b:Specific() - -def run = - import NormalPriority.given - val x = summon[General] - val _: Specific = x // <- b was picked \ No newline at end of file