scala / scala3

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

Java unrecognized trait mixin method override #13104

Open hamdiallam opened 3 years ago

hamdiallam commented 3 years ago

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 commented 3 years ago

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.

hamdiallam commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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: https://github.com/lampepfl/dotty/commit/6c6443086071774431dae93709b4102ce3c4d15e), 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 https://github.com/lampepfl/dotty/commit/6d0f9ca3ae4c7130c1ee18e0e21f283ff6cb21f3) 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 https://github.com/scala/bug/issues/11512#issuecomment-489424170 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 https://github.com/lampepfl/dotty/commit/6d0f9ca3ae4c7130c1ee18e0e21f283ff6cb21f3 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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/scala/bug/issues/8905 and all the tests that had to be changed/removed in https://github.com/scala/scala/pull/8037.

Scala 3 doesn't since the forwarders are always ACC_SYNTHETIC, and in https://github.com/scala/bug/issues/11512#issuecomment-489424170 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.