Skip to content

Commit

Permalink
Restrict possible targets of bridge generation in traits
Browse files Browse the repository at this point in the history
  • Loading branch information
WojciechMazur committed May 28, 2024
1 parent aa05a8f commit 52a051f
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 13 deletions.
38 changes: 26 additions & 12 deletions compiler/src/dotty/tools/dotc/transform/Bridges.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]()
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions tests/run/i15402b/ImplJava_2.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
7 changes: 7 additions & 0 deletions tests/run/i15402b/ImplScala_2.scala
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/run/i15402b/NamedJava_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
interface NamedJava {
NamedJava self();
}
2 changes: 2 additions & 0 deletions tests/run/i15402b/NamedScala_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
trait NamedScala:
def self: NamedScala
18 changes: 18 additions & 0 deletions tests/run/i15402b/Usage_3.scala
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion tests/run/mixin-signatures.check
Original file line number Diff line number Diff line change
Expand Up @@ -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) <bridge> <synthetic>
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)
Expand Down

0 comments on commit 52a051f

Please sign in to comment.