scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Generate bridges for inherited methods with narrowed types

Open WojciechMazur opened this issue 3 years ago • 3 comments

Fixes #15402 Replaces the initial PR #15461 with a more sublime solution.

For the equivalent of given code written in Java

trait Named:
  def me: Named

trait Foo extends Named:
  def me: Foo = this
  def foo(x: Named): Named

javac compiler would generate a bridge method in trait Foo: def me: Named = Foo.this.me which allows to safely invoke it in the LambdaMetaFactory. In Scala, we only generated the def me: Foo method, leading to a runtime exception due to the not implemented method when invoked statically. A complete example and longer discussion can be found in the linked issues and PR.

The change was addressed only in the JVM backend, as the problem with SAMs does not apply to Scala.js runtime and since I couldn't think about a good phase suitable for adding the bridge methods in interfaces. This issue however also exists in Scala Native and should probably be addressed in its compiler plugin if this change would remain the backend.

WojciechMazur avatar Jun 29 '22 16:06 WojciechMazur

inspect the contents of Bar, you'll see that it only defines Named foo(Named), and not any bridge for self() either.

Yes, that's correct, the bridge for self() is only generated in Foo, and currently, we do the same in this change as it only applies to the interfaces/traits.

// trait Foo extends Named{ def me: Foo = this}
public interface Foo extends Named {
  public static Foo me$(Foo); // Scala specific method
  public default Named me();
  public default Foo me(); // Entry generated by the change, present in Java
  public abstract Named foo(Named);
}
// trait Foo2 extends Foo { override def me: Foo2 = this }
public interface Foo2 extends Foo {
  public static Foo2 me$(Foo2); // Scala specific
  public default Named me();  // Entry generated by the change, present in Java
  public default Foo me();    // Entry generated by the change, present in Java
  public default Foo2 me();
}

// class Bar extends Foo {def foo(x: Named): Named = x}
public class Bar implements Foo {
  public Bar();
  public Foo me(); // not present in Java
  public Named foo(Named);
  public Named me(); // **Not** introduced by this change - not present in Java
}

I would try to move that change to Erasure as you proposed.

WojciechMazur avatar Jun 30 '22 10:06 WojciechMazur

@lrytz Could you also have a look at this, please?

Do you think Scala 2 should have been doing the same thing, in light of what Java does?

What is your assessment in terms of binary compatibility? AFAICT, this only adds missing bridges, so it should be backward binary compatible, and should not affect separate compilation, but maybe you'll see something I don't.

sjrd avatar Jul 20 '22 09:07 sjrd

This is certainly a complex topic. It's very hard to predict how this will affect

  • binary compatibility
  • Java interop
  • bridge generation of subclasses
  • backwards and forwards compatibility with other Scala compiler versions

I suspect the reason we don't generate bridges in traits is because that was not possible before Java 8, and then it was simply never considered. Generally it seems a good idea, but it's a big change.

With this PR, will the compiler always generate bridges in traits? Or only for non-abstract overrides in traits? Will subclasses still get (now unnecessary) bridge methods?

I believe that the issue you encountered ("creation of bridge inside trait to a method with erased generic method might lead to incorrect interface method lookup and infinite loops") should be fully understood.

Maybe a good starting point would be to compile the standard library before and after this change and see what's changed.

lrytz avatar Jul 27 '22 12:07 lrytz

I'd like to resurrect this PR. I've started with comparing the signatures of Scala 2 stdlib when rebased onto current main. There we can mostly observe only additions to the generated types which shall be backward compatible, however there are 3 changed signatures - I'm not sure why any of them accoured.
Full diff available here: https://gist.github.com/WojciechMazur/8288adbd436d09fc8550dc204a2073e8

--- text/outputs-nightly/scala/collection/immutable/Vector0.class.javap 2024-01-07 22:05:20
+++ text/outputs-pr/scala/collection/immutable/Vector0.class.javap      2024-01-07 22:05:00
@@ -11,7 +11,7 @@
   public static scala.collection.mutable.StringBuilder addString(scala.collection.mutable.StringBuilder, java.lang.String);
   public static scala.collection.mutable.StringBuilder addString(scala.collection.mutable.StringBuilder, java.lang.String, java.lang.String, java.lang.String);
   public static <B> B aggregate(scala.Function0<B>, scala.Function2<B, scala.runtime.Nothing$, B>, scala.Function2<B, B, B>);
-  public static <A> scala.Function1<java.lang.Object, A> andThen(scala.Function1<scala.runtime.Nothing$, A>);
+  public static scala.Function1 andThen(scala.Function1);
   public static <C> scala.PartialFunction<java.lang.Object, C> andThen(scala.Function1<scala.runtime.Nothing$, C>);
   public static <C> scala.PartialFunction<java.lang.Object, C> andThen(scala.PartialFunction<scala.runtime.Nothing$, C>);
   public static java.lang.Object appended(java.lang.Object);
--- text/outputs-nightly/scala/collection/LinearSeqOps.class.javap      2024-01-07 22:03:28
+++ text/outputs-pr/scala/collection/LinearSeqOps.class.javap   2024-01-07 22:03:09
@@ -3,7 +3,6 @@
   public abstract boolean scala$collection$LinearSeqOps$$super$sameElements(scala.collection.IterableOnce);
   public abstract boolean isEmpty();
   public abstract A head();
-  public abstract C tail();
   public static scala.Option headOption$(scala.collection.LinearSeqOps);
   public default scala.Option<A> headOption();
   public static scala.collection.Iterator iterator$(scala.collection.LinearSeqOps);
@@ -44,6 +43,8 @@
   public default scala.Option<A> findLast(scala.Function1<A, java.lang.Object>);
   public static scala.collection.Iterator tails$(scala.collection.LinearSeqOps);
   public default scala.collection.Iterator<C> tails();
+  public static scala.collection.LinearSeq tail$(scala.collection.LinearSeqOps);
+  public default scala.collection.LinearSeq tail();
   private static int loop$1(int, int, scala.collection.LinearSeq);
   private static boolean linearSeqEq$1(scala.collection.LinearSeq, scala.collection.LinearSeq);
   private static scala.collection.LinearSeq tails$$anonfun$1(scala.collection.LinearSeq);
--- text/outputs-nightly/scala/collection/immutable/Vector0.class.javap 2024-01-07 22:05:20
+++ text/outputs-pr/scala/collection/immutable/Vector0.class.javap      2024-01-07 22:05:00
@@ -11,7 +11,7 @@
   public static scala.collection.mutable.StringBuilder addString(scala.collection.mutable.StringBuilder, java.lang.String);
   public static scala.collection.mutable.StringBuilder addString(scala.collection.mutable.StringBuilder, java.lang.String, java.lang.String, java.lang.String);
   public static <B> B aggregate(scala.Function0<B>, scala.Function2<B, scala.runtime.Nothing$, B>, scala.Function2<B, B, B>);
-  public static <A> scala.Function1<java.lang.Object, A> andThen(scala.Function1<scala.runtime.Nothing$, A>);
+  public static scala.Function1 andThen(scala.Function1);
   public static <C> scala.PartialFunction<java.lang.Object, C> andThen(scala.Function1<scala.runtime.Nothing$, C>);
   public static <C> scala.PartialFunction<java.lang.Object, C> andThen(scala.PartialFunction<scala.runtime.Nothing$, C>);
   public static java.lang.Object appended(java.lang.Object);

WojciechMazur avatar Jan 07 '24 22:01 WojciechMazur