Skip to content

Commit f5f390e

Browse files
committed
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.
1 parent 87f5ca0 commit f5f390e

File tree

2 files changed

+36
-38
lines changed

2 files changed

+36
-38
lines changed

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

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,9 +1310,6 @@ trait Implicits:
13101310
// message if one of the critical candidates is part of the result of the implicit search.
13111311
val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], Message)]()
13121312

1313-
def isWarnPriorityChangeVersion(sv: SourceVersion): Boolean =
1314-
sv.stable == SourceVersion.`3.6` || sv == SourceVersion.`3.7-migration`
1315-
13161313
/** Compare `alt1` with `alt2` to determine which one should be chosen.
13171314
*
13181315
* @return a number > 0 if `alt1` is preferred over `alt2`
@@ -1337,37 +1334,38 @@ trait Implicits:
13371334
else
13381335
val cmp = comp(using searchContext())
13391336
val sv = Feature.sourceVersion
1340-
if isWarnPriorityChangeVersion(sv) then
1337+
val isLastOldVersion = sv.stable == SourceVersion.`3.6`
1338+
val isMigratingVersion = sv == SourceVersion.`3.7-migration`
1339+
if isLastOldVersion || isMigratingVersion then
13411340
val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution))
13421341
if disambiguate && cmp != prev then
1343-
def warn(msg: Message) =
1344-
val critical = alt1.ref :: alt2.ref :: Nil
1345-
priorityChangeWarnings += ((critical, msg))
1346-
implicits.println(i"PRIORITY CHANGE ${alt1.ref}, ${alt2.ref}, $disambiguate")
1347-
def choice(c: Int) = c match
1348-
case -1 => "the second alternative"
1349-
case 1 => "the first alternative"
1350-
case _ => "none - it's ambiguous"
1351-
if sv.stable == SourceVersion.`3.6` then
1352-
warn(
1353-
em"""Given search preference for $pt between alternatives
1354-
| ${alt1.ref}
1355-
|and
1356-
| ${alt2.ref}
1357-
|will change.
1358-
|Current choice : ${choice(prev)}
1359-
|New choice from Scala 3.7: ${choice(cmp)}""")
1360-
prev
1361-
else
1362-
warn(
1363-
em"""Given search preference for $pt between alternatives
1364-
| ${alt1.ref}
1365-
|and
1366-
| ${alt2.ref}
1367-
|has changed.
1368-
|Previous choice : ${choice(prev)}
1369-
|New choice from Scala 3.7: ${choice(cmp)}""")
1370-
cmp
1342+
implicits.println(i"PRIORITY CHANGE ${alt1.ref}, ${alt2.ref}")
1343+
val (loser, winner) =
1344+
prev match
1345+
case 1 => (alt1, alt2)
1346+
case -1 => (alt2, alt1)
1347+
case 0 =>
1348+
cmp match
1349+
case 1 => (alt2, alt1)
1350+
case -1 => (alt1, alt2)
1351+
def choice(nth: String, c: Int) =
1352+
if c == 0 then "none - it's ambiguous"
1353+
else s"the $nth alternative"
1354+
val (change, whichChoice) =
1355+
if isLastOldVersion
1356+
then ("will change", "Current choice ")
1357+
else ("has changed", "Previous choice")
1358+
val msg =
1359+
em"""Given search preference for $pt between alternatives
1360+
| ${loser.ref}
1361+
|and
1362+
| ${winner.ref}
1363+
|$change.
1364+
|$whichChoice : ${choice("first", prev)}
1365+
|New choice from Scala 3.7: ${choice("second", cmp)}"""
1366+
val critical = alt1.ref :: alt2.ref :: Nil
1367+
priorityChangeWarnings += ((critical, msg))
1368+
if isLastOldVersion then prev else cmp
13711369
else cmp max prev
13721370
// When ranking, alt1 is always the new candidate and alt2 is the
13731371
// solution found previously. We keep the candidate if the outcome is 0
@@ -1424,8 +1422,8 @@ trait Implicits:
14241422
case fail: SearchFailure =>
14251423
fail.reason match
14261424
case ambi: AmbiguousImplicits =>
1427-
if compareAlternatives(ambi.alt1, alt2) < 0 &&
1428-
compareAlternatives(ambi.alt2, alt2) < 0
1425+
if compareAlternatives(ambi.alt1, alt2, disambiguate = true) < 0
1426+
&& compareAlternatives(ambi.alt2, alt2, disambiguate = true) < 0
14291427
then alt2
14301428
else alt1
14311429
case _ =>

tests/neg/given-triangle.check

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
|Ambiguous given instances: both given instance given_B and given instance given_C match type A of parameter a of method f
55
|
66
|Note: Given search preference for A between alternatives
7-
| (given_A : A)
8-
|and
97
| (given_B : B)
8+
|and
9+
| (given_A : A)
1010
|will change.
11-
|Current choice : the second alternative
12-
|New choice from Scala 3.7: the first alternative
11+
|Current choice : the first alternative
12+
|New choice from Scala 3.7: the second alternative

0 commit comments

Comments
 (0)