Fix #24193: Do not try equally preferred alternatives in case of divergent implicit#25109
Fix #24193: Do not try equally preferred alternatives in case of divergent implicit#25109Alex1005a wants to merge 3 commits intoscala:mainfrom
Conversation
… divergent implicit
| case _ => fail | ||
| end healAmbiguous | ||
|
|
||
| inline def unrecoverableDivergentImplicit(failure: SearchFailureType): Boolean = { |
There was a problem hiding this comment.
I don't get the logic here. If I understand correctly, if there is a diverging implicit we require an alternative that is strictly better than that? What would happen if there are two alternatives of equal precedence, we get the divergent one first, but the second one succeeds. Will the second one be returned? I don't see how that could be the case.
There was a problem hiding this comment.
If divergent first beats success later the logic is just wrong, not what is specced.
There was a problem hiding this comment.
What would happen if there are two alternatives of equal precedence, we get the divergent one first, but the second one succeeds. Will the second one be returned?
For this, I used the condition (pt frozen_=:= failure.expectedType). If the first alternative is divergent, then if the second one succeeds, there will be ambiguity, as in the case of the code in the comment, where there is ambiguity between good and diverge(good).
But I didn't take into account that first alternative might fail not only because of diverging. For example, in the code below, a diverging implicit error will be thrown, although implicit good should be used, since no implicit instance of Int can be found for diverge.
trait A[T]
implicit def diverge[T](implicit a: A[T], i: Int): A[T] = ???
implicit def good[T](implicit d1: DummyImplicit, d2: DummyImplicit): A[T] = ???
def geta[T: A] = ???
def testgeta = {
geta // diverging implicit search
}So I need to change the code to account for this, or come up with another way to fix the issue.
There was a problem hiding this comment.
If the first alternative is divergent, then if the second one succeeds, there will be ambiguity, as in the case of the code in the #25109 (comment), where there is ambiguity between good and diverge(good).
Note the spec says it should succeed with the second in this case. In Scala 3, a divergent implicit is a failure just like an implicit not found. Either failure can be superseded by subsequent successes. In Scala 2 it was different, there a divergent implicit aborted the whole search.
Maybe one could take a closer look at the problematic implicits in cats-effect and/or fs2 and see whether something can be improved there.
There was a problem hiding this comment.
So it turns out that this is a bug, that the code from the comment compiles with an ambiguous implicit error?
There was a problem hiding this comment.
Also, why should diverge(good) win against good? I understand why this happens in this code:
trait A[T]
implicit def good[T](implicit d1: DummyImplicit, d2: DummyImplicit): A[T] = ???
object Inside {
implicit def diverge[T](implicit a: A[T]): A[T] = ???
def geta[T: A] = ???
def testgeta = {
geta // diverge(good) found
}
}But why should this happen when both implicits are in the same scope and equally preferred?
There was a problem hiding this comment.
I unfortunately don't have time to figure out what happens here. Someone needs to disect and minimize and compare with the spec to be able to tell whether this is as expected or not.
There was a problem hiding this comment.
disect and minimize
It seems like the examples are fairly minimal and don't have any third-party dependencies. Or do you mean to minimize the implicit examples in cats-effect and check them?
compare with the spec
Could you please elaborate what exactly you're referring to? There's a spec in the Scala 3 repository about implicits, but it seems to be a slightly modified copy of the Scala 2 documentation. I assume the 'Changes in Implicit Resolution' documentation page could be considered a spec?
Fixes #24193
In case of implicit search specified in the test, it takes O(n!) attempts to return the error:
No given instance of type Clock.To avoid this, a skip of attempts is added for equally preferred alternatives in case of a divergent implicit error (with the same type as the one being searched for), because if the implicit is found, there will still be an ambiguous implicit error.
Because of this condition, in some places there may now be a divergent implicit error instead of an ambiguous implicit.
In theory, this could result in no constraints being imposed on the result type where they were previously, but I haven't found a way to reproduce this yet.
This PR also possibly fixes #18763 (I definitely saw a decrease in the number of implicit searches when debugging).