Skip to content

Commit

Permalink
Improve backward binary compatibility: Don't generate trait bridges f…
Browse files Browse the repository at this point in the history
…or 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
  • Loading branch information
WojciechMazur committed May 28, 2024
1 parent 52a051f commit 46d86a3
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 5 deletions.
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
9 changes: 6 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/Bridges.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions tests/run/i15402b.scala.scala
Original file line number Diff line number Diff line change
@@ -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)

3 changes: 2 additions & 1 deletion tests/run/mixin-signatures.check
Original file line number Diff line number Diff line change
Expand Up @@ -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) <bridge> <synthetic>
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)
Expand Down
74 changes: 74 additions & 0 deletions tests/run/trait-bridge-signatures.check
Original file line number Diff line number Diff line change
@@ -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() <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$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 <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>)
}

122 changes: 122 additions & 0 deletions tests/run/trait-bridge-signatures.scala
Original file line number Diff line number Diff line change
@@ -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<Nothing, A] type
trait Func1[-T1, +R] {
def apply(v1: T1): R
def andThen[A](g: Func1[R, A]): Func1[T1, A] = ???
}
trait PartialFunc[-A, +B] extends Func1[A, B] { self =>
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)("<bridge>"),
Option.when(m.isSynthetic)("<synthetic>")
).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")
}
}

0 comments on commit 46d86a3

Please sign in to comment.