-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Change implicit resolution rule wrt implicit parameters #6071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@milessabin We discussed at the SIP meeting whether we should drop number of implicit parameters as a disambiguation criterion. But after thinking about it, I believe there are two reasons to keep it like this.
This shows that values take precedence over methods. By analogy methods without implicit parameters should take precedence over methods with implicit parameters since that would be the case anyway in the next step where we compare as if implicit parameters were normal parameters. Normal overloading rules do not have an analogue that compares number of parameters, but if they had one, preferring methods with fewer parameters would be consistent with the rule above.
|
After thinking even more about it, I now believe we should revise the rule. The idea of using implicits as a priority mechanism is so that we can write something like this: class LowPrio
class MidPrio extends LowPrio
class HighPrio extends MidPrio
implied for LowPrio
implied for MidPrio
implied for HighPrio
implied fallback given LowPrio for A
implied standard given MidPrio for A
implied specialized given HighPrio for A (usually in different modules, of course). This is a good alternative to the previous priorization by owner subclassing since it places no demands on where implied instances are put and is stable under aliasing/forwarding. But to do this reliably it should also work if some of the alternatives take additional implicit parameters. E.g. implied standard given MidPrio given B for A The "more implicit parameters is better" rule would make An alternative A is more specific than an alternative B if
Besides being more useful, this new version is also in better alignment to what happens in the normal overloading case. |
@milessabin Latest comment is implemented in 9881529. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odersky Yes, I think this works.
On reflection, perhaps not ... counterexample on its way ... |
counterexamples seems to have got lost on its way 😉? |
Sorry, still brewing. A ton of stuff seems to have broken for me recently and I'm trying to tease out exactly what and why. |
I will say that I'm finding the new rules extremely confusing. I've not been able to construct a useful example which uses the Can we consider back-tracking a bit? I think we might have given up on the "more arguments is more specific" rule too quickly. In the |
With essential arguments I assume you mean implicit ones? In that case the essential arguments must come in an implicit parameter section following the priority arguments. E.g. implied lowest given Priority.Low for T
implied highest given Priority.High given EssentialArgument for T
No, we need an ordering for an undetermined type parameter |
The method with fewer parameters wins now over methods with more parameters. The reason for the change is i3430.scala, with Nil.sum Here, orderings for any type are eligible implied instances for `sum`'s implicit `Ordering` parameter. If we prioritize arguments with implicit parameters, we have to try all tuple orderings which themselves take as many implicit parameters as the tuple has elements. This leads to an infinite recursion with very broad fanout, so the observed effect is that the compiler hangs. By prioritizing shorter implicit parameter lists, we avoid this problem.
After thinking even more about it, I now believe we should revise the rule. The idea of using implicits as a priority mechanism is so that we can write something like this: ``` class LowPrio class MidPrio extends LowPrio class HighPrio extends MidPrio implied for LowPrio implied for MidPrio implied for HighPrio implied fallback given LowPrio for A implied standard given MidPrio for A implied specialized given HighPrio for A ``` (usually in different modules, of course). This is a good alternative to the previous priorization by owner subclassing since it places no demands on where implied instances are put and is stable under aliasing/forwarding. But to do this reliably it should also work if some of the alternatives take additional implicit parameters. E.g. ``` implied standard given MidPrio given B for A ``` The "more implicit parameters is better" rule would make standard of higher priority than specialized, which defeats the purpose.
Ahh, right. So this won't be available for implicit methods until multiple implicit parameter lists are supported?
Right, but it could be instantiated earlier (that's what my scalac change does). If we can consider literally any |
Exactly. And now I understand why you were confused. Multiple implicit parameter lists were not handled correctly in overloading resolution but I just fixed that. implicied-specifity.scala contains a test implied for Arg
class Bam(val str: String)
implied lo given Low for Bam("lo")
implied hi given High given Arg for Bam("hi")
assert(the[Bam].str == "hi") |
OK, that looks a lot better. That gives us explicit prioritization. You also suggested that it would be possible to reduce the priority of an implicit by adding additional dummy arguments. Again I couldn't make that work with this PR ... is it still supposed to be possible? |
The test previously compiled by accident due to the "more implicit parameters win" rule. Once that rule got changed it should have failed, but it seems the error (illegal forward reference) was not detected. Rebasing this commit on latest master detects the error: ``` implicit val co_i: Conversion[Char, Position[CharSequence]] = the[Conversion[Char, Position[CharSequence]]] ^ co_i is a forward reference extending over the definition of co_i ``` The fix to the test avoids the error again.
If you add implicit parameters to something that did not have them before that works. Example: class Bam2(val str: String)
implied lo2 given Low for Bam2("lo")
implied mid2 given High given Arg for Bam2("mid")
implied hi2 for Bam2("hi")
assert(the[Bam2].str == "hi") But it currently does not work if the normal implicit already takes implicit parameters and we want to add implicit with reduced priority. We could probably make that work in the same way by just adding arbitrary implicit parameters. I can give it a try. |
The main case for this would be for a fallback, which typically wouldn't have implicit arguments itself, eg., implicit def high[T](implicit arg: Arg[T]): Foo[T]
implicit def low[T]: Foo[T] We want |
I fear that's beyond all our approaches. We always determine applicability without knowing whether implicit arguments can be fulfilled or not. I believe the only exception to this rule is the current scheme where we first do a contextual search and only if that fails we do a type based search. So if It seems |
The "more implicit arguments wins" rule does solve this problem though. That's why I'm suggesting we revisit the |
I don't think so. It would pick No, scratch that. That's only for overload resolution. Implicits prune differently. Have to think some more about it. |
@odersky I tried a few variations on the "disjunctive implicit argument to reduce priority" idea. It sort of works in simple cases like the one you've covered in the test. But it's really awkward if the argument we're disjoining with is more complex, ie. has type parameters, or has a type member which is then depended on by a subsequent implicit argument. One related alternative would be to have the implicit argument (ie. I think we have to find a solution for fallbacks which doesn't depend on cooperative/closed world assumptions. |
Inline traces are bulky, it's best not to show them by default
Default arguments must be passed as `given` arguments to match. Without this change we get a stackoverflow in implied-priority.scala.
What about the technique shown in the latest commit on this PR? Direct link to test source: I tried to elaborate there all techniques I know for prioritising implicits, both cooperative and after the fact. You'll need this branch to run it, as I had to fix a bug related to the interaction of |
I'm going to merge this now since I'd like to continue with another PR on top of it: replace implicit match expressions. It turns out we can simulate them using default arguments to implicit parameters. |
The method with fewer implicit parameters now wins over methods with more parameters.
The reason for the change is i3430.scala, with
Here, orderings for any type are eligible implied instances for
sum
's implicitOrdering
parameter. If we prioritize arguments with implicit parameters, we have to try all
tuple orderings which themselves take as many implicit parameters as the tuple has elements.
This leads to an infinite recursion with very broad fanout, so the observed effect is that
the compiler hangs. What we would expect instead is an implicit ambiguity error.
By prioritizing shorter implicit parameter lists, we avoid this problem. A previous PR #6034 used a
different strategy, taking implicits parameters only into account if the result types of implied instances
were unifiable. But that seems equally ad hoc, and more fragile than the current rule
To summarize: With the currently implemented rule we can add weaker alternatives to a set of implied instances by adding implicit parameters. Previously adding implicit parameters gave stronger alternatives. We can do that now as well, but using a different trick based on intersecting the implicit result type with some opaque type that erases to
Any
. This is shown inrun/implicit-specificity.scala
.Between a set of alternatives with the same number of implicit parameters nothing has changed: We still prefer the alternative with the most specific parameters.