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

Lack of bridges for classes extending traits with erased overloads prevent extension in java #11484

Closed
martijnhoekstra opened this issue Apr 13, 2019 · 10 comments · Fixed by scala/scala#8037
Milestone

Comments

@martijnhoekstra
Copy link

martijnhoekstra commented Apr 13, 2019

given

class Foo[A]
sealed trait Trait[A] {
  def overloaded(that: List[Trait[A]]): Trait[A] = that.head
  def overloaded(that: List[Foo[A]]): Foo[A] = that.head
}
abstract class AbstractClass[A] extends Trait[A]

a java class

public class Failure extends AbstractClass<String>{
}

used to be possible, but is impossible in 2.13.0-RC1, and fails with

name clash: <A>overloaded(scala.collection.immutable.List<example.Foo<A>>) in example.Trait and <A>overloaded(scala.collection.immutable.List<example.Trait<A>>) in example.Trait have the same erasure, yet neither overrides the other

This regressed in scala/scala#7843

@martijnhoekstra martijnhoekstra changed the title Lack of bridges for classes extending traits with by-name overloads prevent extension in java Lack of bridges for classes extending traits with erased overloads prevent extension in java Apr 13, 2019
@SethTisue SethTisue added this to the 2.13.0-RC2 milestone Apr 13, 2019
@SethTisue
Copy link
Member

SethTisue commented Apr 13, 2019

when you say "but is impossible", what exactly is the error message or unexpected behavior?

in your example, is it inadvertent/inessential that the A in the methods shadows the A in the enclosing trait? (since fixed)

(initial reaction:)

The JVM itself allows overloads to differ only in return type. (I say "only" because the argument types here have the same erasure.) But the Java language does not allow it.

So rather than wondering why this doesn't work now, I'm curious why it worked before.

@martijnhoekstra
Copy link
Author

martijnhoekstra commented Apr 13, 2019

I'm amazed it worked before too, to be frank. In fact, I'm amazed I can declare the trait as such in the first place.

In 2.13.0-RC1 / 2.13.0-pre-b851a9f I get

[info] Compiling 1 Java source to C:\Users\marti\scala\test\target\scala-2.13.0-pre-b851a9f\classes ...
[error] C:\Users\marti\scala\test\src\main\java\Failure.java:3:1: name clash: <A>overloaded(scala.collection.immutable.List<example.Foo<A>>) in example.Trait and <A>overloaded(scala.collection.immutable.List<example.Trait<A>>) in example.Trait have the same erasure, yet neither overrides the other

Under 2.12.x that message doesn't exist, and if I change the scala file to

class Foo[A] {
  def doesitwork = "indeed it does"
}
sealed trait Trait[A] {
  def doesitwork = "yes it does"
  def overloaded(that: List[Trait[A]]): Trait[A] = that.head
  def overloaded(that: List[Foo[A]]): Foo[A] = that.head
}
abstract class AbstractClass[A] extends Trait[A]

object Main {
  def main(args: Array[String]): Unit = {
    val f = new Failure()
    val fs = List(f)
    val foos = List(new Foo[String]())
    println(f.overloaded(fs).doesitwork)
    println(f.overloaded(foos).doesitwork)
  }
}

to convince myself overload resolution works, it prints

Yes it does
Indeed it does

as "expected"

The shadowed type parameter was an artifact of mis-mimimization, and has been removed from the example

@lrytz
Copy link
Member

lrytz commented Apr 15, 2019

I looked at this a bit. The bytecode we generate seems valid, in 2.12.x and in 2.13.0-RC1. So this seems to be a limitation in Javac. cc @smarter

@smarter
Copy link
Member

smarter commented Apr 15, 2019

If we remove the abstract class from the equation and try:

class Foo[A]
sealed trait Trait[A] {
  def overloaded(that: List[Trait[A]]): Trait[A] = that.head
  def overloaded(that: List[Foo[A]]): Foo[A] = that.head
}
public class Failure implements Trait<String>{
}

Then javac fails compilation when Trait is compiled by either 2.12 or 2.13:

error: name clash: overloaded(List<Foo<A>>) in Trait and overloaded(List<Trait<A>>) in Trait have the same erasure, yet neither overrides the other

Now that the methods in AbstractClass are marked as bridges, javac does the same with a class that extends the trait. At least that's consistent.

@smarter
Copy link
Member

smarter commented Apr 15, 2019

I propose closing this as "working as intended". You can always manually add override that forwards to the trait methods in the abstract class to get back to the previous behavior.

@smarter smarter closed this as completed Apr 15, 2019
@smarter smarter reopened this Apr 15, 2019
@smarter
Copy link
Member

smarter commented Apr 15, 2019

(Didn't mean to close myself, sorry)

@martijnhoekstra
Copy link
Author

martijnhoekstra commented Apr 15, 2019

@smarter The abstract class go-in-between is used sometimes as a trick for providing an easy base for java to extend without having to manually wire in the forwarders. Having this as a workaround at least while the issue in javac is still open would IMO be nice. Javac interop is trying enough already, and removing this lifeline IMO makes thing significantly more annoying for those trying to make scala reasonably callable from Java.

@smarter
Copy link
Member

smarter commented Apr 15, 2019

Is there an actual issue open against javac for this or is it the expected behavior ?

@martijnhoekstra
Copy link
Author

I'm not sure, I figured it was an open issue based on earlier phrasing of @lrytz that I read too much into ("limitation of javac")

@lrytz
Copy link
Member

lrytz commented Apr 16, 2019

I just said "seems to be a limitation in Javac", also because it's much easier to blame others :-) But they might not consider this a bug. I don't know how important it is for Javac to interop with arbitrary valid bytecode, vs bytecode that Javac generates. We can report the issue through https://bugreport.java.com/bugreport

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

Successfully merging a pull request may close this issue.

4 participants