Imported From: https://issues.scala-lang.org/browse/SI-9871?orig=1
Reporter: Moses Nakamura (mosesn)

@retronym said:
Cross linking with: scala/scala-dev#157, in which we hope to be able to improve this before we commit 2.12.0

Moses Nakamura (mosesn) said:
Does scala/scala#5307 fix this?

@SethTisue said:
[~mosesn] we think it may be fixed, but I can't easily test it since the reproduction info at twitter/scrooge#238 has multiple Scala-based dependencies. to be sure of a fix, everything would have to be rebuilt using the latest compiler.

if you can supply a standalone reproduction, I'd be happy to check it against PR 5307 for you (and add it to our suite of regression tests, too!)

another possibility would be for you to wait for 2.12.0-RC1 to come out and then attempt to rebuild all your libs against it.

I'm retargeting for 2.12.1 because this isn't a blocker for 2.12.0-RC1 or 2.12.0. but we're definitely interested in working with you to find out if this has been resolved.

@SethTisue said (edited on Aug 23, 2016 10:14:28 PM UTC):
not completely sure it's relevant to your actual code, but I'd like to note it here, since it came up for me right away experimenting with implementing the English description ("We have a class...") above:

any time you have the Foo extends FunctionN pattern you may run into https://github.com/scala/scala-dev/issues/138 which prevents even something as simple as:

abstract class Service extends (Int => String)
(_.toString): Service

