From 2f3f02f50f30e1bd182cb9c606e3edf37cd4f6b4 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 25 May 2024 15:34:59 +0200 Subject: [PATCH 1/4] Avoid useless warnings about priority change in implicit search Warn about priority change in implicit search only if one of the participating candidates appears in the final result. It could be that we have an priority change between two ranked candidates that both are superseded by the result of the implicit search. In this case, no warning needs to be reported. --- .../dotty/tools/dotc/typer/Implicits.scala | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 54821444aed6..a15541fa9c76 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -419,6 +419,12 @@ object Implicits: sealed abstract class SearchResult extends Showable { def tree: Tree def toText(printer: Printer): Text = printer.toText(this) + + /** The references that were found, there can be two of them in the case + * of an AmbiguousImplicits failure + */ + def found: List[TermRef] + def recoverWith(other: SearchFailure => SearchResult): SearchResult = this match { case _: SearchSuccess => this case fail: SearchFailure => other(fail) @@ -434,13 +440,17 @@ object Implicits: * @param tstate The typer state to be committed if this alternative is chosen */ case class SearchSuccess(tree: Tree, ref: TermRef, level: Int, isExtension: Boolean = false)(val tstate: TyperState, val gstate: GadtConstraint) - extends SearchResult with RefAndLevel with Showable + extends SearchResult with RefAndLevel with Showable: + final def found = ref :: Nil /** A failed search */ case class SearchFailure(tree: Tree) extends SearchResult { require(tree.tpe.isInstanceOf[SearchFailureType], s"unexpected type for ${tree}") final def isAmbiguous: Boolean = tree.tpe.isInstanceOf[AmbiguousImplicits | TooUnspecific] final def reason: SearchFailureType = tree.tpe.asInstanceOf[SearchFailureType] + final def found = tree.tpe match + case tpe: AmbiguousImplicits => tpe.alt1.ref :: tpe.alt2.ref :: Nil + case _ => Nil } object SearchFailure { @@ -1290,6 +1300,11 @@ 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) + // with a candidate ref mentioned in the warning. We report the associated + // message if the candidate ref is part of the result of the implicit search + var priorityChangeWarnings = mutable.ListBuffer[(TermRef, Message)]() + /** Compare `alt1` with `alt2` to determine which one should be chosen. * * @return a number > 0 if `alt1` is preferred over `alt2` @@ -1306,6 +1321,8 @@ trait Implicits: */ def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true)) + def warn(msg: Message) = + priorityChangeWarnings += (alt1.ref -> msg) += (alt2.ref -> msg) if alt1.ref eq alt2.ref then 0 else if alt1.level != alt2.level then alt1.level - alt2.level else @@ -1319,16 +1336,16 @@ trait Implicits: case 1 => "the first alternative" case _ => "none - it's ambiguous" if sv.stable == SourceVersion.`3.5` then - report.warning( + 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.6: ${choice(cmp)}""", srcPos) + |New choice from Scala 3.6: ${choice(cmp)}""") prev else - report.warning( + warn( 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) + |New choice from Scala 3.6: ${choice(cmp)}""") cmp else cmp else cmp @@ -1578,7 +1595,10 @@ trait Implicits: validateOrdering(ord) throw ex - rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + for (ref, msg) <- priorityChangeWarnings do + if result.found.contains(ref) then report.warning(msg, srcPos) + result end searchImplicit def isUnderSpecifiedArgument(tp: Type): Boolean = From 23a6027bb5a0710dce2b26ac99103e08a5d7caff Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 27 May 2024 17:57:03 +0200 Subject: [PATCH 2/4] Re-enable semanticdb test --- tests/semanticdb/expect/InventedNames.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/semanticdb/expect/InventedNames.scala b/tests/semanticdb/expect/InventedNames.scala index 61baae46c832..42c14c90e370 100644 --- a/tests/semanticdb/expect/InventedNames.scala +++ b/tests/semanticdb/expect/InventedNames.scala @@ -32,7 +32,7 @@ given [T]: Z[T] with val a = intValue val b = given_String -//val c = given_Double +val c = given_Double val d = given_List_T[Int] val e = given_Char val f = given_Float From a47035ed7be4d59eb3f2ce0ee108bc35798c7ca0 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 27 May 2024 23:38:03 +0200 Subject: [PATCH 3/4] Update semanticDB expect files --- tests/semanticdb/expect/InventedNames.expect.scala | 2 +- tests/semanticdb/metac.expect | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/semanticdb/expect/InventedNames.expect.scala b/tests/semanticdb/expect/InventedNames.expect.scala index b92e9aa940a7..7c5b008209c2 100644 --- a/tests/semanticdb/expect/InventedNames.expect.scala +++ b/tests/semanticdb/expect/InventedNames.expect.scala @@ -32,7 +32,7 @@ given [T/*<-givens::InventedNames$package.given_Z_T#[T]*/]: Z/*->givens::Z#*/[T/ val a/*<-givens::InventedNames$package.a.*/ = intValue/*->givens::InventedNames$package.intValue.*/ val b/*<-givens::InventedNames$package.b.*/ = given_String/*->givens::InventedNames$package.given_String.*/ -//val c = given_Double +val c/*<-givens::InventedNames$package.c.*/ = given_Double/*->givens::InventedNames$package.given_Double().*/ val d/*<-givens::InventedNames$package.d.*/ = given_List_T/*->givens::InventedNames$package.given_List_T().*/[Int/*->scala::Int#*/] val e/*<-givens::InventedNames$package.e.*/ = given_Char/*->givens::InventedNames$package.given_Char.*/ val f/*<-givens::InventedNames$package.f.*/ = given_Float/*->givens::InventedNames$package.given_Float.*/ diff --git a/tests/semanticdb/metac.expect b/tests/semanticdb/metac.expect index 98657f122255..84c3e7c6a110 100644 --- a/tests/semanticdb/metac.expect +++ b/tests/semanticdb/metac.expect @@ -2093,15 +2093,16 @@ Schema => SemanticDB v4 Uri => InventedNames.scala Text => empty Language => Scala -Symbols => 44 entries -Occurrences => 64 entries -Synthetics => 2 entries +Symbols => 45 entries +Occurrences => 66 entries +Synthetics => 3 entries Symbols: -givens/InventedNames$package. => final package object givens extends Object { self: givens.type => +23 decls } +givens/InventedNames$package. => final package object givens extends Object { self: givens.type => +24 decls } givens/InventedNames$package.`* *`. => final implicit lazy val given method * * Long givens/InventedNames$package.a. => val method a Int givens/InventedNames$package.b. => val method b String +givens/InventedNames$package.c. => val method c Double givens/InventedNames$package.d. => val method d List[Int] givens/InventedNames$package.e. => val method e Char givens/InventedNames$package.f. => val method f Float @@ -2192,6 +2193,8 @@ Occurrences: [32:8..32:16): intValue -> givens/InventedNames$package.intValue. [33:4..33:5): b <- givens/InventedNames$package.b. [33:8..33:20): given_String -> givens/InventedNames$package.given_String. +[34:4..34:5): c <- givens/InventedNames$package.c. +[34:8..34:20): given_Double -> givens/InventedNames$package.given_Double(). [35:4..35:5): d <- givens/InventedNames$package.d. [35:8..35:20): given_List_T -> givens/InventedNames$package.given_List_T(). [35:21..35:24): Int -> scala/Int# @@ -2211,6 +2214,7 @@ Occurrences: Synthetics: [24:0..24:0): => *(x$1) +[34:8..34:20):given_Double => *(intValue) [40:8..40:15):given_Y => *(given_X) expect/Issue1749.scala From 9f388042ec243eb10054224b247e3abeb6a2c88c Mon Sep 17 00:00:00 2001 From: Natsu Kagami Date: Tue, 28 May 2024 16:15:53 +0200 Subject: [PATCH 4/4] Only return the warning if the other candidate succeeds tryImplicit --- .../dotty/tools/dotc/typer/Implicits.scala | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index a15541fa9c76..3e0d1b7f8c69 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1319,12 +1319,10 @@ trait Implicits: * 3.6 and higher: compare with preferGeneral = true * */ - def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = + def compareAlternativesWithWarning(alt1: RefAndLevel, alt2: RefAndLevel): (Int, Option[Message]) = def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true)) - def warn(msg: Message) = - priorityChangeWarnings += (alt1.ref -> msg) += (alt2.ref -> msg) - if alt1.ref eq alt2.ref then 0 - else if alt1.level != alt2.level then alt1.level - alt2.level + if alt1.ref eq alt2.ref then (0, None) + else if alt1.level != alt2.level then (alt1.level - alt2.level, None) else var cmp = comp(using searchContext()) val sv = Feature.sourceVersion @@ -1335,20 +1333,25 @@ trait Implicits: case -1 => "the second alternative" case 1 => "the first alternative" case _ => "none - it's ambiguous" + val sv = Feature.sourceVersion if sv.stable == SourceVersion.`3.5` then - warn( - em"""Given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} will change + (prev, + Some(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 + |New choice from Scala 3.6: ${choice(cmp)}""")) else - warn( - em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} + (cmp, + Some(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)}""") - cmp - else cmp - else cmp + |New choice from Scala 3.6: ${choice(cmp)}""")) + else (cmp, None) + else (cmp, None) + def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int = + inline def warn(msg: Message) = + priorityChangeWarnings += (alt1.ref -> msg) += (alt2.ref -> msg) + val (cmp, warnings) = compareAlternativesWithWarning(alt1, alt2) + warnings.foreach(warn(_)) + cmp end compareAlternatives /** If `alt1` is also a search success, try to disambiguate as follows: @@ -1449,7 +1452,16 @@ trait Implicits: val newPending = if (retained eq found) || remaining.isEmpty then remaining else remaining.filterConserve(cand => - compareAlternatives(retained, cand) <= 0) + compareAlternativesWithWarning(retained, cand) match + case (cmp, _) if cmp <= 0 => true + case (cmp, Some(msg)) => + if negateIfNot(tryImplicit(cand, contextual)).isInstanceOf[SearchSuccess] then + priorityChangeWarnings += (cand.ref -> msg) += (retained.ref -> msg) + false + case _ => false + + + ) rank(newPending, retained, rfailures) case fail: SearchFailure => // The ambiguity happened in the current search: to recover we