Skip to content

Commit 87f5ca0

Browse files
committed
Fix ranking logic
1 parent f62e141 commit 87f5ca0

File tree

4 files changed

+80
-10
lines changed

4 files changed

+80
-10
lines changed

compiler/src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ object desugar {
483483
params.map: param =>
484484
val normFlags = param.mods.flags &~ GivenOrImplicit | (mparam.mods.flags & (GivenOrImplicit))
485485
param.withMods(param.mods.withFlags(normFlags))
486-
.showing(i"ADAPTED PARAM $result ${result.mods.flags} for ${meth.name}")
486+
.showing(i"adapted param $result ${result.mods.flags} for ${meth.name}", Printers.desugar)
487487
else params
488488
(normParams ++ mparams) :: Nil
489489
case mparams :: mparamss1 =>

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

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,8 +1369,13 @@ trait Implicits:
13691369
|New choice from Scala 3.7: ${choice(cmp)}""")
13701370
cmp
13711371
else cmp max prev
1372-
// When ranking, we keep the better of cmp and prev, which ends up retaining a candidate
1373-
// if it is retained in either version.
1372+
// When ranking, alt1 is always the new candidate and alt2 is the
1373+
// solution found previously. We keep the candidate if the outcome is 0
1374+
// (ambiguous) or 1 (first wins). Or, when ranking in healImplicit we keep the
1375+
// candidate only if the outcome is 1. In both cases, keeping the better
1376+
// of `cmp` and `prev` means we keep candidates that could match
1377+
// in either scheme. This means that subsequent disambiguation
1378+
// comparisons will record a warning if cmp != prev.
13741379
else cmp
13751380
end compareAlternatives
13761381

@@ -1416,7 +1421,15 @@ trait Implicits:
14161421
if diff < 0 then alt2
14171422
else if diff > 0 then alt1
14181423
else SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument), span)
1419-
case _: SearchFailure => alt2
1424+
case fail: SearchFailure =>
1425+
fail.reason match
1426+
case ambi: AmbiguousImplicits =>
1427+
if compareAlternatives(ambi.alt1, alt2) < 0 &&
1428+
compareAlternatives(ambi.alt2, alt2) < 0
1429+
then alt2
1430+
else alt1
1431+
case _ =>
1432+
alt2
14201433

14211434
/** Try to find a best matching implicit term among all the candidates in `pending`.
14221435
* @param pending The list of candidates that remain to be tested
@@ -1621,7 +1634,7 @@ trait Implicits:
16211634
throw ex
16221635

16231636
val sorted = sort(eligible)
1624-
val result = sorted match
1637+
val res = sorted match
16251638
case first :: rest =>
16261639
val firstIsImplicit = first.ref.symbol.is(Implicit)
16271640
if rest.exists(_.ref.symbol.is(Implicit) != firstIsImplicit) then
@@ -1638,11 +1651,11 @@ trait Implicits:
16381651

16391652
// Issue all priority change warnings that can affect the result
16401653
val shownWarnings = priorityChangeWarnings.toList.collect:
1641-
case (critical, msg) if result.found.exists(critical.contains(_)) =>
1654+
case (critical, msg) if res.found.exists(critical.contains(_)) =>
16421655
msg
1643-
result match
1644-
case result: SearchFailure =>
1645-
result.reason match
1656+
res match
1657+
case res: SearchFailure =>
1658+
res.reason match
16461659
case ambi: AmbiguousImplicits =>
16471660
// Make warnings part of error message because otherwise they are suppressed when
16481661
// the error is emitted.
@@ -1652,7 +1665,7 @@ trait Implicits:
16521665
for msg <- shownWarnings do
16531666
report.warning(msg, srcPos)
16541667

1655-
result
1668+
res
16561669
end searchImplicit
16571670

16581671
def isUnderSpecifiedArgument(tp: Type): Boolean =

tests/pos/i15264.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import language.`3.7`
12
object priority:
23
// lower number = higher priority
34
class Prio0 extends Prio1

tests/warn/i15264.scala

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Note: No check file for this test since the precise warning messages are non-deterministic
2+
import language.`3.7-migration`
3+
object priority:
4+
// lower number = higher priority
5+
class Prio0 extends Prio1
6+
object Prio0 { given Prio0() }
7+
8+
class Prio1 extends Prio2
9+
object Prio1 { given Prio1() }
10+
11+
class Prio2
12+
object Prio2 { given Prio2() }
13+
14+
object repro:
15+
// analogous to cats Eq, Hash, Order:
16+
class A[V]
17+
class B[V] extends A[V]
18+
class C[V] extends A[V]
19+
20+
class Q[V]
21+
22+
object context:
23+
// prios work here, which is cool
24+
given[V](using priority.Prio0): C[V] = new C[V]
25+
given[V](using priority.Prio1): B[V] = new B[V]
26+
given[V](using priority.Prio2): A[V] = new A[V]
27+
28+
object exports:
29+
// so will these exports
30+
export context.given
31+
32+
// if you import these don't import from 'context' above
33+
object qcontext:
34+
// base defs, like what you would get from cats
35+
given ga: A[Int] = new B[Int] // added so that we don't get an ambiguity in test2
36+
given gb: B[Int] = new B[Int]
37+
given gc: C[Int] = new C[Int]
38+
39+
// these seem like they should work but don't
40+
given gcq[V](using p0: priority.Prio0)(using c: C[V]): C[Q[V]] = new C[Q[V]]
41+
given gbq[V](using p1: priority.Prio1)(using b: B[V]): B[Q[V]] = new B[Q[V]]
42+
given gaq[V](using p2: priority.Prio2)(using a: A[V]): A[Q[V]] = new A[Q[V]]
43+
44+
object test1:
45+
import repro.*
46+
import repro.exports.given
47+
48+
// these will work
49+
val a = summon[A[Int]] // warn
50+
51+
52+
object test2:
53+
import repro.*
54+
import repro.qcontext.given
55+
56+
val a = summon[A[Q[Int]]] // warn

0 commit comments

Comments
 (0)