scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.8k stars 1.05k forks source link

Regression as side-effect of mapping context bounds to using arguments #21506

Closed WojciechMazur closed 1 day ago

WojciechMazur commented 2 weeks ago

Based on OpenCB failure in paulbutcher/scalamock

It seems like a side effect of changing mapping for context bounds is a different behaviour when using overloaded functions. Previously compiler seemed to try apply non-implicit arguments whenever implicit ones were not applicable. Now only the first overload is chosen.

Behaves the same under Scala 3.5 with -source:future which is expected.

Compiler version

3.6 nightly Bisect points to b8f9c2c2cc17abc1caf14199dfd67ca28130aef2

Minimized code

import scala.language.implicitConversions
trait MockFunctions:
  protected case class FunctionName(name: String)
  protected implicit def functionName(name: String): FunctionName = ???
  protected def mockFunction[T1, R: Defaultable](name: FunctionName): MockFunction1[T1, R] = ???
  protected def mockFunction[T1, R: Defaultable]: MockFunction1[T1, R] = ???

trait Defaultable[T]
object Defaultable:
  given Defaultable[Unit] = ???

class MockFunction1[T1, R](name: Symbol):
  def apply(v1: T1): R = ???

@main def Test = new MockFunctions {
    val infer = mockFunction[String, Unit]("mockFun")
    val fails: MockFunction1[String, Unit] = infer
}

Output

[error] ./test.scala:17:46
[error] Found:    (infer : Unit)
[error] Required: MockFunction1[String, Unit]
[error]     val fails: MockFunction1[String, Unit] = infer

Expectation

WojciechMazur commented 2 weeks ago

Additional example based on OpenCB failure in ruippeixotog/akka-testkit-specs2

class MySpec extends AkkaMatchers:
  val _ = receive("hello")

trait AkkaMatchers:
  def receive: UntypedFullReceiveMatcher = ???

trait UntypedFullReceiveMatcher extends FullReceiveMatcher[String]:
  def apply[A: scala.reflect.ClassTag]: FullReceiveMatcher[A]

trait FullReceiveMatcher[A]:
  def apply(msg: A): FullReceiveMatcher[A]

yields:

-- [E051] Reference Error: /Users/wmazur/projects/sandbox/test.scala:2:10 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2 |  val _ = receive("hello")
  |          ^^^^^^^
  |          Ambiguous overload. The overloaded alternatives of method apply in trait UntypedFullReceiveMatcher with types
  |           [A](using evidence$1: scala.reflect.ClassTag[A]): FullReceiveMatcher[A]
  |           (msg: String): FullReceiveMatcher[String]
  |          both match arguments (("hello" : String))
  |
odersky commented 1 day ago

The first example essentially comes down to this, once we get all type arguments and implicit arguments out of the way:

case class FunctionName(name: String)
implicit def functionName(name: String): FunctionName = ???

class MockFunction1:
  def apply(v1: String): Unit = ???

def mockFunction(name: FunctionName): MockFunction1 = ???
def mockFunction: MockFunction1 = ???

val infer = mockFunction("abc")
val fail: MockFunction1 = infer

We have two overloaded variants. One takes a FunctionName, the other returns a value with an apply method that takes a String. We pass a String and pick the second alternative. In the full example, we previously implicitly converted the String to a FunctionName and called the first alternative. But I think that's wrong. If we can make sense of the example without implicit conversions, we should do so. The reason for the previous behavior is that we had some mistakes in overloading resolution with implicit arguments that were fixed for using clauses.

The line in question is line 2119 in Applications.scala:

          else if alts.exists(_.widen.stripPoly.isContextualMethod) then
            return resolveMapped(alts, alt => stripImplicit(alt.widen), pt)

Here, we skip any using clauses in parameter lists before we proceed matching the rest of the applications. That line makes sense IMO. We did not do the same for implicits, which was a mistake. But we did not correct that mistake since we wanted to keep the status quo as much as possible for implicits.

odersky commented 1 day ago

The Akka example is similar, and comes down to the same problem.

odersky commented 1 day ago

So I think this has to be a won't fix. There's nothing we can reasonably do here. The inference is as intended.