Skip to content

Commit 3cf4ada

Browse files
committed
Add priority change warnings to ambiguous implicits error messages
Previously warnings were produced but not shown since at the same position we already have an ambiguity error. We now add the note to the error message.
1 parent bdfb12e commit 3cf4ada

File tree

9 files changed

+108
-5
lines changed

9 files changed

+108
-5
lines changed

compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2988,7 +2988,7 @@ class MissingImplicitArgument(
29882988

29892989
/** Default error message for non-nested ambiguous implicits. */
29902990
def defaultAmbiguousImplicitMsg(ambi: AmbiguousImplicits) =
2991-
s"Ambiguous given instances: ${ambi.explanation}${location("of")}"
2991+
s"Ambiguous given instances: ${ambi.explanation}${location("of")}${ambi.priorityChangeWarningNote}"
29922992

29932993
/** Default error messages for non-ambiguous implicits, or nested ambiguous
29942994
* implicits.

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,12 @@ object Implicits:
549549
/** An ambiguous implicits failure */
550550
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
551551

552+
private[Implicits] var priorityChangeWarning: Message | Null = null
553+
554+
def priorityChangeWarningNote(using Context): String =
555+
if priorityChangeWarning != null then s"\n\nNote: $priorityChangeWarning"
556+
else ""
557+
552558
def msg(using Context): Message =
553559
var str1 = err.refStr(alt1.ref)
554560
var str2 = err.refStr(alt2.ref)
@@ -1348,13 +1354,21 @@ trait Implicits:
13481354
case _ => "none - it's ambiguous"
13491355
if sv.stable == SourceVersion.`3.5` then
13501356
warn(
1351-
em"""Given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} will change
1357+
em"""Given search preference for $pt between alternatives
1358+
| ${alt1.ref}
1359+
|and
1360+
| ${alt2.ref}
1361+
|will change.
13521362
|Current choice : ${choice(prev)}
13531363
|New choice from Scala 3.6: ${choice(cmp)}""")
13541364
prev
13551365
else
13561366
warn(
1357-
em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref}
1367+
em"""Given search preference for $pt between alternatives
1368+
| ${alt1.ref}
1369+
|and
1370+
| ${alt2.ref}
1371+
|has changed.
13581372
|Previous choice : ${choice(prev)}
13591373
|New choice from Scala 3.6: ${choice(cmp)}""")
13601374
cmp
@@ -1615,6 +1629,12 @@ trait Implicits:
16151629
val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil)
16161630
for (critical, msg) <- priorityChangeWarnings do
16171631
if result.found.exists(critical.contains(_)) then
1632+
result match
1633+
case result: SearchFailure =>
1634+
result.reason match
1635+
case ambi: AmbiguousImplicits => ambi.priorityChangeWarning = msg
1636+
case _ =>
1637+
case _ =>
16181638
report.warning(msg, srcPos)
16191639
result
16201640
end searchImplicit

tests/neg/given-triangle.check

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,11 @@
22
15 |@main def Test = f // error
33
| ^
44
|Ambiguous given instances: both given instance given_B and given instance given_C match type A of parameter a of method f
5+
|
6+
|Note: Given search preference for A between alternatives
7+
| (given_A : A)
8+
|and
9+
| (given_B : B)
10+
|will change.
11+
|Current choice : the second alternative
12+
|New choice from Scala 3.6: the first alternative

tests/neg/i21303/JavaEnum.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public enum JavaEnum { ABC, DEF, GHI }

tests/neg/i21303/Test.scala

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//> using options -source 3.6-migration
2+
import scala.deriving.Mirror
3+
import scala.compiletime.*
4+
import scala.reflect.ClassTag
5+
import scala.annotation.implicitNotFound
6+
7+
8+
trait TSType[T]
9+
object TSType extends DefaultTSTypes with TSTypeMacros
10+
11+
trait TSNamedType[T] extends TSType[T]
12+
13+
trait DefaultTSTypes extends JavaTSTypes
14+
trait JavaTSTypes {
15+
given javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSNamedType[E] = ???
16+
}
17+
object DefaultTSTypes extends DefaultTSTypes
18+
trait TSTypeMacros {
19+
inline given [T: Mirror.Of]: TSType[T] = derived[T]
20+
inline def derived[T](using m: Mirror.Of[T]): TSType[T] = {
21+
val elemInstances = summonAll[m.MirroredElemTypes]
22+
???
23+
}
24+
25+
private inline def summonAll[T <: Tuple]: List[TSType[_]] = {
26+
inline erasedValue[T] match {
27+
case _: EmptyTuple => Nil
28+
case _: (t *: ts) => summonInline[TSType[t]] :: summonAll[ts]
29+
}
30+
}
31+
}
32+
33+
@main def Test = summon[TSType[JavaEnum]] // error

tests/pos/i21303/JavaEnum.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public enum JavaEnum { ABC, DEF, GHI }

tests/pos/i21303/Test.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import scala.deriving.Mirror
2+
import scala.compiletime.*
3+
import scala.reflect.ClassTag
4+
import scala.annotation.implicitNotFound
5+
6+
7+
trait TSType[T]
8+
object TSType extends DefaultTSTypes with TSTypeMacros
9+
10+
trait TSNamedType[T] extends TSType[T]
11+
12+
trait DefaultTSTypes extends JavaTSTypes
13+
trait JavaTSTypes {
14+
given javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSType[E] = ???
15+
}
16+
object DefaultTSTypes extends DefaultTSTypes
17+
trait TSTypeMacros {
18+
inline given [T: Mirror.Of]: TSType[T] = derived[T]
19+
inline def derived[T](using m: Mirror.Of[T]): TSType[T] = {
20+
val elemInstances = summonAll[m.MirroredElemTypes]
21+
???
22+
}
23+
24+
private inline def summonAll[T <: Tuple]: List[TSType[_]] = {
25+
inline erasedValue[T] match {
26+
case _: EmptyTuple => Nil
27+
case _: (t *: ts) => summonInline[TSType[t]] :: summonAll[ts]
28+
}
29+
}
30+
}
31+
32+
@main def Test = summon[TSType[JavaEnum]]

tests/warn/i21036a.check

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
-- Warning: tests/warn/i21036a.scala:7:17 ------------------------------------------------------------------------------
22
7 |val y = summon[A] // warn
33
| ^
4-
| Given search preference for A between alternatives (b : B) and (a : A) will change
4+
| Given search preference for A between alternatives
5+
| (b : B)
6+
| and
7+
| (a : A)
8+
| will change.
59
| Current choice : the first alternative
610
| New choice from Scala 3.6: the second alternative

tests/warn/i21036b.check

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
-- Warning: tests/warn/i21036b.scala:7:17 ------------------------------------------------------------------------------
22
7 |val y = summon[A] // warn
33
| ^
4-
| Change in given search preference for A between alternatives (b : B) and (a : A)
4+
| Given search preference for A between alternatives
5+
| (b : B)
6+
| and
7+
| (a : A)
8+
| has changed.
59
| Previous choice : the first alternative
610
| New choice from Scala 3.6: the second alternative

0 commit comments

Comments
 (0)