From 35b5f7cccbef0dfacc8c48e2fbd6c4f864c0bb7e Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 11:28:18 +0200 Subject: [PATCH 1/5] 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. --- community-build/community-projects/PPrint | 2 +- .../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 | 11 --- 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 +- 26 files changed, 424 insertions(+), 56 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/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/community-build/community-projects/PPrint b/community-build/community-projects/PPrint index 34a777f687bc..2203dc6081f5 160000 --- a/community-build/community-projects/PPrint +++ b/community-build/community-projects/PPrint @@ -1 +1 @@ -Subproject commit 34a777f687bc851953e682f99edcae9d2875babc +Subproject commit 2203dc6081f5e8fa89f552b155724b0a8fdcec03 diff --git a/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 f112b6fb5aa7..38b49e63c685 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2988,7 +2988,7 @@ class MissingImplicitArgument( /** Default error message for non-nested ambiguous implicits. */ def defaultAmbiguousImplicitMsg(ambi: AmbiguousImplicits) = - s"Ambiguous given instances: ${ambi.explanation}${location("of")}" + s"Ambiguous given instances: ${ambi.explanation}${location("of")}${ambi.priorityChangeWarningNote}" /** Default error messages for non-ambiguous implicits, or nested ambiguous * implicits. diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 42765cd6c0bf..d063854038a1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1762,6 +1762,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 @@ -1788,6 +1799,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` ? @@ -1830,15 +1850,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`. @@ -1876,9 +1896,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) @@ -1892,13 +1911,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) => @@ -1909,9 +1922,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 @@ -1982,13 +1993,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..4cb3741b2d65 --- /dev/null +++ b/tests/neg/i21212.scala @@ -0,0 +1,11 @@ + +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 index 2116beb72012..1a1f2e35819a 100644 --- a/tests/pos/i21212.scala +++ b/tests/pos/i21212.scala @@ -20,14 +20,3 @@ class UsingArguments[F[_]](using Temporal[F])(using err: MonadError[F, Throwable val bool: F[Boolean] = ??? def works = toFunctorOps(bool).map(_ => ()) // warns under -source:3.5 - -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] // picks (most general) a2, was ambiguous - def test3(implicit a3: A, b3: B) = summon[A] // picks (most specific) b3 - -end Minimization 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 7c4bd676744afe67b297fd32b66b4bb40cec78a2 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 13:48:25 +0200 Subject: [PATCH 2/5] 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 f62e14169656dde9c1abcc587b70f163d81bb442 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 5 Aug 2024 18:14:37 +0200 Subject: [PATCH 3/5] Delay priority change until 3.7 Warnings from 3.6, change in 3.7. This is one version later than originally planned. --- .../dotty/tools/dotc/typer/Applications.scala | 14 +++++++------- .../src/dotty/tools/dotc/typer/Implicits.scala | 18 +++++++++--------- tests/neg/ctx-bounds-priority.scala | 2 +- tests/neg/given-triangle.check | 2 +- tests/neg/given-triangle.scala | 2 +- tests/neg/i15264.scala | 2 +- tests/neg/i21212.scala | 2 +- tests/neg/i21303/Test.scala | 2 +- 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 +- 16 files changed, 30 insertions(+), 30 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index d063854038a1..7cb0dbcdc833 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1767,7 +1767,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 @@ -1803,7 +1803,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 @@ -1869,7 +1869,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 implicit 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) @@ -1884,14 +1884,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/tests/neg/ctx-bounds-priority.scala b/tests/neg/ctx-bounds-priority.scala index 6594642d67c3..023a3273d586 100644 --- a/tests/neg/ctx-bounds-priority.scala +++ b/tests/neg/ctx-bounds-priority.scala @@ -1,4 +1,4 @@ -//> using options -source 3.6 +//> using options -source 3.7 trait Eq[A] trait Order[A] extends Eq[A]: def toOrdering: Ordering[A] 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.scala b/tests/neg/i21212.scala index 4cb3741b2d65..99e4c44f9489 100644 --- a/tests/neg/i21212.scala +++ b/tests/neg/i21212.scala @@ -1,4 +1,4 @@ - +//> 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/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 87f5ca00cabb04361c4270f6c98f1ea30b21a1e9 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 6 Aug 2024 19:57:01 +0200 Subject: [PATCH 4/5] Fix ranking logic --- .../src/dotty/tools/dotc/ast/Desugar.scala | 2 +- .../dotty/tools/dotc/typer/Implicits.scala | 31 +++++++--- tests/pos/i15264.scala | 1 + tests/warn/i15264.scala | 56 +++++++++++++++++++ 4 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 tests/warn/i15264.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 4574dd72ffa8..026b8a409d3d 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -483,7 +483,7 @@ object desugar { params.map: param => val normFlags = param.mods.flags &~ GivenOrImplicit | (mparam.mods.flags & (GivenOrImplicit)) param.withMods(param.mods.withFlags(normFlags)) - .showing(i"ADAPTED PARAM $result ${result.mods.flags} for ${meth.name}") + .showing(i"adapted param $result ${result.mods.flags} for ${meth.name}", Printers.desugar) else params (normParams ++ mparams) :: Nil case mparams :: mparamss1 => 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 f5f390ef575a3f5101960144125697db9b7e66ce Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 6 Aug 2024 20:05:59 +0200 Subject: [PATCH 5/5] 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