Revert "Emit mixin forwarders as bridges"

sjrd · sjrd · commit 2d8765b44344 · 2024-11-05T15:06:29.000+01:00
This reverts commit 6d0f9ca. Forward port of most commits in scala/scala#8037 Compensate some of the consequences by adding the `MixedIn` flag. In addition to the use seen in the diff in `BCodeHelpers`, the JS backend has an existing test for `isOneOf(Bridge | MixedIn)` which needs this compensation.
diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
@@ -491,7 +491,7 @@ 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.isOneOf(Bridge | MixedIn)) m0.nextOverriddenSymbol else m0
         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))
@@ -507,10 +507,7 @@ trait BCodeHelpers extends BCodeIdiomatic {
           // we generate ACC_SYNTHETIC forwarders so Java compilers ignore them.
           val isSynthetic =
             m0.name.is(NameKinds.SyntheticSetterName) ||
-            // Only hide bridges generated at Erasure, mixin forwarders are also
-            // marked as bridge but shouldn't be hidden since they don't have a
-            // non-bridge overload.
-            m0.is(Bridge) && m0.initial.validFor.firstPhaseId == erasurePhase.next.id
+            m0.is(Bridge)
           addForwarder(jclass, moduleClass, m, isSynthetic)
         }
       }
diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala
@@ -306,7 +306,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
       for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth))
       yield {
         util.Stats.record("mixin forwarders")
-        transformFollowing(DefDef(mkForwarderSym(meth.asTerm, Bridge), forwarderRhsFn(meth)))
+        transformFollowing(DefDef(mkForwarderSym(meth.asTerm, extraFlags = MixedIn), forwarderRhsFn(meth)))
       }
 
     cpy.Template(impl)(
diff --git a/compiler/test/dotc/run-test-pickling.blacklist b/compiler/test/dotc/run-test-pickling.blacklist
@@ -12,13 +12,11 @@ i9473.scala
 i13433.scala
 i13433b.scala
 macros-in-same-project1
-mixin-forwarder-overload
 t10889
 t3452d
 t3452e
 t3452g
 t7374
-t8905
 tuple-drop.scala
 tuple-ops.scala
 tuple-ops.scala
@@ -48,4 +46,3 @@ named-tuples-strawman-2.scala
 
 # typecheckErrors method unpickling
 typeCheckErrors.scala
-
diff --git a/tests/run/mixin-bridge-methods.scala b/tests/run/mixin-bridge-methods.scala
@@ -0,0 +1,16 @@
+// scalajs: --skip
+
+trait Foo {
+  def getFoo() = "foo"
+}
+
+class Sub extends Foo {
+  def getBar() = "bar"
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val ms = classOf[Sub].getDeclaredMethods
+    assert(ms forall (x => !x.isBridge), ms mkString " ")
+  }
+}
diff --git a/tests/run/mixin-forwarder-overload/A.scala b/tests/run/mixin-forwarder-overload/A.scala
diff --git a/tests/run/mixin-forwarder-overload/Test.java b/tests/run/mixin-forwarder-overload/Test.java
diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check
@@ -1,19 +1,19 @@
 class Test$bar1$ {
+  public java.lang.String Test$bar1$.f(java.lang.Object)
   public java.lang.Object Test$bar1$.f(java.lang.Object) <bridge> <synthetic>
-  public java.lang.String Test$bar1$.f(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar1$.g(java.lang.String)
   public java.lang.Object Test$bar1$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar1$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Test$bar1$.h(java.lang.Object) <bridge> <synthetic>
+  public java.lang.Object Test$bar1$.h(java.lang.Object)
 }
 
 class Test$bar2$ {
+  public java.lang.Object Test$bar2$.f(java.lang.String)
   public java.lang.Object Test$bar2$.f(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Test$bar2$.f(java.lang.String) <bridge> <synthetic>
   public java.lang.String Test$bar2$.g(java.lang.String)
   public java.lang.Object Test$bar2$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar2$.g(java.lang.String) <bridge> <synthetic>
-  public java.lang.Object Test$bar2$.h(java.lang.Object) <bridge> <synthetic>
+  public java.lang.Object Test$bar2$.h(java.lang.Object)
 }
 
 class Test$bar3$ {
@@ -23,7 +23,7 @@ class Test$bar3$ {
   public java.lang.String Test$bar3$.g(java.lang.String)
   public java.lang.Object Test$bar3$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar3$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Foo3.h(java.lang.Object) <bridge> <synthetic>
+  public java.lang.Object Foo3.h(java.lang.Object)
 }
 
 class Test$bar4$ {
@@ -33,7 +33,7 @@ class Test$bar4$ {
   public java.lang.String Test$bar4$.g(java.lang.String)
   public java.lang.Object Test$bar4$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar4$.g(java.lang.String) <bridge> <synthetic>
-  public java.lang.Object Foo4.h(java.lang.Object) <bridge> <synthetic>
+  public java.lang.Object Foo4.h(java.lang.Object)
 }
 
 class Test$bar5$ {
@@ -45,7 +45,7 @@ class Test$bar5$ {
   public java.lang.Object Test$bar5$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar5$.g(java.lang.String) <bridge> <synthetic>
   public java.lang.String Test$bar5$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Test$bar5$.h(java.lang.Object) <bridge> <synthetic>
+  public java.lang.Object Test$bar5$.h(java.lang.Object)
 }
 
 interface Foo1 {
diff --git a/tests/run/t3452b-bcode/J_2.java b/tests/run/t3452b-bcode/J_2.java
@@ -0,0 +1,6 @@
+public class J_2 {
+  public static void j() {
+    StringSearch.search("test");
+    StringSearch.searchC("test");
+  }
+}
diff --git a/tests/run/t3452b-bcode/S_1.scala b/tests/run/t3452b-bcode/S_1.scala
@@ -0,0 +1,17 @@
+trait Search[M] {
+  def search(input: M): C[Int] = {
+    println("Search received: " + input)
+    null
+  }
+}
+
+class SearchC[M] {
+  def searchC(input: M): C[Int] = {
+    println("SearchC received: " + input)
+    null
+  }
+}
+
+object StringSearch extends SearchC[String] with Search[String]
+
+trait C[T]
diff --git a/tests/run/t3452b-bcode/S_3.scala b/tests/run/t3452b-bcode/S_3.scala
@@ -0,0 +1,7 @@
+// scalajs: --skip
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    J_2.j()
+  }
+}
diff --git a/tests/run/t3452d/A.scala b/tests/run/t3452d/A.scala
@@ -2,6 +2,6 @@ trait TraversableLike[A, Repr] {
   def tail: Repr = null.asInstanceOf[Repr]
 }
 
-abstract class AbstractTrav[A] extends TraversableLike[A, Iterable[A]]
+abstract class AbstractTrav[A] extends TraversableLike[A, Traversable[A]]
 
 class C[A] extends AbstractTrav[A]
diff --git a/tests/run/t3452d/Test.java b/tests/run/t3452d/Test.java
@@ -3,6 +3,8 @@
 public class Test {
 	public static void main(String[] args) {
 		C<String> c = new C<String>();
-		scala.collection.Iterable<String> ls = c.tail();
+		// TODO add a bridge during mixin so we can expose
+		// sharper generic signature for `tail`.
+		/*Traversable<String>*/ Object ls = c.tail();
 	}
 }
diff --git a/tests/run/t3452g/A.scala b/tests/run/t3452g/A.scala
@@ -4,8 +4,6 @@ trait TraversableLike[A, Repr] {
 
 abstract class AbstractTrav[A] extends TraversableLike[A, AbstractTrav[A]]
 
-class C1 extends AbstractTrav[String]
-
 object O extends AbstractTrav[String]
 
-class C2[A] extends AbstractTrav[A]
+class C[A] extends AbstractTrav[A]
diff --git a/tests/run/t3452g/Test.java b/tests/run/t3452g/Test.java
@@ -1,12 +1,15 @@
 // scalajs: --skip
 
 public class Test {
-  public static void main(String[] args) {
-    AbstractTrav<String> lsSharp1 = new C1().tail();
+	public static void main(String[] args) {
+		// To get better types here, we would need to
+		// add bridge during mixin so we can expose
+		// a generic return type of Traversable<A>, because the erasure
+		// of this (Traversable) differs from the erasure of the mixed
+		// method (erasure(Repr) = Object)
 
-    // Object is the result type for the static forwarder (might be because of #11305)
-    Object lsSharp2 = O.tail();
+		Object lsSharp = O.tail();
 
-    AbstractTrav<String> lsSharp3 = new C2<String>().tail();
-  }
+		Object lsSharp2 = new C<String>().tail();
+	}
 }
diff --git a/tests/run/t3452h.scala b/tests/run/t3452h.scala
@@ -1,8 +1,17 @@
-class Mix___eFoo_I_wBar__f extends Foo_I_ with Bar__f { f; }
+class Mix extends Foo with Bar { f; }
 trait T
-abstract class Foo_I_ { class I extends T    ; def f: I         ; f; }
-trait Bar__f          { type  I>:Null<:T;      def f: I = {null}; f; def gobble: I = {null}}
+abstract class Foo {
+  class I extends T
+  def f: I
+  f
+}
+trait Bar {
+  type I >: Null <: T
+  def f: I = null
+  f
+  def gobble: I = null
+}
 
 object Test extends App {
-  new Mix___eFoo_I_wBar__f
+  new Mix
 }
diff --git a/tests/run/t8905/DoubleRDD.scala b/tests/run/t8905/DoubleRDD.scala
diff --git a/tests/run/t8905/Test.java b/tests/run/t8905/Test.java

Original file line number	Diff line number	Diff line change
`@@ -306,7 +306,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>`
`306`	`306`	`for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth))`
`307`	`307`	`yield {`
`308`	`308`	`util.Stats.record("mixin forwarders")`
`309`		`- transformFollowing(DefDef(mkForwarderSym(meth.asTerm, Bridge), forwarderRhsFn(meth)))`
	`309`	`+ transformFollowing(DefDef(mkForwarderSym(meth.asTerm, extraFlags = MixedIn), forwarderRhsFn(meth)))`
`310`	`310`	`}`
`311`	`311`
`312`	`312`	`cpy.Template(impl)(`