Skip to content
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

Different algorithm for handling SAMs in overloading #11990

Closed
wants to merge 3 commits into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Apr 5, 2021

This an alternative to #11945.

Here we always try SAM conversions alongside subtyping but when comparing
alternatives we prefer normal subtyping over SAM conversions.

Unfortunately, this also fails zio

Overloading resolution had two stages to determine when an alternative is applicable"

 1. Match with subtyping only
 2. Match with implicit conversions and SAM conversions

If some alternatives are eligible under (1), only those alternatives are considered.
If no alternatives are eligible under (1), alternatives are searched using (2).

This behavior is different from Scala 2, which seems to use SAM conversions more aggressively.
I am not sure what Scala 2 does. One obvious change would be to allow SAM conversions under (1).
But I am reluctant to do that, since a SAM conversion can easily be introduced by accident.
For instance, we might have two overloaded variants like this:
```scala
   def foo(x: C) = ...
   def foo(x: A => B) = ...
   foo((x: A) => x.toB)
```
Now, if `C` happens to be a SAM type that has an abstract method from A to B, this would be ambiguous.
Generally, it feels like a SAM conversion could too easily be considered by accident here.

On the other hand, if `C` was marked with `@FunctionalInterface` it would explicitly expect function
arguments, so then we should treat it as morally equivalent to a function type in Scala.

This is what this commit does. It refines the priority for searching applicable methods as follows:

 1. Match with subtyping and with SAM conversions to functional interfaces
 2. Match with implicit conversions and SAM conversions to other types
Here we always try SAM conversions alongside subtyping but when comparing
alternatives we prefer normal subtyping over SAM conversions.

Unfortunately, this also fails zio
smarter added a commit to dotty-staging/dotty that referenced this pull request Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM by Java.

Fixes scala#11938.
smarter added a commit to dotty-staging/dotty that referenced this pull request Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
smarter added a commit to dotty-staging/dotty that referenced this pull request Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
@odersky odersky closed this Apr 17, 2021
smarter added a commit to dotty-staging/dotty that referenced this pull request Apr 17, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
michelou pushed a commit to michelou/dotty that referenced this pull request Apr 18, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant