Skip to content

No generic signature emitted for trait getter #6350

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
smarter opened this issue Apr 22, 2019 · 7 comments
Open

No generic signature emitted for trait getter #6350

smarter opened this issue Apr 22, 2019 · 7 comments

Comments

@smarter
Copy link
Member

smarter commented Apr 22, 2019

Given:

trait Foo {
  val bla: List[String] = List("foo")
}
class Bar extends Foo

Decompiling with cfr shows:

public interface Foo {
    default public void $init$() {
    }

    public List<String> bla();

    default public List initial$bla() {
        return package$.MODULE$.List().apply((Seq)Predef$.MODULE$.wrapRefArray((Object[])new String[]{"foo"}));
    }
}
public class Bar
implements Foo {
    private final List bla;

    public Bar() {
        this.bla = Foo.super.initial$bla();
        Foo.super.$init$();
    }

    public List bla() {
        return this.bla;
    }
}

Notice the lack of generic signature on bla() in Bar, since trait getters are emitted in Mixin after erasure, getting a generic signature is not trivial. For mixin forwarders we work around this by emitting them as bridges (cf 6d0f9ca), but we should probably not do that for trait setters since they are not legitimately bridges.

We could also try to move trait getter (and setter, and maybe more stuff) before erasure since that's what scalac appear to be doing.

@smarter
Copy link
Member Author

smarter commented Apr 22, 2019

The lack of generic signature for trait getters leads to the following warning when compiling dotty-sbt-bridge in the Dotty build:

> dotty-sbt-bridge/compile
[info] Updating dotty-sbt-bridge...
[info] Done updating.
[info] Compiling 9 Java sources to /home/smarter/opt/dotty/sbt-bridge/src/target/classes ...
[warn] /home/smarter/opt/dotty/sbt-bridge/src/xsbt/DelegatingReporter.java:24:1: dotty$tools$dotc$reporting$UniqueMessagePositions$$positions() in dotty.tools.dotc.reporting.AbstractReporter implements dotty$tools$dotc$reporting$UniqueMessagePositions$$positions() in dotty.tools.dotc.reporting.UniqueMessagePositions
[warn]   return type requires unchecked conversion from scala.collection.mutable.HashMap to scala.collection.mutable.HashMap<scala.Tuple2<dotty.tools.dotc.util.SourceFile,java.lang.Object>,java.lang.Object>
[warn] final public class DelegatingReporter extends AbstractReporter {
[info] Done compiling.

@odersky
Copy link
Contributor

odersky commented Mar 30, 2020

We could also try to move trait getter (and setter, and maybe more stuff) before erasure since that's what scalac appear to be doing.

Note that the mixin in scalac is somewhat of a mess, precisely because there is no good time to do it. So we should step very carefully before trying to copy things.

@lrytz
Copy link
Member

lrytz commented Jun 3, 2021

This also shows up in Akka. Looking at the bytecode, the trait getter, setter and also the field are lacking signatures.

trait L[+T]
trait T { private var transitionEvent: L[String] = ??? }
class C extends T
public class Test extends C { }
Test.java:1: warning: [unchecked] T$$transitionEvent() in C implements T$$transitionEvent() in T
public class Test extends C {
       ^
  return type requires unchecked conversion from L to L<String>

Note that the mixin in scalac is somewhat of a mess

Mixin got a good overhaul for 2.12 in scala/scala#5141 (and, I guess, some follw-ups).

@michelou
Copy link
Contributor

Here is a minimization of the warning mentioned by @smarter (and taken from sbt-bridge); it consists of 4 files which are compiled in 2 steps (see below) :

Source file src\main\scala\Reporter.scala :

// minimized from compiler/src/dotty/tools/dotc/reporting/Reporter.scala

abstract class Reporter // extends interfaces.ReporterResult

Source file src\main\scala\UniqueMessagePositions.scala :

// minimized from compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala

import scala.collection.mutable

trait UniqueMessagePositions extends Reporter {
  private val positions = new mutable.HashMap[(/*SourceFile*/String, Int), Int]
}

Source file src\main\scala\AbstractReporter.scala :

// minimized from compiler/src/dotty/tools/dotc/reporting/AbstractReporter.scala

abstract class AbstractReporter extends Reporter with UniqueMessagePositions

Java source file src\test\java\DelegatingReporter.java

// minimized from sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java

final public class DelegatingReporter extends AbstractReporter {
}

2-steps compilation (Java 1.8.0u302 and Scala 3.0.2-RC2 on Windows):

call %SCALA3_HOME%\bin\scalac -d target\classes src\main\scala\AbstractReporter.scala src\main\scala\Reporter.scala src\main\scala\UniqueMessagePositions.scala

%JAVA_HOME%\bin\javac.exe -Xlint:unchecked -cp %SCALA3_HOME%\lib\scala-asm-9.1.0-scala-1.jar;%SCALA3_HOME%\lib\scala-library-2.13.6.jar;%SCALA3_HOME%\lib\scala3-compiler_3-3.0.2-RC2.jar;%SCALA3_HOME%\lib\scala3-interfaces-3.0.2-RC2.jar;%SCALA3_HOME%\lib\scala3-library_3-3.0.2-RC2.jar;target\classes -d target\classes src\test\java\DelegatingReporter.java

Output :

src\test\java\DelegatingReporter.java:3: warning: [unchecked] UniqueMessagePositions$$positions() in AbstractReporter implements UniqueMessagePositions$$positions() in UniqueMessagePositions
final public class DelegatingReporter extends AbstractReporter {
             ^
  return type requires unchecked conversion from HashMap to HashMap<Tuple2<String,Object>,Object>
1 warning

@odersky
Copy link
Contributor

odersky commented Apr 5, 2022

Is this still an issue after we changed the mixin scheme?

@smarter
Copy link
Member Author

smarter commented Apr 5, 2022

Yes, this issue is still present.

@smarter
Copy link
Member Author

smarter commented Apr 5, 2022

It's possible we could fix this just by emitting the getter/setter in the subclasses with ACC_SYNTHETIC (in my initial comment I mentioned emitting methods as bridges but I was confused: the only flag javac cares about is ACC_SYNTHETIC which we can emit on any method). However, using ACC_SYNTHETIC when implementing trait methods can lead to other issues as tracked in #13104

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

4 participants