From 9c01e1b1e5ddb9382ddeae3b5b8ee1fd7bac7e35 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Wed, 29 Jun 2022 18:36:22 +0200 Subject: [PATCH 1/8] Gen bridge for inherited methods with narrowed types. --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 43 +++++++++++++++++++ tests/run/i15402.scala | 18 ++++++++ 2 files changed, 61 insertions(+) create mode 100644 tests/run/i15402.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 394700c2898e..42ce9979d869 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -688,6 +688,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { genTraitConstructorDefDef(dd) else genStaticForwarderForDefDef(dd) + genInterfaceMethodBridgeForDefDef(dd) genDefDef(dd) else genDefDef(dd) @@ -768,6 +769,48 @@ trait BCodeSkelBuilder extends BCodeHelpers { ).transform(dd.rhs) }) + /** Creates a bridges for interfece inherited and narrowed methods, + * to workaround JVM limitations when using LambdaMetaFactory (see issue #15402) + * Example: + * ``` + * trait Foo: + * def self: Foo + * + * trait Bar extends Foo: + * def self: Bar = this + * def SAM(x: Any): Any + * + * def usage(x: List[Foo]) = x.map(_.self) + * ``` + * We need to define a bridge in trait Bar `def self: Foo = Bar.this.self` + * in order to prevent JVM runtime exceptions. Otherwise at the time of + * accessing `_.self` in the SAM instance created based on the `trait Bar` + * it will complain about missing implementation of `def self: Foo`. + * Javac compiler does exactly the same. + */ + private def genInterfaceMethodBridgeForDefDef(dd: DefDef): Unit = + val sym = dd.symbol + sym.owner.directlyInheritedTraits + .flatMap { parent => + val inheritedSym = parent.info.decl(sym.name) + Option.when( + inheritedSym.exists && + sym.signature != inheritedSym.signature && + sym.info <:< inheritedSym.info + )(inheritedSym.symbol.asTerm) + }.distinctBy(_.signature) + .foreach(genInterfaceMethodBridge(sym.asTerm, _)) + + private def genInterfaceMethodBridge(sym: TermSymbol, inheritedSym: TermSymbol): Unit = + assert(sym.name == inheritedSym.name, "Not an override") + val owner = sym.owner.asClass + val bridgeSym = inheritedSym.copy(owner = owner, flags = sym.flags).asTerm + val bridge = tpd.DefDef(bridgeSym, {paramss => + val params = paramss.head + tpd.Apply(tpd.This(owner).select(sym), params) + }) + genDefDef(bridge) + private def genStaticForwarderForDefDef(dd: DefDef): Unit = val forwarderDef = makeStaticForwarder(dd) genDefDef(forwarderDef) diff --git a/tests/run/i15402.scala b/tests/run/i15402.scala new file mode 100644 index 000000000000..31686e9bd3df --- /dev/null +++ b/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 From c7cea463a3c73553a71f0c3ee33eaa6537f0ce5f Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 1 Jul 2022 12:43:50 +0200 Subject: [PATCH 2/8] Move generatation of trait bridges to Erasure --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 43 ------------------- .../dotty/tools/dotc/transform/Bridges.scala | 15 +++++-- .../dotty/tools/dotc/transform/Erasure.scala | 2 +- tests/run/mixin-signatures.check | 6 +-- 4 files changed, 14 insertions(+), 52 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 42ce9979d869..394700c2898e 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -688,7 +688,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { genTraitConstructorDefDef(dd) else genStaticForwarderForDefDef(dd) - genInterfaceMethodBridgeForDefDef(dd) genDefDef(dd) else genDefDef(dd) @@ -769,48 +768,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { ).transform(dd.rhs) }) - /** Creates a bridges for interfece inherited and narrowed methods, - * to workaround JVM limitations when using LambdaMetaFactory (see issue #15402) - * Example: - * ``` - * trait Foo: - * def self: Foo - * - * trait Bar extends Foo: - * def self: Bar = this - * def SAM(x: Any): Any - * - * def usage(x: List[Foo]) = x.map(_.self) - * ``` - * We need to define a bridge in trait Bar `def self: Foo = Bar.this.self` - * in order to prevent JVM runtime exceptions. Otherwise at the time of - * accessing `_.self` in the SAM instance created based on the `trait Bar` - * it will complain about missing implementation of `def self: Foo`. - * Javac compiler does exactly the same. - */ - private def genInterfaceMethodBridgeForDefDef(dd: DefDef): Unit = - val sym = dd.symbol - sym.owner.directlyInheritedTraits - .flatMap { parent => - val inheritedSym = parent.info.decl(sym.name) - Option.when( - inheritedSym.exists && - sym.signature != inheritedSym.signature && - sym.info <:< inheritedSym.info - )(inheritedSym.symbol.asTerm) - }.distinctBy(_.signature) - .foreach(genInterfaceMethodBridge(sym.asTerm, _)) - - private def genInterfaceMethodBridge(sym: TermSymbol, inheritedSym: TermSymbol): Unit = - assert(sym.name == inheritedSym.name, "Not an override") - val owner = sym.owner.asClass - val bridgeSym = inheritedSym.copy(owner = owner, flags = sym.flags).asTerm - val bridge = tpd.DefDef(bridgeSym, {paramss => - val params = paramss.head - tpd.Apply(tpd.This(owner).select(sym), params) - }) - genDefDef(bridge) - private def genStaticForwarderForDefDef(dd: DefDef): Unit = val forwarderDef = makeStaticForwarder(dd) genDefDef(forwarderDef) diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 482e5056fad0..9f0c32f89a45 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -31,10 +31,12 @@ 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. The only exception is generation of bridges for traits, + * where we want to be able to deduplicate bridges already defined in parents. */ - override def parents = Array(root.superClass) + override lazy val parents = + if(root.is(Trait)) super.parents + else Array(root.superClass) override def exclude(sym: Symbol) = !sym.isOneOf(MethodOrModule) || sym.isAllOf(Module | JavaDefined) || super.exclude(sym) @@ -172,9 +174,14 @@ 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] = + // When adding bridges to traits ignore non-public methods + // see `DottyBackendTests.invocationReceivers` val opc = inContext(preErasureCtx) { new BridgesCursor } while opc.hasNext do - if !opc.overriding.is(Deferred) then + if + !opc.overriding.is(Deferred) && + (!root.is(Trait) || opc.overridden.isPublic) + then addBridgeIfNeeded(opc.overriding, opc.overridden) opc.next() if bridges.isEmpty then stats diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index be00d952566c..34139b4ac891 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/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) } diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check index 98979ab8d99b..30e41c49f623 100644 --- a/tests/run/mixin-signatures.check +++ b/tests/run/mixin-signatures.check @@ -49,10 +49,9 @@ 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) @@ -62,10 +61,9 @@ interface Foo1 { } 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 default java.lang.Object Foo2.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.Object Foo2.g(java.lang.String) From 07671f1794e546a8176431f3bc75935e135fe791 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sun, 3 Jul 2022 00:04:24 +0200 Subject: [PATCH 3/8] Restrict possible targets of bridge generation in traits --- .../dotty/tools/dotc/transform/Bridges.scala | 38 +++++++++++++------ tests/run/i15402b/ImplJava_2.java | 25 ++++++++++++ tests/run/i15402b/ImplScala_2.scala | 7 ++++ tests/run/i15402b/NamedJava_1.java | 3 ++ tests/run/i15402b/NamedScala_1.scala | 2 + tests/run/i15402b/Usage_3.scala | 18 +++++++++ tests/run/mixin-signatures.check | 3 +- 7 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 tests/run/i15402b/ImplJava_2.java create mode 100644 tests/run/i15402b/ImplScala_2.scala create mode 100644 tests/run/i15402b/NamedJava_1.java create mode 100644 tests/run/i15402b/NamedScala_1.scala create mode 100644 tests/run/i15402b/Usage_3.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 9f0c32f89a45..4c1b2bb98c98 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -31,12 +31,9 @@ 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. The only exception is generation of bridges for traits, - * where we want to be able to deduplicate bridges already defined in parents. + * are also counted. */ - override lazy val parents = - if(root.is(Trait)) super.parents - else Array(root.superClass) + override def parents = Array(root.superClass) override def exclude(sym: Symbol) = !sym.isOneOf(MethodOrModule) || sym.isAllOf(Module | JavaDefined) || super.exclude(sym) @@ -45,6 +42,25 @@ 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{ + // 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 protected def matches(sym1: Symbol, sym2: Symbol): Boolean = + sym1.signature.consistentParams(sym2.signature) && super.matches(sym1, sym2) + + override def exclude(sym: Symbol) = + !sym.isPublic || super.exclude(sym) + } + val site = root.thisType private var toBeRemoved = immutable.Set[Symbol]() @@ -174,14 +190,12 @@ 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] = - // When adding bridges to traits ignore non-public methods - // see `DottyBackendTests.invocationReceivers` - val opc = inContext(preErasureCtx) { new BridgesCursor } + val opc = inContext(preErasureCtx) { + if (root.is(Trait)) new TraitBridgesCursor + else new BridgesCursor + } while opc.hasNext do - if - !opc.overriding.is(Deferred) && - (!root.is(Trait) || opc.overridden.isPublic) - then + if !opc.overriding.is(Deferred) then addBridgeIfNeeded(opc.overriding, opc.overridden) opc.next() if bridges.isEmpty then stats diff --git a/tests/run/i15402b/ImplJava_2.java b/tests/run/i15402b/ImplJava_2.java new file mode 100644 index 000000000000..d4ac03240db5 --- /dev/null +++ b/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; + } +} diff --git a/tests/run/i15402b/ImplScala_2.scala b/tests/run/i15402b/ImplScala_2.scala new file mode 100644 index 000000000000..fc6cf49f53a2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/run/i15402b/NamedJava_1.java b/tests/run/i15402b/NamedJava_1.java new file mode 100644 index 000000000000..297812a05e91 --- /dev/null +++ b/tests/run/i15402b/NamedJava_1.java @@ -0,0 +1,3 @@ +interface NamedJava { + NamedJava self(); +} diff --git a/tests/run/i15402b/NamedScala_1.scala b/tests/run/i15402b/NamedScala_1.scala new file mode 100644 index 000000000000..f5e4944035bc --- /dev/null +++ b/tests/run/i15402b/NamedScala_1.scala @@ -0,0 +1,2 @@ +trait NamedScala: + def self: NamedScala diff --git a/tests/run/i15402b/Usage_3.scala b/tests/run/i15402b/Usage_3.scala new file mode 100644 index 000000000000..d254dcc8a3ad --- /dev/null +++ b/tests/run/i15402b/Usage_3.scala @@ -0,0 +1,18 @@ +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 \ No newline at end of file diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check index 30e41c49f623..34adaf49d461 100644 --- a/tests/run/mixin-signatures.check +++ b/tests/run/mixin-signatures.check @@ -61,9 +61,10 @@ interface Foo1 { } 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 default java.lang.Object Foo2.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.Object Foo2.g(java.lang.String) From 3dffc79205dfceec1ed2984abfbccceb4a0a9c4f Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 28 May 2024 16:13:23 +0200 Subject: [PATCH 4/8] 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") + } +} From fe2a1a17d1599c24f308c6511b33a044d9a4ffdc Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 31 May 2024 01:07:42 +0200 Subject: [PATCH 5/8] Exclude run/i15402b from scalajs tests --- tests/run/i15402b/Usage_3.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run/i15402b/Usage_3.scala b/tests/run/i15402b/Usage_3.scala index d254dcc8a3ad..8709a5f80cec 100644 --- a/tests/run/i15402b/Usage_3.scala +++ b/tests/run/i15402b/Usage_3.scala @@ -1,3 +1,4 @@ +// scalajs: --skip class Names(xs: List[NamedScala | NamedJava]): def mkString = xs.map{ case n: NamedScala => n.self From 2d69f6d08ff5c5f2962975e2222d93933e51dd43 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 31 May 2024 01:08:58 +0200 Subject: [PATCH 6/8] Allow to skip priotirization of decls in the TraitBridgesCursor - fixes bridges generated in play-framework --- .../dotty/tools/dotc/transform/Bridges.scala | 1 + .../dotc/transform/OverridingPairs.scala | 26 +++++++++++++------ tests/run/trait-bridge-signatures.check | 11 ++++++++ tests/run/trait-bridge-signatures.scala | 16 ++++++++++++ 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 8f1877b0cd20..4cdb96e83c0c 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -50,6 +50,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { * 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 diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 4020291dded0..38c8d5014732 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/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 } diff --git a/tests/run/trait-bridge-signatures.check b/tests/run/trait-bridge-signatures.check index b6e071b62e8d..26de5b586b90 100644 --- a/tests/run/trait-bridge-signatures.check +++ b/tests/run/trait-bridge-signatures.check @@ -72,3 +72,14 @@ class EmptyCollection { - generic: public static PartialFunc EmptyCollection.andThen(Func1) } +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) + public default play$StandaloneWSRequest play$StandaloneWSRequest.addCookies(scala.collection.immutable.Seq) + - generic: public default play$StandaloneWSRequest play$StandaloneWSRequest.addCookies(scala.collection.immutable.Seq) + public abstract play$StandaloneWSRequest play$StandaloneWSRequest.withCookies(scala.collection.immutable.Seq) + - generic: public abstract play$StandaloneWSRequest play$StandaloneWSRequest.withCookies(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) +} + diff --git a/tests/run/trait-bridge-signatures.scala b/tests/run/trait-bridge-signatures.scala index 679b508d5ced..410803df2e22 100644 --- a/tests/run/trait-bridge-signatures.scala +++ b/tests/run/trait-bridge-signatures.scala @@ -71,6 +71,20 @@ trait Collection[+A] extends PartialFunc[Int, A] { } case object EmptyCollection extends Collection[Nothing] +object play { + trait WSCookie + trait StandaloneWSRequest { + type Self <: StandaloneWSRequest { type Self = StandaloneWSRequest.this.Self } + def withCookies(cookies: WSCookie*): Self + def addCookies(cookies: WSCookie*): Self = ??? + } + + trait WSRequest extends StandaloneWSRequest { + override type Self = WSRequest + override def addCookies(cookies: WSCookie*): Self + override def withCookies(cookie: WSCookie*): Self + } +} object Test { def flagsString(m: java.lang.reflect.Method) = { @@ -118,5 +132,7 @@ object Test { ).foreach(show(_, m => List("head", "tail", "from").contains(m.getName()))) show(Class.forName("EmptyCollection"), _.getName() == "andThen") + + show(classOf[play.WSRequest]) } } From e52535f8f2557478bffb32c5c8d39d0da6ff4c21 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 31 May 2024 14:35:46 +0200 Subject: [PATCH 7/8] Always generate bridges in traits - makes the trait-bridge-signatures.play reproducer compliant with Java counterpart --- .../src/dotty/tools/dotc/transform/Bridges.scala | 14 +++++--------- tests/run/mixin-signatures.check | 12 ++++-------- tests/run/trait-bridge-signatures.check | 14 ++++++-------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 4cdb96e83c0c..1e7f79a7c9df 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -51,16 +51,11 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { */ 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 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) } @@ -194,12 +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 forTrait = root.is(Trait) val opc = inContext(preErasureCtx) { - if (root.is(Trait)) new TraitBridgesCursor - else new BridgesCursor + 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 diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check index 98979ab8d99b..30593d791c0a 100644 --- a/tests/run/mixin-signatures.check +++ b/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) 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) 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) 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) public default java.lang.Object Base.h(java.lang.Object) generic: public default R Base.h(T) } diff --git a/tests/run/trait-bridge-signatures.check b/tests/run/trait-bridge-signatures.check index 26de5b586b90..345645e1ec84 100644 --- a/tests/run/trait-bridge-signatures.check +++ b/tests/run/trait-bridge-signatures.check @@ -27,8 +27,7 @@ interface collections$LinearSeqOps { - 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() + public default java.lang.Object collections$LinearSeqOps.tail() } interface collections$LinearSeq { @@ -36,8 +35,7 @@ interface collections$LinearSeq { - 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() + public default java.lang.Object collections$LinearSeqOps.tail() } interface collections$Iterable { @@ -75,11 +73,11 @@ class EmptyCollection { 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) - public default play$StandaloneWSRequest play$StandaloneWSRequest.addCookies(scala.collection.immutable.Seq) - - generic: public default play$StandaloneWSRequest play$StandaloneWSRequest.addCookies(scala.collection.immutable.Seq) - public abstract play$StandaloneWSRequest play$StandaloneWSRequest.withCookies(scala.collection.immutable.Seq) - - generic: public abstract play$StandaloneWSRequest play$StandaloneWSRequest.withCookies(scala.collection.immutable.Seq) + public default play$StandaloneWSRequest play$WSRequest.addCookies(scala.collection.immutable.Seq) + 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) + public default play$StandaloneWSRequest play$WSRequest.withCookies(scala.collection.immutable.Seq) + public static play$StandaloneWSRequest play$WSRequest.withCookies$(play$WSRequest,scala.collection.immutable.Seq) } From 9d7a585dae6f4cfed0642888affded09a558cd85 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 31 May 2024 16:33:52 +0200 Subject: [PATCH 8/8] Fix tests failing in the CI, make trait-bridge-signatures test stable with different JVM versions --- .../dotc/transform/SpecializeFunctionsTests.scala | 1 + tests/run/trait-bridge-signatures.scala | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/transform/SpecializeFunctionsTests.scala b/compiler/test/dotty/tools/dotc/transform/SpecializeFunctionsTests.scala index 2982238490e1..bbc0fa8cd3c1 100644 --- a/compiler/test/dotty/tools/dotc/transform/SpecializeFunctionsTests.scala +++ b/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, diff --git a/tests/run/trait-bridge-signatures.scala b/tests/run/trait-bridge-signatures.scala index 410803df2e22..b9da5ed5f4c2 100644 --- a/tests/run/trait-bridge-signatures.scala +++ b/tests/run/trait-bridge-signatures.scala @@ -129,10 +129,19 @@ object Test { classOf[collections.Iterable[?]], classOf[collections.IterableOps[?, ?, ?]], Class.forName("collections$Seq") - ).foreach(show(_, m => List("head", "tail", "from").contains(m.getName()))) + ).foreach: cls => + show(cls, filter = m => List("head", "tail", "from").contains(m.getName()) && { + // Divirgence between JVM version + // In JDK 8 (CI Windows runner) `java.lang.Object collections$IterableOps.head()` is shown + // but it's not shown in JDK 17 (CI Unix runner) + // Filter out problematic case to make tests more stable + if cls == classOf[collections.LinearSeq[?]] then + List(cls, classOf[collections.LinearSeqOps[?, ?, ?]]).contains(m.getDeclaringClass()) + else true + } + ) show(Class.forName("EmptyCollection"), _.getName() == "andThen") - show(classOf[play.WSRequest]) } }