From 2f3f02f50f30e1bd182cb9c606e3edf37cd4f6b4 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 25 May 2024 15:34:59 +0200 Subject: [PATCH 1/5] 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/5] 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/5] 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 03ced0d378ccbcd67a17428947fd1b1afb9cf3ae Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 28 May 2024 18:00:27 +0200 Subject: [PATCH 4/5] Drop priority change warnings that don't qualify Drop priority change warnings if one the mentioned references does not succeed via tryImplicit. --- .../dotty/tools/dotc/typer/Implicits.scala | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index a15541fa9c76..72a9b7545d05 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1301,9 +1301,10 @@ trait Implicits: 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)]() + // with the candidate refs mentioned in the warning. We report the associated + // message if both candidates qualify in tryImplicit and at least one of the candidates + // is part of the result of the implicit search. + val priorityChangeWarnings = mutable.ListBuffer[(TermRef, TermRef, Message)]() /** Compare `alt1` with `alt2` to determine which one should be chosen. * @@ -1322,7 +1323,7 @@ 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) + priorityChangeWarnings += ((alt1.ref, alt2.ref, msg)) if alt1.ref eq alt2.ref then 0 else if alt1.level != alt2.level then alt1.level - alt2.level else @@ -1440,7 +1441,11 @@ trait Implicits: // need a candidate better than `cand` healAmbiguous(fail, newCand => compareAlternatives(newCand, cand) > 0) - else rank(remaining, found, fail :: rfailures) + else + // keep only warnings that don't involve the failed candidate reference + priorityChangeWarnings.filterInPlace: (ref1, ref2, _) => + ref1 != cand.ref && ref2 != cand.ref + rank(remaining, found, fail :: rfailures) case best: SearchSuccess => if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) best @@ -1596,8 +1601,9 @@ trait Implicits: throw ex val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil) - for (ref, msg) <- priorityChangeWarnings do - if result.found.contains(ref) then report.warning(msg, srcPos) + for (ref1, ref2, msg) <- priorityChangeWarnings do + if result.found.exists(ref => ref == ref1 || ref == ref2) then + report.warning(msg, srcPos) result end searchImplicit From 3112bb72f56ca5d4463308e2178c0ffcce6f70da Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 28 May 2024 18:40:55 +0200 Subject: [PATCH 5/5] Add test for #20484 --- tests/pos/i20484.scala | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tests/pos/i20484.scala diff --git a/tests/pos/i20484.scala b/tests/pos/i20484.scala new file mode 100644 index 000000000000..2f02e6206101 --- /dev/null +++ b/tests/pos/i20484.scala @@ -0,0 +1,3 @@ +given Int = ??? +given Char = ??? +val a = summon[Int] \ No newline at end of file