Generate bridges for inherited methods with narrowed types

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

@@ -491,7 +491,13 @@ trait BCodeHelpers extends BCodeIdiomatic {
       report.debuglog(s"Potentially conflicting names for forwarders: $conflictingNames")
 
       for (m0 <- sortedMembersBasedOnFlags(moduleClass.info, required = Method, excluded = ExcludedForwarder)) {
-        val m = if (m0.is(Bridge)) m0.nextOverriddenSymbol else m0
+        val m =
+          if !m0.is(Bridge) then m0
+          else
+            m0.allOverriddenSymbols.find(!_.is(Bridge))
+            .filterNot(_.is(Deferred))
+            .getOrElse(m0.nextOverriddenSymbol)
+
         if (m == NoSymbol)
           report.log(s"$m0 is a bridge method that overrides nothing, something went wrong in a previous phase.")
         else if (m.isType || m.is(Deferred) || (m.owner eq defn.ObjectClass) || m.isConstructor || m.name.is(ExpandedName))

compiler/src/dotty/tools/dotc/transform/Bridges.scala

@@ -31,8 +31,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
 
     /** Only use the superclass of `root` as a parent class. This means
      *  overriding pairs that have a common implementation in a trait parent
-     *  are also counted. This is necessary because we generate bridge methods
-     *  only in classes, never in traits.
+     *  are also counted.
      */
     override def parents = Array(root.superClass)
 
@@ -43,6 +42,24 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
       OverridingPairs.isOverridingPair(sym1, sym2, parent.thisType)
   }
 
+  /** Usually, we don't want to create bridges for methods defined in traits, but for some cases it is necessary.
+   *  For example with SAM methods combined with covariant result types (see issue #15402).
+   *  In some cases, creating a bridge inside a trait to an erased generic method leads to incorrect
+   *  interface method lookup and infinite loops at run-time. (e.g., in cats' `WriterTApplicative.map`).
+   *  To avoid that issue, we limit bridges to methods with the same set of parameters and a different, covariant result type.
+   *  We also ignore non-public methods (see `DottyBackendTests.invocationReceivers` for a test case).
+   */
+  private class TraitBridgesCursor(using Context) extends BridgesCursor{
+    override protected def prioritizeDeferred: Boolean = false
+
+    // Get full list of parents to deduplicate already defined bridges in the parents
+    override lazy val parents: Array[Symbol] =
+      root.info.parents.map(_.classSymbol).toArray
+
+    override def exclude(sym: Symbol) =
+      !sym.isPublic || super.exclude(sym)
+  }
+
   val site = root.thisType
 
   private var toBeRemoved = immutable.Set[Symbol]()
@@ -84,7 +101,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
                       |clashes with definition of the member itself; both have erased type ${info(member)(using elimErasedCtx)}."""",
                   bridgePosFor(member))
     }
-    else if !inContext(preErasureCtx)(site.memberInfo(member).matches(site.memberInfo(other))) then
+    else if !(inContext(preErasureCtx)(site.memberInfo(member).matches(site.memberInfo(other))) || member.owner.isAnonymousClass) then
       // Neither symbol signatures nor pre-erasure types seen from root match; this means
       // according to Scala 2 semantics there is no override.
       // A bridge might introduce a classcast exception.
@@ -172,9 +189,13 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
    *  time deferred methods in `stats` that are replaced by a bridge with the same signature.
    */
   def add(stats: List[untpd.Tree]): List[untpd.Tree] =
-    val opc = inContext(preErasureCtx) { new BridgesCursor }
+    val forTrait = root.is(Trait)
+    val opc = inContext(preErasureCtx) {
+       if forTrait then TraitBridgesCursor()
+       else BridgesCursor()
+    }
     while opc.hasNext do
-      if !opc.overriding.is(Deferred) then
+      if forTrait || !opc.overriding.is(Deferred) then
         addBridgeIfNeeded(opc.overriding, opc.overridden)
       opc.next()
     if bridges.isEmpty then stats

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -1104,5 +1104,5 @@ object Erasure {
   }
 
   private def takesBridges(sym: Symbol)(using Context): Boolean =
-    sym.isClass && !sym.isOneOf(Flags.Trait | Flags.Package)
+    sym.isClass && !sym.is(Flags.Package)
 }

compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala

@@ -49,25 +49,35 @@ object OverridingPairs:
     protected def matches(sym1: Symbol, sym2: Symbol): Boolean =
       sym1.isType || sym1.asSeenFrom(self).matches(sym2.asSeenFrom(self))
 
+    /** Should deffered declarations took precedence over the concreate defintions
+     *  In same cases, eg. trait bridges, cursor might lead to incorrect results
+     *  when overriden and overriding methods would be swapped leading to incorrect behaviour
+     */
+    protected def prioritizeDeferred = true
+
     /** The symbols that can take part in an overriding pair */
-    private val decls = {
+    val decls = {
       val decls = newScope
       // fill `decls` with overriding shadowing overridden */
-      def fillDecls(bcs: List[Symbol], deferred: Boolean): Unit = bcs match {
+      def fillDecls(bcs: List[Symbol], include: Symbol => Boolean): Unit = bcs match {
         case bc :: bcs1 =>
-          fillDecls(bcs1, deferred)
+          fillDecls(bcs1, include)
           var e = bc.info.decls.lastEntry
           while (e != null) {
-            if (e.sym.is(Deferred) == deferred && !exclude(e.sym))
+            if (include(e.sym) && !exclude(e.sym))
               decls.enter(e.sym)
             e = e.prev
           }
         case nil =>
       }
-      // first, deferred (this will need to change if we change lookup rules!
-      fillDecls(base.info.baseClasses, deferred = true)
-      // then, concrete.
-      fillDecls(base.info.baseClasses, deferred = false)
+      val baseClasses = base.info.baseClasses
+      if prioritizeDeferred then
+        // first, deferred (this will need to change if we change lookup rules!
+        fillDecls(baseClasses, _.is(Deferred))
+        // then, concrete.
+        fillDecls(baseClasses, !_.is(Deferred))
+      else
+        fillDecls(baseClasses, _ => true)
       decls
     }
 

compiler/test/dotty/tools/dotc/transform/SpecializeFunctionsTests.scala

@@ -147,6 +147,7 @@ class SpecializeFunctionsTests extends DottyBytecodeTest {
         }
         .map(_.name)
         .toList
+        .distinct // ignore bridge methods
 
       assert(
         apps.length == 1,

tests/run/i15402.scala

@@ -0,0 +1,18 @@
+trait Named:
+  def me: Named
+
+trait Foo extends Named:
+  def me: Foo = this
+  def foo(x: Named): Named
+
+trait Foo2 extends Foo
+
+class Names(xs: List[Named]):
+  def mkString = xs.map(_.me).mkString(",")
+
+object Names:
+  def single[T <: Named](t: T): Names = Names(List(t))
+
+@main def Test() =
+  Names.single[Foo](identity).mkString
+  Names.single[Foo2](identity).mkString

tests/run/i15402b.scala.scala

@@ -0,0 +1,9 @@
+trait Adapter[T] extends Function1[T, Unit]
+
+// Works in Scala 2.12 and 2.13 but generates wrong bytecode for Scala 3
+// due to using `(arg: Number) => ()` instead of `(arg: T) => ()`
+def makeAdapter[T <: Number]: Adapter[T] = (arg: Number) => ()
+
+// In Scala 3 this caused a java.lang.AbstractMethodError
+@main def Test = makeAdapter[Integer](123)
+

tests/run/i15402b/ImplJava_2.java

@@ -0,0 +1,25 @@
+interface FooJavaFromScala extends NamedScala {
+  default FooJavaFromScala self() {
+    return this;
+  }
+  NamedScala foo(NamedScala x);
+}
+
+interface FooJavaFromJava extends NamedJava {
+  default FooJavaFromJava self() {
+    return this;
+  }
+  NamedJava foo(NamedJava x);
+}
+
+class BarJavaFromJava implements FooJavaFromJava {
+  public NamedJava foo(NamedJava x) {
+    return x;
+  }
+}
+
+class BarJavaFromScala implements FooJavaFromScala {
+  public NamedScala foo(NamedScala x) {
+    return x;
+  }
+}

tests/run/i15402b/ImplScala_2.scala

@@ -0,0 +1,7 @@
+trait FooScalaFromScala extends NamedScala:
+  def self: FooScalaFromScala = this
+  def foo(x: NamedScala): NamedScala
+
+trait FooScalaFromJava extends NamedJava:
+  def self: FooScalaFromJava = this
+  def foo(x: NamedJava): NamedJava

tests/run/i15402b/NamedJava_1.java

@@ -0,0 +1,3 @@
+interface NamedJava {
+    NamedJava self();
+}

tests/run/i15402b/NamedScala_1.scala

@@ -0,0 +1,2 @@
+trait NamedScala:
+  def self: NamedScala

tests/run/i15402b/Usage_3.scala

@@ -0,0 +1,19 @@
+// scalajs: --skip
+class Names(xs: List[NamedScala | NamedJava]):
+  def mkString = xs.map{
+    case n: NamedScala => n.self
+    case n: NamedJava => n.self
+  }.mkString(",")
+
+object Names:
+  def single[T <: NamedScala](t: T): Names = Names(List(t))
+  def single[T <: NamedJava](t: T): Names = Names(List(t))
+
+
+@main def Test() =
+  Names.single[FooJavaFromJava](identity).mkString
+  Names.single[FooJavaFromScala](identity).mkString
+  Names(List(new BarJavaFromJava())).mkString
+  Names(List(new BarJavaFromScala())).mkString
+  Names.single[FooScalaFromJava](identity).mkString // failing in #15402
+  Names.single[FooScalaFromScala](identity).mkString // failing in #15402

tests/run/mixin-signatures.check

@@ -49,27 +49,23 @@ class Test$bar5$ {
 }
 
 interface Foo1 {
-  public abstract java.lang.Object Base.f(java.lang.Object)
-    generic: public abstract R Base.f(T)
   public default java.lang.String Foo1.f(java.lang.Object)
     generic: public default java.lang.String Foo1.f(T)
-  public abstract java.lang.Object Base.g(java.lang.Object)
-    generic: public abstract R Base.g(T)
+  public default java.lang.Object Foo1.f(java.lang.Object) <bridge> <synthetic>
   public abstract java.lang.String Foo1.g(java.lang.Object)
     generic: public abstract java.lang.String Foo1.g(T)
+  public default java.lang.Object Foo1.g(java.lang.Object) <bridge> <synthetic>
   public default java.lang.Object Base.h(java.lang.Object)
     generic: public default R Base.h(T)
 }
 
 interface Foo2 {
-  public abstract java.lang.Object Base.f(java.lang.Object)
-    generic: public abstract R Base.f(T)
   public default java.lang.Object Foo2.f(java.lang.String)
     generic: public default R Foo2.f(java.lang.String)
-  public abstract java.lang.Object Base.g(java.lang.Object)
-    generic: public abstract R Base.g(T)
+  public default java.lang.Object Foo2.f(java.lang.Object) <bridge> <synthetic>
   public abstract java.lang.Object Foo2.g(java.lang.String)
     generic: public abstract R Foo2.g(java.lang.String)
+  public default java.lang.Object Foo2.g(java.lang.Object) <bridge> <synthetic>
   public default java.lang.Object Base.h(java.lang.Object)
     generic: public default R Base.h(T)
 }

tests/run/trait-bridge-signatures.check

@@ -0,0 +1,83 @@
+interface issue15402$Named {
+  public abstract issue15402$Named issue15402$Named.me()
+}
+
+interface issue15402$Foo {
+  public default issue15402$Foo issue15402$Foo.me()
+  public default issue15402$Named issue15402$Foo.me() <bridge> <synthetic>
+}
+
+interface issue15402$Foo2 {
+  public default issue15402$Foo issue15402$Foo.me()
+  public default issue15402$Named issue15402$Foo.me() <bridge> <synthetic>
+}
+
+interface Adapter {
+  public abstract java.lang.Object scala.Function1.apply(java.lang.Object)
+  - generic: public abstract R scala.Function1.apply(T1)
+}
+
+class issue15402b$$anon$1 {
+  public final void issue15402b$$anon$1.apply(java.lang.Number)
+  public java.lang.Object issue15402b$$anon$1.apply(java.lang.Object) <bridge> <synthetic>
+}
+
+interface collections$LinearSeqOps {
+  public abstract java.lang.Object collections$LinearSeqOps.head()
+  - generic: public abstract A collections$LinearSeqOps.head()
+  public abstract collections$LinearSeq collections$LinearSeqOps.tail()
+  - generic: public abstract C collections$LinearSeqOps.tail()
+  public default java.lang.Object collections$LinearSeqOps.tail() <bridge> <synthetic>
+}
+
+interface collections$LinearSeq {
+  public abstract java.lang.Object collections$LinearSeqOps.head()
+  - generic: public abstract A collections$LinearSeqOps.head()
+  public abstract collections$LinearSeq collections$LinearSeqOps.tail()
+  - generic: public abstract C collections$LinearSeqOps.tail()
+  public default java.lang.Object collections$LinearSeqOps.tail() <bridge> <synthetic>
+}
+
+interface collections$Iterable {
+  public default java.lang.Object collections$IterableOps.head()
+  - generic: public default A collections$IterableOps.head()
+  public default java.lang.Object collections$IterableOps.tail()
+  - generic: public default C collections$IterableOps.tail()
+}
+
+interface collections$IterableOps {
+  public default java.lang.Object collections$IterableOps.head()
+  - generic: public default A collections$IterableOps.head()
+  public default java.lang.Object collections$IterableOps.tail()
+  - generic: public default C collections$IterableOps.tail()
+}
+
+interface collections$Seq {
+  public static collections$Seq collections$Seq.from(collections$IterableOnce)
+  - generic: public static <E> collections$Seq<E> collections$Seq.from(collections$IterableOnce<E>)
+  public static collections$SeqOps collections$Seq.from(collections$IterableOnce) <synthetic>
+  public static java.lang.Object collections$Seq.from(collections$IterableOnce) <synthetic>
+  public default java.lang.Object collections$IterableOps.head()
+  - generic: public default A collections$IterableOps.head()
+  public default java.lang.Object collections$IterableOps.tail()
+  - generic: public default C collections$IterableOps.tail()
+}
+
+class EmptyCollection {
+  public static Func1 EmptyCollection.andThen(Func1) <synthetic>
+  - generic: public static <A> Func1<java.lang.Object, A> EmptyCollection.andThen(Func1<scala.runtime.Nothing$, A>)
+  public static PartialFunc EmptyCollection.andThen(Func1)
+  - generic: public static <C> PartialFunc<java.lang.Object, C> EmptyCollection.andThen(Func1<scala.runtime.Nothing$, C>)
+}
+
+interface play$WSRequest {
+  public abstract play$WSRequest play$WSRequest.addCookies(scala.collection.immutable.Seq)
+  - generic: public abstract play$WSRequest play$WSRequest.addCookies(scala.collection.immutable.Seq<play$WSCookie>)
+  public default play$StandaloneWSRequest play$WSRequest.addCookies(scala.collection.immutable.Seq) <bridge> <synthetic>
+  public static play$StandaloneWSRequest play$WSRequest.addCookies$(play$WSRequest,scala.collection.immutable.Seq)
+  public abstract play$WSRequest play$WSRequest.withCookies(scala.collection.immutable.Seq)
+  - generic: public abstract play$WSRequest play$WSRequest.withCookies(scala.collection.immutable.Seq<play$WSCookie>)
+  public default play$StandaloneWSRequest play$WSRequest.withCookies(scala.collection.immutable.Seq) <bridge> <synthetic>
+  public static play$StandaloneWSRequest play$WSRequest.withCookies$(play$WSRequest,scala.collection.immutable.Seq)
+}
+