from compiling. (Noticed first by Lukas, scala/scala#5307 (comment))

Adriaan has a WIP patch at scala/scala#5126 but its future is unclear (in any case, it won't make 2.12.0). making this compile makes other things not compile (since new ambiguities are possible), so it's not clear-cut what to do.

Moses Nakamura (mosesn) said:
Happy to wait. We're already publishing against 2.12.0-M4, so if we can get our deps onboard the RC1 train, we'll try it out.

@SethTisue said (edited on Aug 23, 2016 10:44:31 PM UTC):
OK cool.

in other news, scala/scala#5357 attempts to make the code in my last comment compile (without addressing all of 5126), and may make RC1

@SethTisue said:
[~mosesn] should this ticket be closed now?

Moses Nakamura (mosesn) said:
@SethTisue I haven't had a chance to check yet whether it's fixed, we're still blocked on scalatest migration. Will update you in once we're cut over.

Moses Nakamura (mosesn) said (edited on Dec 2, 2016 9:35:45 PM UTC):
@SethTisue, we just tried the nightly build (2.12.1-371bc2c-nightly) but it still fails with the same error message. You can try to reproduce it yourself in finagle-core.

@xeno-by said (edited on Dec 3, 2016 12:15:47 AM UTC):
Here's a minimized repro

abstract class MyFunction1[T, U] extends Function1[T, U]

object Test {
  def foo(x: Function1[Int, Int]) = ???
  def foo(x: MyFunction1[Int, Int]) = ???

  val myf: MyFunction1[Int, Int] = ???
  foo(myf)
}

16:14 ~/Projects/2121/sandbox (2.12.x)$ s
Test.scala:8: error: ambiguous reference to overloaded definition,
both method foo in object Test of type (x: MyFunction1[Int,Int])Nothing
and  method foo in object Test of type (x: Int => Int)Nothing
match argument types (MyFunction1[Int,Int])
  foo(myf)
  ^

Moses Nakamura (mosesn) said:
We also noticed that the problem goes away if we add another abstract method, or if it's still a SAM but it doesn't extend Function1.

It seems like the problem arises only when it both extends Function1 and it's a SAM.

@xeno-by said (edited on Dec 3, 2016 1:06:26 AM UTC):
There's weird behavior exhibited by isAsSpecific in Infer.scala: https://github.com/scala/scala/blob/ccfa36071f0623d64474f37b12d40a838aac1b4e/src/compiler/scala/tools/nsc/typechecker/Infer.scala#L808.

In particular, it thinks that:

isAsSpecific((x: Int => Int)Nothing, (x: MyFunction1[Int,Int])Nothing) = true

In order to check specificity, isAsSpecific calls isCompatible (https://github.com/scala/scala/blob/ccfa36071f0623d64474f37b12d40a838aac1b4e/src/compiler/scala/tools/nsc/typechecker/Infer.scala#L294) passing Int => Int and MyFunction1[Int, Int] as arguments to that method.

The isCompatible check succeeds because it falls through into the isCompatibleSam case. It thinks that Int => Int is compatible with MyFunction1[Int, Int].

After isCompatible returns true, isAsSpecific concludes that the overload with Int => Int is as specific as the overload with MyFunction1[Int, Int]. As a result, overload resolution fails with an ambiguity.

I think that isCompatible works incorrectly. Not every Int => Int is compatible with MyFunction1[Int, Int]. Only lambdas are compatible with that type. Am I correct in pointing this out, or that was done deliberately?

@adriaanm said:
Your assessment is correct, but, as you suspected, this is for backwards compatibility.

We pretend all expressions of function type are compatible with the corresponding sam type, even if it's only really true for function literals, as you say. This is to make overloading work for the other variation when MyFunction1 is a sam, but not a subclass of Function1. Is there any way you could remove that superclass? I know that would break on 2.11 :-/ We have also considered a @NotASam annotation.

@adriaanm said:
Regarding timing, we plan to tag 2.12.1 on Monday. I'm sorry we won't be able to address this by then.

@xeno-by said:
@adriaanm I have implemented your @notASam idea in a compiler plugin: https://github.com/xeno-by/workaround9871.

@xeno-by said:
Do you guys think that @notASam could become a thing in 2.12.2? Or you're leaning towards something else?

Moses Nakamura (mosesn) said:
@adriaanm we ended up deleting the old method. We didn't want to make open source users add a compiler plugin for the library. With that said, this is a pretty painful migration step. In 2.12, users will have essentially the same API as we exposed previously, but for 2.11 compatibility, we need to pretend that it doesn't exist. It would be great if you could improve resolution so that you don't break backwards compatibility with 2.11 anymore.

@adriaanm said:
Yes, unfortunately, due to the nature of this problem, making your case work would break someone else's. This would be another nice application of configurable error reporting (proposed but never implemented), which you could use here to @deprecate the method on 2.12 to bar it from consideration during compilation on 2.12, while leaving the definition in place (the design space needs exploring -- I could imagine still emitting bytecode for the method or eliding it entirely).

I'm experiencing the same issue in Scala 2.13.0-M5.

I'm using a Java library called Bukkit and the BukkitScheduler#runTask method is overloaded
As the last parameter it accepts a Runnable (a functional interface), and the overload (which is depcreated) accepts a BukkitRunnable (an abstract class with a single abstract method).

When calling this method using a lambda expression in Scala 2.12, it compiles my lambda to an instance of the BukkitRunnable, so it essentially chooses the wrong overload. In Scala 2.13 however the compiler gives me an error:

[ERROR] C:\Users\Test\Dropbox\Spigot\JannyRecipes\src\main\scala\xyz\janboerman\recipes\gui\AnvilButton.scala:29: overloaded method value runTask with alternatives:
  (x$1: org.bukkit.plugin.Plugin,x$2: org.bukkit.scheduler.BukkitRunnable)org.bukkit.scheduler.BukkitTask <and>
  (x$1: org.bukkit.plugin.Plugin,x$2: Runnable)org.bukkit.scheduler.BukkitTask
 cannot be applied to (P, () => net.wesjd.anvilgui.AnvilGUI)
[ERROR]             plugin.getServer.getScheduler.runTask(plugin, () => {
[ERROR]                                           ^
[ERROR] one error found

javac doesn't have this problem because in Java only functional interfaces can be implemented using lambda expressions, not abstract classes. It would be nice if the scala compiler picked the same overload as the java compiler in such a case.