Compilation error when subclassing a class with methods inherited from a trait with mixin from Java

[raboof]

Not sure if the title correctly summarizes the problem, but the following compiled with 2.13.0-M5, but not on RC1:

Scala:

trait SuiteMixin { this: Suite =>
  def rerunner: Option[String]
}
trait Suite {
  def rerunner: Option[String] = ???
}
abstract class JournalSpec extends Suite with SuiteMixin

Java:

class MyJournalSpecTest extends JournalSpec {
}

Compiler error:

[error] /home/aengelen/dev/scala-2.13-repro/src/main/java/MyJournalSpecTest.java:1:1: MyJournalSpecTest is not abstract and does not override abstract method rerunner() in SuiteMixin
[error] class MyJournalSpecTest extends JournalSpec {
[error] (Compile / compileIncremental) javac returned non-zero exit code

Looking at the generated classes, Suite and SuiteMixin are the same between M5 and RC1:

Compiled from "Suite.scala"
public interface Suite {
  public static scala.Option rerunner$(Suite);
  public scala.Option<java.lang.String> rerunner();
  public static void $init$(Suite);
}

Compiled from "SuiteMixin.scala"
public interface SuiteMixin {
  public abstract scala.Option<java.lang.String> rerunner();
}

But JournalSpec is different: on RC1 it is missing the <java.lang.String> generic on the rerunner return type:

javap ./target/scala-2.13.0-RC1/classes/JournalSpec.class
Compiled from "JournalSpec.scala"
public abstract class JournalSpec implements Suite,SuiteMixin {
  public scala.Option rerunner();
  public JournalSpec();
}

javap ./target/scala-2.13.0-M5/classes/JournalSpec.class
Compiled from "JournalSpec.scala"
public abstract class JournalSpec implements Suite,SuiteMixin {
  public scala.Option<java.lang.String> rerunner();
  public JournalSpec();
}

[smarter]

But JournalSpec is different: on RC1 it is missing the <java.lang.String> generic on the rerunner return type:

Yes, that's intentional: scala/scala#7843

[error] /home/aengelen/dev/scala-2.13-repro/src/main/java/MyJournalSpecTest.java:1:1: MyJournalSpecTest is not abstract and does not override abstract method rerunner() in SuiteMixin

What's the output of javac -version for you ? I tried reproducing this locally and instead of this error I got javac to crash, both on 8 and on 11...:

An exception has occurred in the compiler (1.8.0_191). Please file a bug against the Java compiler via the Java bug reporting page (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com) for duplicates. Include your program and the following diagnostic in your report. Thank you. java.lang.NullPointerException at com.sun.tools.javac.comp.Flow$AssignAnalyzer.visitIdent(Flow.java:2403) at com.sun.tools.javac.tree.JCTree$JCIdent.accept(JCTree.java:2011) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scanExpr(Flow.java:1635) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.visitApply(Flow.java:2258) at com.sun.tools.javac.tree.JCTree$JCMethodInvocation.accept(JCTree.java:1465) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.tree.TreeScanner.visitExec(TreeScanner.java:175) at com.sun.tools.javac.tree.JCTree$JCExpressionStatement.accept(JCTree.java:1296) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:57) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.visitBlock(Flow.java:1883) at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:909) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.visitMethodDef(Flow.java:1811) at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:778) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.visitClassDef(Flow.java:1749) at com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:693) at com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:49) at com.sun.tools.javac.comp.Flow$BaseAnalyzer.scan(Flow.java:404) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.scan(Flow.java:1382) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.analyzeTree(Flow.java:2446) at com.sun.tools.javac.comp.Flow$AssignAnalyzer.analyzeTree(Flow.java:2429) at com.sun.tools.javac.comp.Flow.analyzeTree(Flow.java:211) at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1327) at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1296) at com.sun.tools.javac.main.JavaCompiler.compile2(JavaCompiler.java:901) at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:860) at com.sun.tools.javac.main.Main.compile(Main.java:523) at com.sun.tools.javac.main.Main.compile(Main.java:381) at com.sun.tools.javac.main.Main.compile(Main.java:370) at com.sun.tools.javac.main.Main.compile(Main.java:361) at com.sun.tools.javac.Main.compile(Main.java:56)

[raboof]

javac 11.0.3 (from java-11-openjdk-amd64 in Debian)

Shared as sbt project at https://github.com/raboof/scala-repro-11512

