Avoid useless warnings about priority change in implicit search

[odersky]

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.

[hamzaremmal]

We can maybe rebase this PR on main now that #20441 was merged and uncomment this test to see if it fixes it

[natsukagami]

We should also bring back the expected outputs of the test

> git diff

diff --git a/tests/semanticdb/expect/InventedNames.expect.scala b/tests/semanticdb/expect/InventedNames.expect.scala
index b92e9aa940..7c5b008209 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 98657f1222..84c3e7c6a1 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

I did scala3-compiler-bootstrapped/Test/runMain dotty.tools.dotc.semanticdb.updateExpect to get this diff, but it is identical to the one before.

[hamzaremmal]

This will fail since commit 5a6ca9d also adapted the .expect and .expect.scala files to satisfy the changes in the test

[natsukagami]

Actually I don't think this seems to fix the issue.

The problem seems to lie in

scala3/compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines 1434 to 1435 in d3df8ca

	else remaining.filterConserve(cand =>
	compareAlternatives(retained, cand) <= 0)

Where we still add both the given_Int and given_Char references to the map,

I think when doing this preemptive pruning we have to be careful, something like this when comparing A and B (where A is a SearchSuccess and B is a candidate):

• If A is always better: it's safe to prune B.
• If A is only better than B in >=3.6: we have to perform tryImplicit B, and then
  • if B succeeds => emit warning
  • if not, no warnings
• If B is always better than A: it's safe to just keep B.

[odersky]

About the last commit: I believe it does the right thing, but see #20487 for an alternative that allocates less.