From 46d86a34e75e13f904531cad9c64df1bc92dc715 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 28 May 2024 16:13:23 +0200 Subject: [PATCH] Improve backward binary compatibility: Don't generate trait bridges for abstract type overrides. When creating static forwarders try to use overriden non-bridge method, or fallback to first overriden symbol. Add tests for reproducers based on Scala 2 stdlib --- .../tools/backend/jvm/BCodeHelpers.scala | 8 +- .../dotty/tools/dotc/transform/Bridges.scala | 9 +- tests/run/i15402b.scala.scala | 9 ++ tests/run/mixin-signatures.check | 3 +- tests/run/trait-bridge-signatures.check | 74 +++++++++++ tests/run/trait-bridge-signatures.scala | 122 ++++++++++++++++++ 6 files changed, 220 insertions(+), 5 deletions(-) create mode 100644 tests/run/i15402b.scala.scala create mode 100644 tests/run/trait-bridge-signatures.check create mode 100644 tests/run/trait-bridge-signatures.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index f8866f40d9d4..2c793d176a81 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/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)) diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 4c1b2bb98c98..8f1877b0cd20 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -54,8 +54,11 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { override lazy val parents: Array[Symbol] = root.info.parents.map(_.classSymbol).toArray - override protected def matches(sym1: Symbol, sym2: Symbol): Boolean = - sym1.signature.consistentParams(sym2.signature) && super.matches(sym1, sym2) + override protected def matches(sym: Symbol, overriden: Symbol): Boolean = + def resultType(sym: Symbol) = sym.info.finalResultType.typeSymbol + def consistentParams = sym.signature.consistentParams(overriden.signature) + def overridesAbstractResultType = resultType(overriden).isAbstractOrParamType + consistentParams && !overridesAbstractResultType && super.matches(sym, overriden) override def exclude(sym: Symbol) = !sym.isPublic || super.exclude(sym) @@ -102,7 +105,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. diff --git a/tests/run/i15402b.scala.scala b/tests/run/i15402b.scala.scala new file mode 100644 index 000000000000..3493d40d2987 --- /dev/null +++ b/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) + diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check index 34adaf49d461..98979ab8d99b 100644 --- a/tests/run/mixin-signatures.check +++ b/tests/run/mixin-signatures.check @@ -49,9 +49,10 @@ 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 default java.lang.Object Foo1.f(java.lang.Object) public abstract java.lang.Object Base.g(java.lang.Object) generic: public abstract R Base.g(T) public abstract java.lang.String Foo1.g(java.lang.Object) diff --git a/tests/run/trait-bridge-signatures.check b/tests/run/trait-bridge-signatures.check new file mode 100644 index 000000000000..b6e071b62e8d --- /dev/null +++ b/tests/run/trait-bridge-signatures.check @@ -0,0 +1,74 @@ +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() +} + +interface issue15402$Foo2 { + public default issue15402$Foo issue15402$Foo.me() + public default issue15402$Named issue15402$Foo.me() +} + +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) +} + +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$IterableOps.tail() + - generic: public default C collections$IterableOps.tail() +} + +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$IterableOps.tail() + - generic: public default C collections$IterableOps.tail() +} + +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 collections$Seq collections$Seq.from(collections$IterableOnce) + public static collections$SeqOps collections$Seq.from(collections$IterableOnce) + public static java.lang.Object collections$Seq.from(collections$IterableOnce) + 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) + - generic: public static Func1 EmptyCollection.andThen(Func1) + public static PartialFunc EmptyCollection.andThen(Func1) + - generic: public static PartialFunc EmptyCollection.andThen(Func1) +} + diff --git a/tests/run/trait-bridge-signatures.scala b/tests/run/trait-bridge-signatures.scala new file mode 100644 index 000000000000..679b508d5ced --- /dev/null +++ b/tests/run/trait-bridge-signatures.scala @@ -0,0 +1,122 @@ +// scalajs: --skip + +import scala.language.unsafeNulls +import java.lang.reflect.Method + +object issue15402 { + trait Named: + def me: Named + + trait Foo extends Named: + def me: Foo = this + def foo(x: Named): Named + + trait Foo2 extends Foo +} + +trait Adapter[T] extends Function1[T, Unit] +object issue15402b { + def makeAdapter[T <: Number]: Adapter[T] = (arg: Number) => () + val adapterInteger = makeAdapter[Integer] +} + +// Regression test based on Scala 2.13 stdlib +object collections { + trait IterableOnce[A] + trait Iterable[+A] extends IterableOps[A, Iterable, Iterable[A]] + trait IterableFactory[+CC[_]] { + def from[A](source: IterableOnce[A]): CC[A] + } + + trait IterableOps[+A, +CC[_], +C] { + def head: A = null.asInstanceOf[A] + def tail: C = null.asInstanceOf[C] + } + trait Seq[+A] extends Iterable[A] with SeqOps[A, Seq, Seq[A]] + object Seq extends SeqFactory.Delegate[Seq] { + override def from[E](it: IterableOnce[E]): Seq[E] = ??? + } + trait SeqOps[+A, +CC[_], +C] extends IterableOps[A, CC, C] + trait SeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] extends IterableFactory[CC] + object SeqFactory { + class Delegate[CC[A] <: SeqOps[A, Seq, Seq[A]]] extends SeqFactory[CC] { + def from[E](it: IterableOnce[E]): CC[E] = ??? + } + } + trait LinearSeq[+A] + extends Seq[A] + with LinearSeqOps[A, LinearSeq, LinearSeq[A]] + trait LinearSeqOps[ + +A, + +CC[X] <: LinearSeq[X], + +C <: LinearSeq[A] with LinearSeqOps[A, CC, C] + ] extends SeqOps[A, CC, C] { + override def head: A + override def tail: C + } +} + + +// Regression test based on Scala 2.13 stdlib +// andThen for Nil was generated with Function1 instead of Function1 + override def andThen[C](k: Func1[B, C]): PartialFunc[A, C] = ??? +} +trait Collection[+A] extends PartialFunc[Int, A] { + override def apply(v: Int): A = ??? +} +case object EmptyCollection extends Collection[Nothing] + + +object Test { + def flagsString(m: java.lang.reflect.Method) = { + val str = List( + Option.when(m.isBridge)(""), + Option.when(m.isSynthetic)("") + ).flatten.mkString(" ") + + if (str.isEmpty()) "" else " " + str + } + + def show(clazz: Class[_], filter: Method => Boolean = _ => true): Unit = { + print(clazz.toString + " {") + clazz.getMethods + .filter(filter) + .sortBy(x => (x.getName, x.isBridge, x.toString)) + .foreach { m => + print("\n " + m + flagsString(m)) + if (m.toString() != m.toGenericString) { + print("\n - generic: " + m.toGenericString) + } + } + println("\n}") + println("") + } + + def main(args: Array[String]): Unit = { + List( + classOf[issue15402.Named], + classOf[issue15402.Foo], + classOf[issue15402.Foo2], + ).foreach(show(_, _.getName() == "me")) + + List( + classOf[Adapter[?]], + issue15402b.adapterInteger.getClass() + ).foreach(show(_, _.getName() == "apply")) + + List( + classOf[collections.LinearSeqOps[?, ?, ?]], + classOf[collections.LinearSeq[?]], + classOf[collections.Iterable[?]], + classOf[collections.IterableOps[?, ?, ?]], + Class.forName("collections$Seq") + ).foreach(show(_, m => List("head", "tail", "from").contains(m.getName()))) + + show(Class.forName("EmptyCollection"), _.getName() == "andThen") + } +}