Skip to content

Commit

Permalink
Fix healAmbiguous to compareAlternatives with disambiguate = true
Browse files Browse the repository at this point in the history
On the final result, compared with all the ambiguous candidates we are trying
to recover from. We should still use `disambiguate = false` when filtering the
`pending` candidates for the purpose of warnings, as in the other cases.

Before, we could heal an ambiguous SearchFailure with a candidate which was
considered better only under the alternative given prioritization scheme,
see scala#21045 for details.
  • Loading branch information
EugeneFlesselle committed Aug 6, 2024
1 parent 9b6a73e commit 0de4a2f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 23 deletions.
18 changes: 10 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1439,9 +1439,14 @@ trait Implicits:
* candidate that is strictly better than the failed candidate(s).
* If no such candidate is found, we propagate the ambiguity.
*/
def healAmbiguous(fail: SearchFailure, betterThanFailed: Candidate => Boolean) =
val newPending = remaining.filter(betterThanFailed)
rank(newPending, fail, Nil).recoverWith(_ => fail)
def healAmbiguous(fail: SearchFailure, ambiguousCands: List[RefAndLevel]) =
def betterThanAll(newCand: RefAndLevel, disambiguate: Boolean): Boolean =
ambiguousCands.forall(compareAlternatives(newCand, _, disambiguate) > 0)

val newPending = remaining.filter(betterThanAll(_, disambiguate = false))
rank(newPending, fail, Nil) match
case found: SearchSuccess if betterThanAll(found, disambiguate = true) => found
case _ => fail

negateIfNot(tryImplicit(cand, contextual)) match {
case fail: SearchFailure =>
Expand All @@ -1456,8 +1461,7 @@ trait Implicits:
else
// The ambiguity happened in a nested search: to recover we
// need a candidate better than `cand`
healAmbiguous(fail, newCand =>
compareAlternatives(newCand, cand) > 0)
healAmbiguous(fail, cand :: Nil)
else
// keep only warnings that don't involve the failed candidate reference
priorityChangeWarnings.filterInPlace: (critical, _) =>
Expand All @@ -1476,9 +1480,7 @@ trait Implicits:
// The ambiguity happened in the current search: to recover we
// need a candidate better than the two ambiguous alternatives.
val ambi = fail.reason.asInstanceOf[AmbiguousImplicits]
healAmbiguous(fail, newCand =>
compareAlternatives(newCand, ambi.alt1) > 0 &&
compareAlternatives(newCand, ambi.alt2) > 0)
healAmbiguous(fail, ambi.alt1 :: ambi.alt2 :: Nil)
}
}
case nil =>
Expand Down
4 changes: 0 additions & 4 deletions tests/neg/i21212.check

This file was deleted.

11 changes: 0 additions & 11 deletions tests/neg/i21212.scala

This file was deleted.

11 changes: 11 additions & 0 deletions tests/pos/i21212.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,14 @@ class UsingArguments[F[_]](using Temporal[F])(using err: MonadError[F, Throwable
val bool: F[Boolean] = ???
def works = toFunctorOps(bool).map(_ => ()) // warns under -source:3.5


object Minimization:

trait A
trait B extends A

def test1(using a1: A)(using b1: B) = summon[A] // picks (most general) a1
def test2(using a2: A)(implicit b2: B) = summon[A] // picks (most general) a2, was ambiguous
def test3(implicit a3: A, b3: B) = summon[A] // picks (most specific) b3

end Minimization

0 comments on commit 0de4a2f

Please sign in to comment.