[raboof]

javac 1.8.0_212 (also openjdk from Debian) also produced the error without crashing for me

[smarter]

Right, seems like the crash was fixed in all recent releases of javac.

[raboof]

Indeed it looks like the generics had nothing to do with it, https://github.com/raboof/scala-repro-11512 also fails on RC1:

[error] /home/aengelen/dev/scala-2.13-repro/src/main/java/MyJournalSpecTest.java:1:1: class MyJournalSpecTest inherits abstract and default for rerunner() from types Suite and SuiteMixin
[error] class MyJournalSpecTest extends JournalSpec {
[error] (Compile / compileIncremental) javac returned non-zero exit code

Suite and SuiteMixin are the same, JournalSpec is the same except for the ACC_BRIDGE flag added in flags added in scala/scala#7843 and ACC_SYNTHETIC...

[smarter]

So, in Java the following is illegal:

interface SuiteMixin {
  public int foo();
}

interface Suite {
  default public int foo() {
    return 0;
  }
}

class Test implements Suite, SuiteMixin {
}

try/Test.java:11: error: types Suite and SuiteMixin are incompatible;
abstract class Test implements Suite, SuiteMixin {
         ^
  class Test inherits abstract and default for foo() from types Suite and SuiteMixin

Instead, you're supposed to manually override foo in Test. The override automatically generated by scalac does not count in 2.13 because it's emitted as SYNTHETIC, this is similar to #11484.

Perhaps we should go back to the old scheme for mixin forwarders in the (somewhat rare) cases where emitting a mixin forwarder is necessary for semantics and not just a performance optimization ? That would be unfortunate but I don't see a better option. /cc @retronym @lrytz

[lrytz]

The override automatically generated by scalac does not count in 2.13 because it's emitted as SYNTHETIC

I guess you mean BRIDGE?

[smarter]

No, for javac the important one is actually SYNTHETIC, synthetic methods are mostly ignored by javac, see e.g. this thread: https://www.mail-archive.com/jvm-languages@googlegroups.com/msg01195.html

[lrytz]

Ah, so scala/scala#7843 added both synthetic and bridge to mixin forwarders (https://github.com/scala/scala/blob/02f59fb504a0e8a6d29ec85da0763346e507bcfb/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala#L713). Not sure that's really requried, but anyway it woud be quite an experiment to change that at this point :-)

[lrytz]

I don't know what solution to prefer. It is a nice property that a Scala class in 2.12 hides the implementation details of extending Scala traits from Java clients. On the other hand, the bugfix helps java interop in other areas. I'm not sure we should go for a mixed approach and make the encoding dependent on the source code pattern, I think that can be very confusing..

[adriaanm]

Sounds like we can reschedule for consideration in 2.14/backlog?

[lrytz]

I'm not sure.. The goal was to improve Java interop, which was achieved as intended, but at the same tame it got worse in other areas. So maybe it's better to stay with the bugs that we have, rather than trading them with others and breaking people's code?

[smarter]

A mixin forwarder added to a class can fall into three categories:

1. The forwarder is required to get the correct runtime semantics (e.g. trait U1 {def f = 1}; trait U2 {self:U1 => override def f = 2}; class D extends U2, we need a forwarder in D otherwise the JVM has no idea which method should be called for f)
2. The forwarder is require to please javac, this is a superset of 1 that also includes cases like this issue (method is present in two traits, one is concrete and the other is abstract) and Lack of bridges for classes extending traits with erased overloads prevent extension in java #11484 (overload of the method is present in the same trait and only differs in its result type)
3. The forwarder is not required, it's just here to improve cold performance of the JVM.

Right now scalac can distinguish between 1. and 3., although it doesn't really do anything meaningful with that distinction anymore now that forwarders are unconditionally generated. I think what we need is to distinguish between 2. and 3. and use the pre-scala/scala#7843 scheme when we're in case 2. This isn't trivial however since I don't think the exact behavior of javac is specified.

All in all, for 2.13 it's probably better to revert to the situation prior to scala/scala#7843, and I'll experiment with the scheme I outlined above in Dotty. My only reason for implementing the bridge-based solution in scalac in the first place was to find the corner cases early, so mission accomplished :).

[lrytz]

My only reason for implementing the bridge-based solution in scalac in the first place was to find the corner cases early, so mission accomplished :).

Sneaky 😀