Skip to content

Structural member calls can fail with AbstractMethodError due to mismatch between bytecode signature and statically known method type #12297

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
smarter opened this issue Dec 27, 2020 · 7 comments

Comments

@smarter
Copy link
Member

smarter commented Dec 27, 2020

Using Scala 2.13.4,

scala> import scala.language.reflectiveCalls

scala> class Sink[A] { def put(x: A): Unit = {} }
class Sink

scala> val a = new Sink[String]
val a: Sink[String] = Sink@5dbbb292

scala> val b: { def put(x: String): Unit } = a
val b: AnyRef{def put(x: String): Unit} = Sink@5dbbb292

scala> b.put("")
java.lang.NoSuchMethodException: Sink.put(java.lang.String)
  at java.lang.Class.getMethod(Class.java:1786)
  at reflMethod$Method1(<console>:1)
  ... 32 elided

There are other ways to run into trouble, a similar problem with lambdas was identified in #10334 (comment) by @lrytz, this was fixed by generating lambdas with bridges in the backend but I believe this isn't a sufficient fix since Java-generated lambdas won't have such bridges.

I think the correct way to fix that involves changing the logic used at runtime to choose which method to call: basically we need to call to check all overloads of put to see which one could be a valid choice given the current signature. This is tricky since there might be more than one, in which case we could try to take the more precise one if it exists and/or fail with a runtime exception anyway.

@smarter smarter changed the title Structural member calls can fail with AbstractMethodError due to variance (among other things) Structural member calls can fail with AbstractMethodError due to mismatch between bytecode signature and statically known method type Dec 27, 2020
@sjrd
Copy link
Member

sjrd commented Dec 28, 2020

I think the correct way to fix that involves changing the logic used at runtime to choose which method to call

Trying to do anything at run-time will

  • fail to resolve overloading in the same way as at compile-time, creating unsoundness, and
  • fail to work at all in JS and Native, since their handling of reflective calls needs the correct parameter types at link time.

@smarter
Copy link
Member Author

smarter commented Dec 28, 2020

fail to resolve overloading in the same way as at compile-time, creating unsoundness, and

If there's only one valid overload then it should match what happens at compile-time, if there's more than one I agree we likely have no choice but to raise an exception.

@sjrd
Copy link
Member

sjrd commented Dec 28, 2020

Whether or not there are several valid overloads will depend on run-time values. So your code will work for some values at run-time, but will not work with other values, even though both compile. That is a very bad situation to get your language into.

@smarter
Copy link
Member Author

smarter commented Dec 28, 2020

Sure, but it's strictly better than the status-quo, the only other alternative I can see is to completely remove reflection-based structural types from the language, or am I missing something?

@sjrd
Copy link
Member

sjrd commented Dec 28, 2020

I don't think it's strictly better. Currently, whether your structural call succeeds only depends on the definition of its receiver. If you know that your receiver is good, then all is fine, and is resolved in the same way as compile-time.

You're proposing to trade that for also allow it to work for more receivers but only for some values of the parameters, and in addition lose the guarantee that the same resolution is done as at compile-time. And also in a way that cannot be made portable. That is not a strict improvement.

@dwijnand
Copy link
Member

Dupe of #10414? Couldn't determine how it could be distinct from that.

@smarter
Copy link
Member Author

smarter commented Jan 5, 2021

We can fuse the two issues yeah.

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