From d4de2cbae94eb36e37277d4cac5a20356e01bb36 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 20 Dec 2023 18:09:33 +0100 Subject: [PATCH 1/8] Fix shapeless 3 deriving test. Typo: mkInstances instead of mkProductInstances, previously got healed by accident because if most specific rule. Change rules for given prioritization Consider the following program: ```scala class A class B extends A class C extends A given A = A() given B = B() given C = C() def f(using a: A, b: B, c: C) = println(a.getClass) println(b.getClass) println(c.getClass) @main def Test = f ``` With the current rules, this would fail with an ambiguity error between B and C when trying to synthesize the A parameter. This is a problem without an easy remedy. We can fix this problem by flipping the priority for implicit arguments. Instead of requiring an argument to be most _specific_, we now require it to be most _general_ while still conforming to the formal parameter. There are three justifications for this change, which at first glance seems quite drastic: - It gives us a natural way to deal with inheritance triangles like the one in the code above. Such triangles are quite common. - Intuitively, we want to get the closest possible match between required formal parameter type and synthetisized argument. The "most general" rule provides that. - We already do a crucial part of this. Namely, with current rules we interpolate all type variables in an implicit argument downwards, no matter what their variance is. This makes no sense in theory, but solves hairy problems with contravariant typeclasses like `Comparable`. Instead of this hack, we now do something more principled, by flipping the direction everywhere, preferring general over specific, instead of just flipping contravariant type parameters. Don't flip contravariant type arguments for overloading resolution Flipping contravariant type arguments was needed for implicit search where it will be replaced by a more general scheme. But it makes no sense for overloading resolution. For overloading resolution, we want to pick the most specific alternative, analogous to us picking the most specific instantiation when we force a fully defined type. Disable implicit search everywhere for disambiaguation Previously, one disambiguation step missed that, whereas implicits were turned off everywhere else. --- .../community-projects/shapeless-3 | 2 +- compiler/src/dotty/tools/dotc/core/Mode.scala | 6 +- .../dotty/tools/dotc/typer/Applications.scala | 30 +++++++--- .../dotty/tools/dotc/typer/Implicits.scala | 15 +++-- .../tools/dotc/typer/ImportSuggestions.scala | 2 +- tests/neg/i15264.scala | 58 +++++++++++++++++++ tests/pos/i15264.scala | 5 +- tests/pos/overload-disambiguation.scala | 13 +++++ tests/run/given-triangle.check | 3 + tests/run/given-triangle.scala | 14 +++++ tests/run/implicit-specifity.scala | 2 +- tests/run/implied-for.scala | 2 +- tests/run/implied-priority.scala | 10 ++-- 13 files changed, 135 insertions(+), 27 deletions(-) create mode 100644 tests/neg/i15264.scala create mode 100644 tests/pos/overload-disambiguation.scala create mode 100644 tests/run/given-triangle.check create mode 100644 tests/run/given-triangle.scala diff --git a/community-build/community-projects/shapeless-3 b/community-build/community-projects/shapeless-3 index d27c5ba1ae51..90f0c977b536 160000 --- a/community-build/community-projects/shapeless-3 +++ b/community-build/community-projects/shapeless-3 @@ -1 +1 @@ -Subproject commit d27c5ba1ae5111b85df2cfb65a26b9246c52570c +Subproject commit 90f0c977b536c06305496600b8b2014c9e8e3d86 diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 71b49394ae14..c3405160bc18 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -41,6 +41,8 @@ object Mode { val Pattern: Mode = newMode(0, "Pattern") val Type: Mode = newMode(1, "Type") + val PatternOrTypeBits: Mode = Pattern | Type + val ImplicitsEnabled: Mode = newMode(2, "ImplicitsEnabled") val InferringReturnType: Mode = newMode(3, "InferringReturnType") @@ -120,8 +122,6 @@ object Mode { /** Read original positions when unpickling from TASTY */ val ReadPositions: Mode = newMode(17, "ReadPositions") - val PatternOrTypeBits: Mode = Pattern | Type - /** We are elaborating the fully qualified name of a package clause. * In this case, identifiers should never be imported. */ @@ -133,6 +133,8 @@ object Mode { /** We are typing the body of an inline method */ val InlineableBody: Mode = newMode(21, "InlineableBody") + val NewGivenRules: Mode = newMode(22, "NewGivenRules") + /** We are synthesizing the receiver of an extension method */ val SynthesizeExtMethodReceiver: Mode = newMode(23, "SynthesizeExtMethodReceiver") diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index ffbb223a923a..e484bef612ed 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1724,9 +1724,12 @@ trait Applications extends Compatibility { * an alternative that takes more implicit parameters wins over one * that takes fewer. */ - def compare(alt1: TermRef, alt2: TermRef)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) { + def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) { record("resolveOverloaded.compare") + val newGivenRules = + ctx.mode.is(Mode.NewGivenRules) && alt1.symbol.is(Given) + /** Is alternative `alt1` with type `tp1` as specific as alternative * `alt2` with type `tp2` ? * @@ -1809,9 +1812,11 @@ trait Applications extends Compatibility { * the intersection of its parent classes instead. */ def isAsSpecificValueType(tp1: Type, tp2: Type)(using Context) = - if (ctx.mode.is(Mode.OldOverloadingResolution)) + if !preferGeneral || ctx.mode.is(Mode.OldOverloadingResolution) then + // Normal specificity test for overloading resultion (where `preferGeneral` is false) + // and in mode Scala3-migration when we compare with the old Scala 2 rules. isCompatible(tp1, tp2) - else { + else val flip = new TypeMap { def apply(t: Type) = t match { case t @ AppliedType(tycon, args) => @@ -1822,13 +1827,20 @@ trait Applications extends Compatibility { case _ => mapOver(t) } } - def prepare(tp: Type) = tp.stripTypeVar match { + + def prepare(tp: Type) = tp.stripTypeVar match case tp: NamedType if tp.symbol.is(Module) && tp.symbol.sourceModule.is(Given) => - flip(tp.widen.widenToParents) - case _ => flip(tp) - } - (prepare(tp1) relaxed_<:< prepare(tp2)) || viewExists(tp1, tp2) - } + tp.widen.widenToParents + case _ => + tp + + val tp1p = prepare(tp1) + val tp2p = prepare(tp2) + if newGivenRules then + (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1) + else + (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2) + end isAsSpecificValueType /** Widen the result type of synthetic given methods from the implementation class to the * type that's implemented. Example diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 5880a659a301..dbfcb6d26e76 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1226,7 +1226,7 @@ trait Implicits: assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType], em"found: $argument: ${argument.tpe}, expected: $pt") - private def nestedContext() = + private def searchContext() = ctx.fresh.setMode(ctx.mode &~ Mode.ImplicitsEnabled) private def isCoherent = pt.isRef(defn.CanEqualClass) @@ -1270,7 +1270,7 @@ trait Implicits: else val history = ctx.searchHistory.nest(cand, pt) val typingCtx = - nestedContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history) + searchContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history) val result = typedImplicit(cand, pt, argument, span)(using typingCtx) result match case res: SearchSuccess => @@ -1297,7 +1297,12 @@ trait Implicits: def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = if alt1.ref eq alt2.ref then 0 else if alt1.level != alt2.level then alt1.level - alt2.level - else explore(compare(alt1.ref, alt2.ref))(using nestedContext()) + else + val was = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext()) + val now = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext().addMode(Mode.NewGivenRules)) + if was != now then + println(i"change in preference for $pt between ${alt1.ref} and ${alt2.ref}, was: $was, now: $now at $srcPos") + now /** If `alt1` is also a search success, try to disambiguate as follows: * - If alt2 is preferred over alt1, pick alt2, otherwise return an @@ -1333,8 +1338,8 @@ trait Implicits: else ctx.typerState - diff = inContext(ctx.withTyperState(comparisonState)): - compare(ref1, ref2) + diff = inContext(searchContext().withTyperState(comparisonState)): + compare(ref1, ref2, preferGeneral = true) else // alt1 is a conversion, prefer extension alt2 over it diff = -1 if diff < 0 then alt2 diff --git a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala index 33643a0fae2f..5ab6a4a5fae6 100644 --- a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala +++ b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala @@ -296,7 +296,7 @@ trait ImportSuggestions: var i = 0 var diff = 0 while i < filled && diff == 0 do - diff = compare(ref, top(i))(using noImplicitsCtx) + diff = compare(ref, top(i), preferGeneral = true)(using noImplicitsCtx) if diff > 0 then rest += top(i) top(i) = ref diff --git a/tests/neg/i15264.scala b/tests/neg/i15264.scala new file mode 100644 index 000000000000..9dbc253cf33e --- /dev/null +++ b/tests/neg/i15264.scala @@ -0,0 +1,58 @@ +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 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]] + +object test2: + import repro.* + import repro.qcontext.given + + // This one will fail as ambiguous - prios aren't having an effect. + // Priorities indeed don't have an effect if the result is already decided + // without using clauses, they onyl act as a tie breaker. + // With the new resolution rules, it's ambiguous since we pick `gaq` for + // summon, and that needs an A[Int], but there are only the two competing choices + // qb and qc. + val a = summon[A[Q[Int]]] // error: ambiguous between qb and qc for A[Int] diff --git a/tests/pos/i15264.scala b/tests/pos/i15264.scala index 05992df61b94..5be8436c12ba 100644 --- a/tests/pos/i15264.scala +++ b/tests/pos/i15264.scala @@ -30,6 +30,7 @@ object repro: // 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] @@ -45,9 +46,9 @@ object test1: // these will work val a = summon[A[Int]] + object test2: import repro.* import repro.qcontext.given - // this one will fail as ambiguous - prios aren't having an effect - val a = summon[A[Q[Int]]] \ No newline at end of file + val a = summon[A[Q[Int]]] diff --git a/tests/pos/overload-disambiguation.scala b/tests/pos/overload-disambiguation.scala new file mode 100644 index 000000000000..58b085758d92 --- /dev/null +++ b/tests/pos/overload-disambiguation.scala @@ -0,0 +1,13 @@ +class A +class B +class C[-T] + +def foo(using A): C[Any] = ??? +def foo(using B): C[Int] = ??? + + +@main def Test = + given A = A() + given B = B() + val x = foo + val _: C[Any] = x diff --git a/tests/run/given-triangle.check b/tests/run/given-triangle.check new file mode 100644 index 000000000000..5ba9e6a1e8b9 --- /dev/null +++ b/tests/run/given-triangle.check @@ -0,0 +1,3 @@ +class A +class B +class C diff --git a/tests/run/given-triangle.scala b/tests/run/given-triangle.scala new file mode 100644 index 000000000000..9d39689996ba --- /dev/null +++ b/tests/run/given-triangle.scala @@ -0,0 +1,14 @@ +class A +class B extends A +class C extends A + +given A = A() +given B = B() +given C = C() + +def f(using a: A, b: B, c: C) = + println(a.getClass) + println(b.getClass) + println(c.getClass) + +@main def Test = f diff --git a/tests/run/implicit-specifity.scala b/tests/run/implicit-specifity.scala index 51fa02d91cfd..fb8f84d9f94d 100644 --- a/tests/run/implicit-specifity.scala +++ b/tests/run/implicit-specifity.scala @@ -38,5 +38,5 @@ object Test extends App { assert(Show[Int] == 0) assert(Show[String] == 1) assert(Show[Generic] == 1) // showGen loses against fallback due to longer argument list - assert(Show[Generic2] == 2) // ... but the opaque type intersection trick works. + assert(Show[Generic2] == 1) // ... and the opaque type intersection trick no longer works with new resolution rules. } diff --git a/tests/run/implied-for.scala b/tests/run/implied-for.scala index c7789ce570e4..a55d59e89505 100644 --- a/tests/run/implied-for.scala +++ b/tests/run/implied-for.scala @@ -20,7 +20,7 @@ object Test extends App { val x2: T = t val x3: D[Int] = d - assert(summon[T].isInstanceOf[B]) + assert(summon[T].isInstanceOf[T]) assert(summon[D[Int]].isInstanceOf[D[_]]) } diff --git a/tests/run/implied-priority.scala b/tests/run/implied-priority.scala index 0822fae6778f..b02412ddaf0c 100644 --- a/tests/run/implied-priority.scala +++ b/tests/run/implied-priority.scala @@ -72,16 +72,16 @@ def test2a = { } /* If that solution is not applicable, we can define an override by refining the - * result type of the given instance, e.g. like this: + * result type of all lower-priority instances, e.g. like this: */ object Impl3 { - given t1[T]: E[T]("low") + trait LowPriority // A marker trait to indicate a lower priority + given t1[T]: E[T]("low") with LowPriority } object Override { - trait HighestPriority // A marker trait to indicate a higher priority - given over[T]: E[T]("hi") with HighestPriority() + given over[T]: E[T]("hi") with {} } def test3 = { @@ -90,7 +90,7 @@ def test3 = { { import Override.given import Impl3.given - assert(summon[E[String]].str == "hi") // `over` takes priority since its result type is a subtype of t1's. + assert(summon[E[String]].str == "hi", summon[E[String]].str) // `Impl3` takes priority since its result type is a subtype of t1's. } } From 1b9a7e0978e1d5e16d750000aac05cac5207ba32 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 21 Dec 2023 11:32:24 +0100 Subject: [PATCH 2/8] Change rules for given prioritization Consider the following program: ```scala class A class B extends A class C extends A given A = A() given B = B() given C = C() def f(using a: A, b: B, c: C) = println(a.getClass) println(b.getClass) println(c.getClass) @main def Test = f ``` With the current rules, this would fail with an ambiguity error between B and C when trying to synthesize the A parameter. This is a problem without an easy remedy. We can fix this problem by flipping the priority for implicit arguments. Instead of requiring an argument to be most _specific_, we now require it to be most _general_ while still conforming to the formal parameter. There are three justifications for this change, which at first glance seems quite drastic: - It gives us a natural way to deal with inheritance triangles like the one in the code above. Such triangles are quite common. - Intuitively, we want to get the closest possible match between required formal parameter type and synthetisized argument. The "most general" rule provides that. - We already do a crucial part of this. Namely, with current rules we interpolate all type variables in an implicit argument downwards, no matter what their variance is. This makes no sense in theory, but solves hairy problems with contravariant typeclasses like `Comparable`. Instead of this hack, we now do something more principled, by flipping the direction everywhere, preferring general over specific, instead of just flipping contravariant type parameters. The behavior is dependent on the Scala version - Old behavior: up to 3.4 - New behavior: from 3.5, 3.5-migration warns on behavior change The CB builds under the new rules. One fix was needed for a shapeless 3 deriving test. There was a typo: mkInstances instead of mkProductInstances, which previously got healed by accident because of the most specific rule. Also: Don't flip contravariant type arguments for overloading resolution Flipping contravariant type arguments was needed for implicit search where it will be replaced by a more general scheme. But it makes no sense for overloading resolution. For overloading resolution, we want to pick the most specific alternative, analogous to us picking the most specific instantiation when we force a fully defined type. Also: Disable implicit search everywhere for disambiaguation Previously, one disambiguation step missed that, whereas implicits were turned off everywhere else. --- compiler/src/dotty/tools/dotc/core/Mode.scala | 13 +- .../dotty/tools/dotc/typer/Applications.scala | 120 ++++++++++-------- .../dotty/tools/dotc/typer/Implicits.scala | 26 ++-- .../changed-features/implicit-resolution.md | 17 ++- tests/neg/i15264.scala | 1 + tests/run/given-triangle.scala | 2 + tests/run/implicit-specifity.scala | 2 + tests/run/implied-priority.scala | 1 + tests/warn/given-triangle.check | 6 + tests/warn/given-triangle.scala | 16 +++ 10 files changed, 137 insertions(+), 67 deletions(-) create mode 100644 tests/warn/given-triangle.check create mode 100644 tests/warn/given-triangle.scala diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index c3405160bc18..5dab5631c62a 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -103,16 +103,19 @@ object Mode { */ val CheckBoundsOrSelfType: Mode = newMode(14, "CheckBoundsOrSelfType") - /** Use Scala2 scheme for overloading and implicit resolution */ - val OldOverloadingResolution: Mode = newMode(15, "OldOverloadingResolution") + /** Use previous Scheme for implicit resolution. Currently significant + * in 3.0-migration where we use Scala-2's scheme instead and in 3.5-migration + * where we use the previous scheme up to 3.4 instead. + */ + val OldImplicitResolution: Mode = newMode(15, "OldImplicitResolution") /** Treat CapturingTypes as plain AnnotatedTypes even in phase CheckCaptures. - * Reuses the value of OldOverloadingResolution to save Mode bits. - * This is OK since OldOverloadingResolution only affects implicit search, which + * Reuses the value of OldImplicitResolution to save Mode bits. + * This is OK since OldImplicitResolution only affects implicit search, which * is done during phases Typer and Inlinig, and IgnoreCaptures only has an * effect during phase CheckCaptures. */ - val IgnoreCaptures = OldOverloadingResolution + val IgnoreCaptures = OldImplicitResolution /** Allow hk applications of type lambdas to wildcard arguments; * used for checking that such applications do not normally arise diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index e484bef612ed..0b4ad5ff9bc8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -22,7 +22,7 @@ import ProtoTypes.* import Inferencing.* import reporting.* import Nullables.*, NullOpsDecorator.* -import config.Feature +import config.{Feature, SourceVersion} import collection.mutable import config.Printers.{overload, typr, unapp} @@ -1709,6 +1709,12 @@ trait Applications extends Compatibility { /** Compare two alternatives of an overloaded call or an implicit search. * * @param alt1, alt2 Non-overloaded references indicating the two choices + * @param preferGeneral When comparing two value types, prefer the more general one + * over the more specific one iff `preferGeneral` is true. + * `preferGeneral` is set to `true` when we compare two given values, since + * then we want the most general evidence that matches the target + * type. It is set to `false` for overloading resolution, when we want the + * most specific type instead. * @return 1 if 1st alternative is preferred over 2nd * -1 if 2nd alternative is preferred over 1st * 0 if neither alternative is preferred over the other @@ -1727,27 +1733,25 @@ 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 newGivenRules = - ctx.mode.is(Mode.NewGivenRules) && alt1.symbol.is(Given) + val compareGivens = alt1.symbol.is(Given) || alt2.symbol.is(Given) - /** Is alternative `alt1` with type `tp1` as specific as alternative + /** Is alternative `alt1` with type `tp1` as good as alternative * `alt2` with type `tp2` ? * - * 1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as specific as `alt2` + * 1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as good as `alt2` * if `alt1` is nullary or `alt2` is applicable to arguments (p1, ..., pn) of * types T1,...,Tn. If the last parameter `pn` has a vararg type T*, then * `alt1` must be applicable to arbitrary numbers of `T` parameters (which * implies that it must be a varargs method as well). * 2. A polymorphic member of type [a1 >: L1 <: U1, ..., an >: Ln <: Un]T is as - * specific as `alt2` of type `tp2` if T is as specific as `tp2` under the + * good as `alt2` of type `tp2` if T is as good as `tp2` under the * assumption that for i = 1,...,n each ai is an abstract type name bounded * from below by Li and from above by Ui. * 3. A member of any other type `tp1` is: - * a. always as specific as a method or a polymorphic method. - * b. as specific as a member of any other type `tp2` if `tp1` is compatible - * with `tp2`. + * a. always as good as a method or a polymorphic method. + * b. as good as a member of any other type `tp2` is `asGoodValueType(tp1, tp2) = true` */ - def isAsSpecific(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) { + def isAsGood(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) { tp1 match case tp1: MethodType => // (1) tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType] @@ -1769,65 +1773,60 @@ trait Applications extends Compatibility { fullyDefinedType(tp1Params, "type parameters of alternative", alt1.symbol.srcPos) val tparams = newTypeParams(alt1.symbol, tp1.paramNames, EmptyFlags, tp1.instantiateParamInfos(_)) - isAsSpecific(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2) + isAsGood(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2) } case _ => // (3) tp2 match case tp2: MethodType => true // (3a) case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a) case tp2: PolyType => // (3b) - explore(isAsSpecificValueType(tp1, instantiateWithTypeVars(tp2))) + explore(isAsGoodValueType(tp1, instantiateWithTypeVars(tp2))) case _ => // 3b) - isAsSpecificValueType(tp1, tp2) + isAsGoodValueType(tp1, tp2) } - /** Test whether value type `tp1` is as specific as value type `tp2`. - * Let's abbreviate this to `tp1 <:s tp2`. - * Previously, `<:s` was the same as `<:`. This behavior is still - * available under mode `Mode.OldOverloadingResolution`. The new behavior - * is different, however. Here, `T <:s U` iff + /** Test whether value type `tp1` is as good as value type `tp2`. + * Let's abbreviate this to `tp1 <:p tp2`. The behavior depends on the Scala version + * and mode. * - * flip(T) <: flip(U) + * - In Scala 2, `<:p` was the same as `<:`. This behavior is still + * 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. * - * where `flip` changes covariant occurrences of contravariant type parameters to - * covariant ones. Intuitively `<:s` means subtyping `<:`, except that all arguments - * to contravariant parameters are compared as if they were covariant. E.g. given class + * - In Scala 3.0-3.4, the behavior is as follows: `T <:p U` iff there is an impliit conversion + * from `T` to `U`, or * - * class Cmp[-X] + * flip(T) <: flip(U) * - * `Cmp[T] <:s Cmp[U]` if `T <: U`. On the other hand, non-variant occurrences - * of parameters are not affected. So `T <: U` would imply `Set[Cmp[U]] <:s Set[Cmp[T]]`, - * as usual, because `Set` is non-variant. + * where `flip` changes covariant occurrences of contravariant type parameters to + * covariant ones. Intuitively `<:p` means subtyping `<:`, except that all arguments + * to contravariant parameters are compared as if they were covariant. E.g. given class * - * This relation might seem strange, but it models closely what happens for methods. - * Indeed, if we integrate the existing rules for methods into `<:s` we have now that + * class Cmp[-X] * - * (T)R <:s (U)R + * `Cmp[T] <:p Cmp[U]` if `T <: U`. On the other hand, non-variant occurrences + * of parameters are not affected. So `T <: U` would imply `Set[Cmp[U]] <:p Set[Cmp[T]]`, + * as usual, because `Set` is non-variant. * - * iff + * - From Scala 3.5, `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. * - * T => R <:s U => R + * - In Scala 3.5-migration, use the 3.5 scheme normally, and the 3.4 scheme if + * `Mode.OldImplicitResolution` is on. This is used to highlight differences in the + * two resolution schemes. * - * Also: If a compared type refers to a given or its module class, use + * 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 isAsSpecificValueType(tp1: Type, tp2: Type)(using Context) = - if !preferGeneral || ctx.mode.is(Mode.OldOverloadingResolution) then - // Normal specificity test for overloading resultion (where `preferGeneral` is false) + def isAsGoodValueType(tp1: Type, tp2: Type)(using Context) = + val oldResolution = ctx.mode.is(Mode.OldImplicitResolution) + if !preferGeneral || Feature.migrateTo3 && oldResolution 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) else - val flip = new TypeMap { - def apply(t: Type) = t match { - case t @ AppliedType(tycon, args) => - def mapArg(arg: Type, tparam: TypeParamInfo) = - if (variance > 0 && tparam.paramVarianceSign < 0) defn.FunctionNOf(arg :: Nil, defn.UnitType) - else arg - mapOver(t.derivedAppliedType(tycon, args.zipWithConserve(tycon.typeParams)(mapArg))) - case _ => mapOver(t) - } - } - def prepare(tp: Type) = tp.stripTypeVar match case tp: NamedType if tp.symbol.is(Module) && tp.symbol.sourceModule.is(Given) => tp.widen.widenToParents @@ -1836,11 +1835,27 @@ trait Applications extends Compatibility { val tp1p = prepare(tp1) val tp2p = prepare(tp2) - if newGivenRules then - (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1) - else + + if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) + || oldResolution + || !compareGivens + then + // Intermediate rules: better means specialize, but map all type arguments downwards + // These are enabled for 3.0-3.4, and for all comparisons between old-style implicits, + // and in 3.5-migration when we compare with previous rules. + val flip = new TypeMap: + def apply(t: Type) = t match + case t @ AppliedType(tycon, args) => + def mapArg(arg: Type, tparam: TypeParamInfo) = + if (variance > 0 && tparam.paramVarianceSign < 0) defn.FunctionNOf(arg :: Nil, defn.UnitType) + else arg + mapOver(t.derivedAppliedType(tycon, args.zipWithConserve(tycon.typeParams)(mapArg))) + case _ => mapOver(t) (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2) - end isAsSpecificValueType + else + // 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 * type that's implemented. Example @@ -1900,9 +1915,8 @@ trait Applications extends Compatibility { def compareWithTypes(tp1: Type, tp2: Type) = val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner) - - val winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) - val winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) + val winsType1 = isAsGood(alt1, tp1, alt2, tp2) + val winsType2 = isAsGood(alt2, tp2, alt1, tp1) overload.println(i"compare($alt1, $alt2)? $tp1 $tp2 $ownerScore $winsType1 $winsType2") if winsType1 && winsType2 diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index dbfcb6d26e76..818512a4fa6f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -531,7 +531,7 @@ object Implicits: |must be more specific than $target""" :: Nil override def msg(using Context) = - super.msg.append(i"\nThe expected type $target is not specific enough, so no search was attempted") + super.msg.append("\nThe expected type $target is not specific enough, so no search was attempted") override def toString = s"TooUnspecific" end TooUnspecific @@ -1110,8 +1110,8 @@ trait Implicits: case result: SearchFailure if result.isAmbiguous => val deepPt = pt.deepenProto if (deepPt ne pt) inferImplicit(deepPt, argument, span) - else if (migrateTo3 && !ctx.mode.is(Mode.OldOverloadingResolution)) - withMode(Mode.OldOverloadingResolution)(inferImplicit(pt, argument, span)) match { + else if (migrateTo3 && !ctx.mode.is(Mode.OldImplicitResolution)) + withMode(Mode.OldImplicitResolution)(inferImplicit(pt, argument, span)) match { case altResult: SearchSuccess => report.migrationWarning( result.reason.msg @@ -1295,14 +1295,24 @@ trait Implicits: * 0 if neither alternative is preferred over the other */ def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = + def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true)) if alt1.ref eq alt2.ref then 0 else if alt1.level != alt2.level then alt1.level - alt2.level else - val was = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext()) - val now = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext().addMode(Mode.NewGivenRules)) - if was != now then - println(i"change in preference for $pt between ${alt1.ref} and ${alt2.ref}, was: $was, now: $now at $srcPos") - now + val cmp = comp(using searchContext()) + if Feature.sourceVersion == SourceVersion.`3.5-migration` then + val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution)) + if cmp != prev then + def choice(c: Int) = c match + case -1 => "the second alternative" + case 1 => "the first alternative" + case _ => "none - it's ambiguous" + report.warning( + em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} + |Previous choice: ${choice(prev)} + |New choice : ${choice(cmp)}""", srcPos) + cmp + end compareAlternatives /** If `alt1` is also a search success, try to disambiguate as follows: * - If alt2 is preferred over alt1, pick alt2, otherwise return an diff --git a/docs/_docs/reference/changed-features/implicit-resolution.md b/docs/_docs/reference/changed-features/implicit-resolution.md index 1396ed04b6d3..0df8d2d60a7a 100644 --- a/docs/_docs/reference/changed-features/implicit-resolution.md +++ b/docs/_docs/reference/changed-features/implicit-resolution.md @@ -165,7 +165,22 @@ Condition (*) is new. It is necessary to ensure that the defined relation is tra [//]: # todo: expand with precise rules -**9.** The following change is currently enabled in `-source future`: + +**9.** Given disambiguation has changed. When comparing two givens that both match an expected type, we used to pick the most specific one, in alignment with +overloading resolution. From Scala 3.5 on, we pick the most general one instead. Compiling with Scala 3.5-migration will print a warning in all cases where the preference has changed. Example: +```scala +class A +class B extends A +class C extends A + +given A = A() +given B = B() +given C = C() + +summon[A] // was ambiguous, will now return `given_A` +``` + +**10.** The following change is currently enabled in `-source future`: Implicit resolution now avoids generating recursive givens that can lead to an infinite loop at runtime. Here is an example: diff --git a/tests/neg/i15264.scala b/tests/neg/i15264.scala index 9dbc253cf33e..e13e1089dba3 100644 --- a/tests/neg/i15264.scala +++ b/tests/neg/i15264.scala @@ -1,3 +1,4 @@ +import language.`3.5` object priority: // lower number = higher priority class Prio0 extends Prio1 diff --git a/tests/run/given-triangle.scala b/tests/run/given-triangle.scala index 9d39689996ba..5ddba8df8b7b 100644 --- a/tests/run/given-triangle.scala +++ b/tests/run/given-triangle.scala @@ -1,3 +1,5 @@ +import language.future + class A class B extends A class C extends A diff --git a/tests/run/implicit-specifity.scala b/tests/run/implicit-specifity.scala index fb8f84d9f94d..14954eddf2ef 100644 --- a/tests/run/implicit-specifity.scala +++ b/tests/run/implicit-specifity.scala @@ -1,3 +1,5 @@ +import language.`3.5` + case class Show[T](val i: Int) object Show { def apply[T](implicit st: Show[T]): Int = st.i diff --git a/tests/run/implied-priority.scala b/tests/run/implied-priority.scala index b02412ddaf0c..61049de8e43e 100644 --- a/tests/run/implied-priority.scala +++ b/tests/run/implied-priority.scala @@ -1,5 +1,6 @@ /* These tests show various mechanisms available for implicit prioritization. */ +import language.`3.5` class E[T](val str: String) // The type for which we infer terms below diff --git a/tests/warn/given-triangle.check b/tests/warn/given-triangle.check new file mode 100644 index 000000000000..69583830c2bc --- /dev/null +++ b/tests/warn/given-triangle.check @@ -0,0 +1,6 @@ +-- Warning: tests/warn/given-triangle.scala:16:18 ---------------------------------------------------------------------- +16 |@main def Test = f // warn + | ^ + | Change in given search preference for A between alternatives (given_A : A) and (given_B : B) + | Previous choice: the second alternative + | New choice : the first alternative diff --git a/tests/warn/given-triangle.scala b/tests/warn/given-triangle.scala new file mode 100644 index 000000000000..bc1a5c774f4f --- /dev/null +++ b/tests/warn/given-triangle.scala @@ -0,0 +1,16 @@ +//> using options -source 3.5-migration + +class A +class B extends A +class C extends A + +given A = A() +given B = B() +given C = C() + +def f(using a: A, b: B, c: C) = + println(a.getClass) + println(b.getClass) + println(c.getClass) + +@main def Test = f // warn From 77218332e5b49f81d8d50a5e54869120919e2d16 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 16 Apr 2024 14:47:37 +0200 Subject: [PATCH 3/8] Fix rebase breakage --- 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 0b4ad5ff9bc8..b023e421f570 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1907,8 +1907,8 @@ trait Applications extends Compatibility { def comparePrefixes = val pre1 = widenPrefix(alt1) val pre2 = widenPrefix(alt2) - val winsPrefix1 = isAsSpecificValueType(pre1, pre2) - val winsPrefix2 = isAsSpecificValueType(pre2, pre1) + val winsPrefix1 = isAsGoodValueType(pre1, pre2) + val winsPrefix2 = isAsGoodValueType(pre2, pre1) if winsPrefix1 == winsPrefix2 then 0 else if winsPrefix1 then 1 else -1 From c54dbbf570503ed8b5299691be6fb08e7e4679be Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 27 Apr 2024 22:37:52 +0200 Subject: [PATCH 4/8] Switch to new rules only if both sides are givens (rather than implicits). --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index b023e421f570..d949090dd514 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1733,7 +1733,7 @@ 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 compareGivens = alt1.symbol.is(Given) || alt2.symbol.is(Given) + val compareGivens = alt1.symbol.is(Given) && alt2.symbol.is(Given) /** Is alternative `alt1` with type `tp1` as good as alternative * `alt2` with type `tp2` ? From b644d303e08bfa4b70498df7d3c170ec3889376c Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 5 May 2024 22:25:08 +0200 Subject: [PATCH 5/8] Refine prioritization rules between givens and implicits In the new system, givens always beat implicits when comparing value types. This is necessary to maintain two invariants in the new system: - When comparing old-style implicits nothing changes, we still use the old rules. - The isAsGood comparison is transitive. Exception: NotGiven has to be treated at lower priority. --- .../dotty/tools/dotc/typer/Applications.scala | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index d949090dd514..aa7659e78146 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1733,8 +1733,6 @@ 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 compareGivens = alt1.symbol.is(Given) && alt2.symbol.is(Given) - /** Is alternative `alt1` with type `tp1` as good as alternative * `alt2` with type `tp2` ? * @@ -1749,7 +1747,7 @@ trait Applications extends Compatibility { * from below by Li and from above by Ui. * 3. A member of any other type `tp1` is: * a. always as good as a method or a polymorphic method. - * b. as good as a member of any other type `tp2` is `asGoodValueType(tp1, tp2) = true` + * b. as good as a member of any other type `tp2` if `asGoodValueType(tp1, tp2) = true` */ def isAsGood(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) { tp1 match @@ -1776,13 +1774,17 @@ 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)) tp2 match case tp2: MethodType => true // (3a) case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a) case tp2: PolyType => // (3b) - explore(isAsGoodValueType(tp1, instantiateWithTypeVars(tp2))) + explore(compareValues(tp1, instantiateWithTypeVars(tp2))) case _ => // 3b) - isAsGoodValueType(tp1, tp2) + compareValues(tp1, tp2) } /** Test whether value type `tp1` is as good as value type `tp2`. @@ -1812,6 +1814,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. * * - In Scala 3.5-migration, use the 3.5 scheme normally, and the 3.4 scheme if * `Mode.OldImplicitResolution` is on. This is used to highlight differences in the @@ -1820,7 +1823,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)(using Context) = + def isAsGoodValueType(tp1: Type, tp2: Type, alt1isGiven: Boolean, alt2isGiven: Boolean)(using Context): Boolean = val oldResolution = ctx.mode.is(Mode.OldImplicitResolution) if !preferGeneral || Feature.migrateTo3 && oldResolution then // Normal specificity test for overloading resolution (where `preferGeneral` is false) @@ -1838,7 +1841,7 @@ trait Applications extends Compatibility { if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`) || oldResolution - || !compareGivens + || !alt1isGiven && !alt2isGiven then // Intermediate rules: better means specialize, but map all type arguments downwards // These are enabled for 3.0-3.4, and for all comparisons between old-style implicits, @@ -1853,8 +1856,9 @@ trait Applications extends Compatibility { case _ => mapOver(t) (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2) else - // New rules: better means generalize - (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1) + // New rules: better means generalize, givens always beat implicits + if alt1isGiven != alt2isGiven then alt1isGiven + else (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1) end isAsGoodValueType /** Widen the result type of synthetic given methods from the implementation class to the @@ -1907,8 +1911,8 @@ trait Applications extends Compatibility { def comparePrefixes = val pre1 = widenPrefix(alt1) val pre2 = widenPrefix(alt2) - val winsPrefix1 = isAsGoodValueType(pre1, pre2) - val winsPrefix2 = isAsGoodValueType(pre2, pre1) + val winsPrefix1 = isCompatible(pre1, pre2) + val winsPrefix2 = isCompatible(pre2, pre1) if winsPrefix1 == winsPrefix2 then 0 else if winsPrefix1 then 1 else -1 From bc26c5176a42281b6621550237818c91e7acdecd Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 17 Apr 2024 23:01:54 +0200 Subject: [PATCH 6/8] Delay roll-out of new prioritization scheme: Now: 3.5: old scheme but warn if there are changes in the future 3.6-migration: new scheme, warn if prioritization has changed 3.6: new scheme, no warning --- .../tools/dotc/config/SourceVersion.scala | 1 + .../dotty/tools/dotc/typer/Applications.scala | 13 ++++---- .../dotty/tools/dotc/typer/Implicits.scala | 32 +++++++++++++++---- .../runtime/stdLibPatches/language.scala | 14 ++++++++ tests/neg/i15264.scala | 2 +- tests/run/implicit-specifity.scala | 2 +- tests/run/implied-priority.scala | 2 +- tests/warn/given-triangle.check | 4 +-- tests/warn/given-triangle.scala | 2 +- 9 files changed, 52 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala index 7a464d331930..3a44021af2df 100644 --- a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala +++ b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala @@ -11,6 +11,7 @@ enum SourceVersion: case `3.3-migration`, `3.3` case `3.4-migration`, `3.4` case `3.5-migration`, `3.5` + case `3.6-migration`, `3.6` // !!! 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 aa7659e78146..d91c4592a77b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1795,7 +1795,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.4, the behavior is as follows: `T <:p U` iff there is an impliit conversion + * - In Scala 3.0-3.5, the behavior is as follows: `T <:p U` iff there is an impliit conversion * from `T` to `U`, or * * flip(T) <: flip(U) @@ -1810,15 +1810,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.5, `T <:p U` means `T <: U` or `T` convertible to `U` + * - From Scala 3.6, `T <:p U` means `T <: U` or `T` convertible to `U` * for overloading resolution (when `preferGeneral is false), and the opposite relation * `U <: T` or `U convertible to `T` for implicit disambiguation between givens * (when `preferGeneral` is true). For old-style implicit values, the 3.4 behavior is kept. * If one of the alternatives is a given and the other is an implicit, the given wins. * - * - In Scala 3.5-migration, use the 3.5 scheme normally, and the 3.4 scheme if - * `Mode.OldImplicitResolution` is on. This is used to highlight differences in the - * two resolution schemes. + * - In Scala 3.5 and Scala 3.6-migration, we issue a warning if the result under + * Scala 3.6 differ wrt to the old behavior up to 3.5. * * Also and only for given resolution: If a compared type refers to a given or its module class, use * the intersection of its parent classes instead. @@ -1844,8 +1843,8 @@ trait Applications extends Compatibility { || !alt1isGiven && !alt2isGiven then // Intermediate rules: better means specialize, but map all type arguments downwards - // These are enabled for 3.0-3.4, and for all comparisons between old-style implicits, - // and in 3.5-migration when we compare with previous rules. + // These are enabled for 3.0-3.5, and for all comparisons between old-style implicits, + // and in 3.5 amd 3.6-migration when we compare with previous rules. val flip = new TypeMap: def apply(t: Type) = t match case t @ AppliedType(tycon, args) => diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 818512a4fa6f..6180fa1a5e52 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1293,25 +1293,43 @@ trait Implicits: * @return a number > 0 if `alt1` is preferred over `alt2` * 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, + * return old result with preferGeneral = false + * 3.6-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 + * */ def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true)) if alt1.ref eq alt2.ref then 0 else if alt1.level != alt2.level then alt1.level - alt2.level else - val cmp = comp(using searchContext()) - if Feature.sourceVersion == SourceVersion.`3.5-migration` then + var cmp = comp(using searchContext()) + val sv = Feature.sourceVersion + if sv == SourceVersion.`3.5` || sv == SourceVersion.`3.6-migration` then val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution)) if cmp != prev then def choice(c: Int) = c match case -1 => "the second alternative" case 1 => "the first alternative" case _ => "none - it's ambiguous" - report.warning( - em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} - |Previous choice: ${choice(prev)} - |New choice : ${choice(cmp)}""", srcPos) - cmp + if sv == SourceVersion.`3.5` then + report.warning( + 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)}""", srcPos) + prev + else + report.warning( + em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} + |Previous choice : ${choice(prev)} + |New choice from Scala 3.6: ${choice(cmp)}""", srcPos) + cmp + else cmp + else cmp end compareAlternatives /** If `alt1` is also a search success, try to disambiguate as follows: diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala index 3c9c172918d2..372e1e34bb85 100644 --- a/library/src/scala/runtime/stdLibPatches/language.scala +++ b/library/src/scala/runtime/stdLibPatches/language.scala @@ -260,6 +260,20 @@ object language: @compileTimeOnly("`3.5` can only be used at compile time in import statements") object `3.5` + /** Set source version to 3.6-migration. + * + * @see [[https://docs.scala-lang.org/scala3/guides/migration/compatibility-intro.html]] + */ + @compileTimeOnly("`3.6-migration` can only be used at compile time in import statements") + object `3.6-migration` + + /** Set source version to 3.6 + * + * @see [[https://docs.scala-lang.org/scala3/guides/migration/compatibility-intro.html]] + */ + @compileTimeOnly("`3.6` can only be used at compile time in import statements") + object `3.6` + // !!! 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/i15264.scala b/tests/neg/i15264.scala index e13e1089dba3..825e74701f73 100644 --- a/tests/neg/i15264.scala +++ b/tests/neg/i15264.scala @@ -1,4 +1,4 @@ -import language.`3.5` +import language.`3.6` object priority: // lower number = higher priority class Prio0 extends Prio1 diff --git a/tests/run/implicit-specifity.scala b/tests/run/implicit-specifity.scala index 14954eddf2ef..da90110c9866 100644 --- a/tests/run/implicit-specifity.scala +++ b/tests/run/implicit-specifity.scala @@ -1,4 +1,4 @@ -import language.`3.5` +import language.`3.6` case class Show[T](val i: Int) object Show { diff --git a/tests/run/implied-priority.scala b/tests/run/implied-priority.scala index 61049de8e43e..15f6a40a27ef 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.5` +import language.`3.6` class E[T](val str: String) // The type for which we infer terms below diff --git a/tests/warn/given-triangle.check b/tests/warn/given-triangle.check index 69583830c2bc..e849f9d4d642 100644 --- a/tests/warn/given-triangle.check +++ b/tests/warn/given-triangle.check @@ -2,5 +2,5 @@ 16 |@main def Test = f // warn | ^ | Change in given search preference for A between alternatives (given_A : A) and (given_B : B) - | Previous choice: the second alternative - | New choice : the first alternative + | Previous choice : the second alternative + | New choice from Scala 3.6: the first alternative diff --git a/tests/warn/given-triangle.scala b/tests/warn/given-triangle.scala index bc1a5c774f4f..ee4888ed1e06 100644 --- a/tests/warn/given-triangle.scala +++ b/tests/warn/given-triangle.scala @@ -1,4 +1,4 @@ -//> using options -source 3.5-migration +//> using options -source 3.6-migration class A class B extends A From 33f801bc3da1dbc4c0ff5322546c44a7b8f3602b Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 5 May 2024 18:25:46 +0200 Subject: [PATCH 7/8] Test extract from reactive-mongo This will now be ambiguous. --- tests/warn/bson.check | 10 ++++++++++ tests/warn/bson/Test.scala | 5 +++++ tests/warn/bson/bson.scala | 29 +++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 tests/warn/bson.check create mode 100644 tests/warn/bson/Test.scala create mode 100644 tests/warn/bson/bson.scala diff --git a/tests/warn/bson.check b/tests/warn/bson.check new file mode 100644 index 000000000000..258ac4b4ff2c --- /dev/null +++ b/tests/warn/bson.check @@ -0,0 +1,10 @@ +-- Warning: tests/warn/bson/Test.scala:5:60 ---------------------------------------------------------------------------- +5 |def typedMapHandler[K, V: BSONHandler] = stringMapHandler[V] // warn + | ^ + |Given search preference for bson.BSONWriter[Map[String, V]] between alternatives (bson.BSONWriter.mapWriter : [V²](using x$1: bson.BSONWriter[V²]): bson.BSONDocumentWriter[Map[String, V²]]) and (bson.BSONWriter.collectionWriter : + | [T, Repr <: Iterable[T]](using x$1: bson.BSONWriter[T], x$2: Repr ¬ Option[T]): bson.BSONWriter[Repr]) will change + |Current choice : the first alternative + |New choice from Scala 3.6: none - it's ambiguous + | + |where: V is a type in method typedMapHandler + | V² is a type variable diff --git a/tests/warn/bson/Test.scala b/tests/warn/bson/Test.scala new file mode 100644 index 000000000000..78b6687adabf --- /dev/null +++ b/tests/warn/bson/Test.scala @@ -0,0 +1,5 @@ +//> using options -source 3.5 +import bson.* + +def stringMapHandler[V](using writer: BSONWriter[Map[String, V]]): BSONHandler[Map[String, V]] = ??? +def typedMapHandler[K, V: BSONHandler] = stringMapHandler[V] // warn diff --git a/tests/warn/bson/bson.scala b/tests/warn/bson/bson.scala new file mode 100644 index 000000000000..d901ee3e3a4f --- /dev/null +++ b/tests/warn/bson/bson.scala @@ -0,0 +1,29 @@ +package bson + +trait BSONWriter[T] +trait BSONDocumentWriter[T] extends BSONWriter[T] +object BSONWriter extends BSONWriterInstances + +trait BSONHandler[T] extends BSONWriter[T] + +private[bson] trait BSONWriterInstances { + given mapWriter[V](using BSONWriter[V]): BSONDocumentWriter[Map[String, V]] = bson.mapWriter[V] + export bson.collectionWriter +} + +final class ¬[A, B] +object ¬ { + implicit def defaultEvidence[A, B]: ¬[A, B] = new ¬[A, B]() + @annotation.implicitAmbiguous("Could not prove type ${A} is not (¬) ${A}") + implicit def ambiguousEvidence1[A]: ¬[A, A] = null + implicit def ambiguousEvidence2[A]: ¬[A, A] = null +} + +private[bson] trait DefaultBSONHandlers extends LowPriorityHandlers +private[bson] trait LowPriorityHandlers{ + given collectionWriter[T, Repr <: Iterable[T]](using BSONWriter[T], Repr ¬ Option[T]): BSONWriter[Repr] = ??? + private[bson] def mapWriter[V](implicit valueWriter: BSONWriter[V]): BSONDocumentWriter[Map[String, V]] = ??? +} + +// --- +package object bson extends DefaultBSONHandlers \ No newline at end of file From 8a3854fb73fcb88375b35106ea337b4ab907f900 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 6 May 2024 09:00:02 +0200 Subject: [PATCH 8/8] Avoid ambiguity errors arising from double references in implicits If two implicit candidate references happen to be the same pick one of them instead of reporting an ambiguity. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 4 +++- tests/{neg => pos}/i12591/Inner.scala | 3 ++- tests/{neg => pos}/i12591/Outer.scala | 0 3 files changed, 5 insertions(+), 2 deletions(-) rename tests/{neg => pos}/i12591/Inner.scala (64%) rename tests/{neg => pos}/i12591/Outer.scala (100%) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 6180fa1a5e52..9f2e0628e70e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1340,7 +1340,9 @@ trait Implicits: case alt1: SearchSuccess => var diff = compareAlternatives(alt1, alt2) assert(diff <= 0) // diff > 0 candidates should already have been eliminated in `rank` - if diff == 0 && alt2.isExtension then + 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 if alt1.isExtension then // Fall back: if both results are extension method applications, // compare the extension methods instead of their wrappers. diff --git a/tests/neg/i12591/Inner.scala b/tests/pos/i12591/Inner.scala similarity index 64% rename from tests/neg/i12591/Inner.scala rename to tests/pos/i12591/Inner.scala index aae9bd5b9234..2f8018c4d824 100644 --- a/tests/neg/i12591/Inner.scala +++ b/tests/pos/i12591/Inner.scala @@ -9,5 +9,6 @@ object Foo: import Foo.TC //Adding import Foo.Bar resolves the issue -val badSummon = summon[TC[Bar]] // error here +val badSummon = summon[TC[Bar]] + // was an ambiguous error, now OK, since the two references are the same diff --git a/tests/neg/i12591/Outer.scala b/tests/pos/i12591/Outer.scala similarity index 100% rename from tests/neg/i12591/Outer.scala rename to tests/pos/i12591/Outer.scala