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

SAM types + Overloading + Eta-expansion #4364

Closed
allanrenucci opened this issue Apr 24, 2018 · 8 comments
Closed

SAM types + Overloading + Eta-expansion #4364

allanrenucci opened this issue Apr 24, 2018 · 8 comments

Comments

@allanrenucci
Copy link
Contributor

The following code snippet fails to compile with Dotty. (Works with scalac)

class Test {
  def println(): Unit = ()
  def println(x: String): Unit = ()

  def foo(x: java.util.function.Consumer[String]) = 1
  foo(println(_))      // ok
  foo(x => println(x)) // ok
  foo(println)         // error
}
-- [E007] Type Mismatch Error: tests/allan/Test.scala:26:6 ---------------------
26 |  foo(println)
   |      ^^^^^^^
   |      found:    Unit
   |      required: java.util.function.Consumer[String]
   |      
one error found
@odersky
Copy link
Contributor

odersky commented Apr 27, 2018

I tried out some things. The problem is, if we allow that, the following also compiles:

new java.io.ObjectPutputStream(println)

println replaces the abstract write method of ObjectOutputStream. I find this too surprising. It would be better to always demand that the argument to a SAM is an explicit closure, i.e. to forbid eta-expansion in this case.

@allanrenucci
Copy link
Contributor Author

I agree. This currently compiles:

class Test {
  def foo(x: Any): Unit = ()
  def test = {
    new java.io.ObjectOutputStream(foo)
  }
}

@Glavo
Copy link
Contributor

Glavo commented May 11, 2018

@odersky I think this problem stems from the functional interface, not Eta-expansion. We should check the functional interface and trigger compiler warnings when the interface is not annotated with @FunctionalInterface. When the target interface is a Scala trait and it not annotated by @FunctionalInterface, we should let the compiler report an error

@smarter
Copy link
Member

smarter commented May 11, 2018

This would add a lot of unnecessary warnings. Java doesn't emit any warning when using a SAM type that is not annotated with @FunctionalInterface.

@Glavo
Copy link
Contributor

Glavo commented May 11, 2018

But Java's behavior creates a lot of unexpected functional interfaces, and in Java, lambda expressions and method reference expressions only be used in scenes where SAM conversion occurs without any confusion, different from Scala's lambda expression and eta-expansion.
I think that in any case, the author of the library or the user of the library should clearly know which interfaces will be used as functional interfaces, users should know what they are doing, and use @unchecked to prevent triggering compiler warnings.

@Glavo
Copy link
Contributor

Glavo commented May 11, 2018

In fact, most of the commonly used functional interfaces in the JDK have been annotated as @FunctionalInterface. It may not be so common for users to actually disable warnings.

@allanrenucci
Copy link
Contributor Author

I guess it worth doing the experiment and see how many warnings we get in our build and the community build

@Blaisorblade
Copy link
Contributor

On the one hand I agree Scalac's behavior is surprising here. OTOH, changing this adds an incompatibility, so we'd have to add the warnings at least to 2.14.

Even if the impact were minor on our community build, we'd need to try this out on Scalac's one to gauge actual impact. The experiment might still help: if the impact is major on our build, we can't change it ourselves.

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

No branches or pull requests

5 participants