Skip to content

Wrong overload resolution: picks Any over Long with an Int argument #7630

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

Closed
sjrd opened this issue Nov 27, 2019 · 5 comments
Closed

Wrong overload resolution: picks Any over Long with an Int argument #7630

sjrd opened this issue Nov 27, 2019 · 5 comments

Comments

@sjrd
Copy link
Member

sjrd commented Nov 27, 2019

minimized code

object Asserts {
  def assertEquals(expected: Any, actual: Any): Unit = {
    println(1)
    assert(expected.equals(actual), s"expected $expected but got $actual")
  }

  def assertEquals(expected: Long, actual: Long): Unit = {
    println(2)
    assert(expected == actual, s"expected $expected but got $actual")
  }
}

object Test {
  def main(args: Array[String]): Unit = {
    def foo(): Long = 42L

    Asserts.assertEquals(42, foo()) // an Int and a Long
  }
}

observed behavior

The first overload, taking Anys, is selected over the second one.

1
Exception in thread "main" java.lang.AssertionError: assertion failed: expected 42 but got 42
        at dotty.DottyPredef$.assertFail(DottyPredef.scala:17)
        at Asserts$.assertEquals(hello.scala:4)
        at Test$.main(hello.scala:17)
        at Test.main(hello.scala)

expectation

Like in Scala 2, the second overload, taking Longs, should be selected, so that it prints:

2

note

The API of Asserts above is defined in JUnit; it cannot be changed.

workaround

We can work around the issue by making sure that both arguments truly are Longs:

object Test {
  def main(args: Array[String]): Unit = {
    def foo(): Long = 42L

    Asserts.assertEquals(42L, foo()) // Long and Long
  }
}
@odersky
Copy link
Contributor

odersky commented Nov 27, 2019

I believe this is a consequence that weak conformance is gone. In Dotty an Int is converted to a Long by a standard implicit conversion. If I translate the examples to classes and conversions:

object X {
  case class Int1()
  case class Long1()
  implicit def i2l(x: Int1): Long1 = Long1()

  def ae(expected: Any, actual: Any): Unit = {
    println(1)
    assert(expected.equals(actual), s"expected $expected but got $actual")
  }

  def ae(expected: Long1, actual: Long1): Unit = {
    println(2)
    assert(expected == actual, s"expected $expected but got $actual")
  }
}

object Test {
  def main(args: Array[String]): Unit = {
    import X._
    ae(Int1(), Long1())
  }
}

I get the same output for both scalac and dotc:

1
Exception in thread "main" java.lang.AssertionError: assertion failed: expected Int1() but got Long1()

@odersky
Copy link
Contributor

odersky commented Nov 27, 2019

Since I guess we do not want to bring weak conformance back, I see no way to fix this. Close?

@sjrd
Copy link
Member Author

sjrd commented Nov 27, 2019

Can it not be fixed directly in overload resolution?

If not fixed, we risk breaking quite a lot of testing code using JUnit, with runtime assertions. That's not a good migration story. :(

@smarter
Copy link
Member

smarter commented Nov 27, 2019

Yeah, isn't Scala overloading resolution supposed to always match Java ?

@odersky
Copy link
Contributor

odersky commented Nov 28, 2019

Yeah, isn't Scala overloading resolution supposed to always match Java ?

There's no such requirement. But we try to come close, for compatibility's sake.

So, we'd have to bring weak conformance back? That would be unfortunate.

odersky added a commit that referenced this issue Dec 11, 2019
Fix #7630: Use weak conformance in overloading resolution
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

3 participants