Skip to content

Java unrecognized trait mixin method override #13104

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

Open
hamdiallam opened this issue Jul 19, 2021 · 6 comments
Open

Java unrecognized trait mixin method override #13104

hamdiallam opened this issue Jul 19, 2021 · 6 comments

Comments

@hamdiallam
Copy link

Compiler version: 3.0.1

Problem

Java cannot extend or implement an abstract class which has an abstract method overridden by a mixin trait.

trait A {
  def foo: Int
}
trait B { self: A =>
  override final def foo: Int = 1
}
abstract class JavaAWithB extends A with B
class Foo extends JavaAWithB { } // Foo must be abstract since it doesn't define `foo`
@odersky
Copy link
Contributor

odersky commented Jul 20, 2021

The Java compiler is more picky than the Scala compiler and the JVM here. To fix this manually, you'd have to make foo non final in B and write a manual override in JavaAWithB.

override def foo: Int = super.foo

The Scala compiler can't help you there. It cannot make f non-final, and it should not add the overrides automatically, since the JVM does not require them and it would just lead to unnecessary code bloat.

@odersky odersky closed this as completed Jul 20, 2021
@hamdiallam
Copy link
Author

So is it not possible for final overrides via mixins that work with Java in Scala 3? Asking because this pattern was supported in Scala 2.

@hamdiallam
Copy link
Author

hamdiallam commented Jul 20, 2021

This is important to us since removing final modifier means losing compile time safety for Scala users mixing in the appropriate trait. A concrete example of this is ClosableOnce and CloseOnce. The intent of mixing in CloseOne is to prevent conflicting Closable implementations which now breaks AbstractClosableOnce in Dotty since it doesn't recognize the overrride

@smarter
Copy link
Member

smarter commented Jul 20, 2021

It cannot make [foo] non-final, and it should not add the overrides automatically, since the JVM does not require them and it would just lead to unnecessary code bloat.

In fact, we do emit foo as non-final in B (default methods cannot be final), and we also emit an override of foo in JavaAWithB even though it's not required at runtime (for performance reasons: 6c64430), but unlike Scala 2 we emit this override with the ACC_SYNTHETIC flag to hide it from Java, this is done because it avoids/fixes a lot of issues with Java generic signatures (cf 6d0f9ca) but it does indeed mean that when Java consumes a Scala class, it can incorrectly conclude that an override is missing.

In fact, I first implemented this scheme in Scala 2 where it ended up being reverted because of this issue, I suggested a possible way forward at the time at scala/bug#11512 (comment) but haven't followed up since then, and I likely won't have the time to do so anytime soon (but maybe @lrytz would be interested? :)).

Note that while reverting 6d0f9ca would fix this issue, it would also break a bunch of the tests in that commit, to keep these tests working some careful work on generic signature generation will be necessary.

@lrytz
Copy link
Member

lrytz commented Jul 21, 2021

While the current scheme in Scala 3 is probably more correct, using the same encoding as Scala 2 would have the benefit of bug-compatibility.

@smarter do you remember if we emit incorrect generic signatures for the forwarders (in Scala 2? Scala 3?), or if we skip the signatures altogether because we can't come up with a correct one?

@smarter
Copy link
Member

smarter commented Jul 21, 2021

do you remember if we emit incorrect generic signatures for the forwarder

Scala 2 does, yes: https://github.com/scala/scala/blob/0c011547b1ccf961ca427c3d3459955e618e93d5/src/compiler/scala/tools/nsc/transform/Mixin.scala#L176-L204 and the issues with that are recorded in scala/bug#8905 and all the tests that had to be changed/removed in scala/scala#8037.

Scala 3 doesn't since the forwarders are always ACC_SYNTHETIC, and in scala/bug#11512 (comment) I suggested we only make the forwarders that are required to please javac non-synthetic, that way even if our generic signatures are sometimes lacking, less code would be affected.

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

5 participants