Skip to content

Commit c94ffac

Browse files
oderskyodersky
committed
Make priority change warning messages stable
Make priority change warning messages stable under different orders of eligible. We now always report the previously chosen alternative first and the new one second.
1 parent f45085a commit c94ffac

File tree

3 files changed

+54
-36
lines changed

3 files changed

+54
-36
lines changed

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

+30-32
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 if cmp != prev then
13721370
0 // When ranking, we retain a candidate if it is retained in either version.
13731371
else cmp

‎tests/neg/given-triangle.check

+4-4
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

‎tests/warn/i15264.check

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-- Warning: tests/warn/i15264.scala:48:26 ------------------------------------------------------------------------------
2+
48 | val a = summon[A[Int]] // warn
3+
| ^
4+
| Given search preference for repro.A[Int] between alternatives
5+
| (repro.exports.given_C_V : [V](using x$1: priority.Prio0): repro.C[V])
6+
| and
7+
| (repro.exports.given_A_V : [V](using x$1: priority.Prio2): repro.A[V])
8+
| has changed.
9+
| Previous choice : the first alternative
10+
| New choice from Scala 3.7: the second alternative
11+
-- Warning: tests/warn/i15264.scala:55:29 ------------------------------------------------------------------------------
12+
55 | val a = summon[A[Q[Int]]] // warn
13+
| ^
14+
| Given search preference for repro.A[repro.Q[Int]] between alternatives
15+
| (repro.qcontext.gbq : [V](using p1: priority.Prio1)(using b: repro.B[V]): repro.B[repro.Q[V]])
16+
| and
17+
| (repro.qcontext.gaq : [V](using p2: priority.Prio2)(using a: repro.A[V]): repro.A[repro.Q[V]])
18+
| has changed.
19+
| Previous choice : the first alternative
20+
| New choice from Scala 3.7: the second alternative

0 commit comments

Comments
 (0)