Skip to content

Commit

Permalink
Filter out more false positives in priority change warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
odersky authored and WojciechMazur committed Jul 10, 2024
1 parent 3e1ed72 commit 450d233
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1318,16 +1318,14 @@ trait Implicits:
* return new result with preferGeneral = true
* 3.6 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.
* In each case a priority change from 0 to -1 or vice versa makes no difference.
* @param only2ndCritical If true only the second alternative is critical in case
* of a priority change.
*/
def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel, only2ndCritical: Boolean = false): Int =
def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel, disambiguate: Boolean = false, only2ndCritical: Boolean = false): Int =
def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))
def warn(msg: Message) =
val critical =
if only2ndCritical then alt2.ref :: Nil
else alt1.ref :: alt2.ref :: Nil
priorityChangeWarnings += ((critical, msg))
if alt1.ref eq alt2.ref then 0
else if alt1.level != alt2.level then alt1.level - alt2.level
else
Expand All @@ -1336,6 +1334,12 @@ trait Implicits:
if sv.stable == SourceVersion.`3.5` || sv == SourceVersion.`3.6-migration` then
val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution))
if cmp != prev then
def warn(msg: Message) =
if disambiguate || cmp > 0 || prev > 0 then
val critical =
if only2ndCritical then alt2.ref :: Nil
else alt1.ref :: alt2.ref :: Nil
priorityChangeWarnings += ((critical, msg))
def choice(c: Int) = c match
case -1 => "the second alternative"
case 1 => "the first alternative"
Expand All @@ -1362,7 +1366,7 @@ trait Implicits:
*/
def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match
case alt1: SearchSuccess =>
var diff = compareAlternatives(alt1, alt2)
var diff = compareAlternatives(alt1, alt2, disambiguate = true)
assert(diff <= 0) // 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
Expand Down Expand Up @@ -1463,7 +1467,7 @@ trait Implicits:
// `retained`. Therefore, it's a priorty change only if the
// second alternative appears in the final search result. Otherwise
// we have the following scenario:
// - 1st alternative, bit not snd appears in final result
// - 1st alternative, but not snd appears in final result
// - Hence, snd was eliminated either here, or otherwise by a direct
// comparison later.
// - Hence, no change in resolution.
Expand Down

0 comments on commit 450d233

Please sign in to comment.