Skip to content

Commit

Permalink
Hide problematic static forwarders
Browse files Browse the repository at this point in the history
Static forwarders for bridges lead to ambiguous errors in some Java
compilers, and the trait setters aren't meant to be called by users.
Since we can't remove them without breaking binary-compatibility, we
mark them ACC_SYNTHETIC so that Java compilers will ignore them. See
also the discussion in scala#12767 which implements an alternate fix.

Fixes scala#12753.

Co-Authored-By: Lukas Rytz <[email protected]>
  • Loading branch information
smarter and lrytz committed Jun 17, 2021
1 parent b1d5760 commit 0ed163e
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 3 deletions.
17 changes: 14 additions & 3 deletions compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.core.NameKinds.ExpandedName
import dotty.tools.dotc.core.Signature
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.NameKinds
import dotty.tools.dotc.core.Symbols._
import dotty.tools.dotc.core.Types
import dotty.tools.dotc.core.Types._
Expand Down Expand Up @@ -507,7 +508,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
*
* must-single-thread
*/
private def addForwarder(jclass: asm.ClassVisitor, module: Symbol, m: Symbol): Unit = {
private def addForwarder(jclass: asm.ClassVisitor, module: Symbol, m: Symbol, isSynthetic: Boolean): Unit = {
val moduleName = internalName(module)
val methodInfo = module.thisType.memberInfo(m)
val paramJavaTypes: List[BType] = methodInfo.firstParamTypes map toTypeKind
Expand All @@ -518,9 +519,10 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
* and we don't know what classes might be subclassing the companion class. See SI-4827.
*/
// TODO: evaluate the other flags we might be dropping on the floor here.
// TODO: ACC_SYNTHETIC ?
val flags = GenBCodeOps.PublicStatic | (
if (m.is(JavaVarargs)) asm.Opcodes.ACC_VARARGS else 0
) | (
if (isSynthetic) asm.Opcodes.ACC_SYNTHETIC else 0
)

// TODO needed? for(ann <- m.annotations) { ann.symbol.initialize }
Expand Down Expand Up @@ -595,7 +597,16 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
report.log(s"No forwarder for non-public member $m")
else {
report.log(s"Adding static forwarder for '$m' from $jclassName to '$moduleClass'")
addForwarder(jclass, moduleClass, m)
// It would be simpler to not generate forwarders for these methods,
// but that wouldn't be binary-compatible with Scala 3.0.0, so instead
// 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
addForwarder(jclass, moduleClass, m, isSynthetic)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ typeclass-derivation3.scala
varargs-abstract
zero-arity-case-class.scala
i12194.scala
i12753
28 changes: 28 additions & 0 deletions tests/run/i12753.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
1
Dbr
1
1
2
1
1
1
1
2
1
1
synthetic public static C D.foo(int)
public static D D.foo(int)
public static D D.t()
synthetic public static java.lang.Object D.bar()
public static java.lang.String D.bar()
public static int O.a()
public static int O.b()
public static int O.c()
public static int O.d()
public static int O.i()
public static int O.j()
public static int O.k()
public static int O.l()
synthetic public static void O.T$_setter_$a_$eq(int)
public static void O.b_$eq(int)
public static void O.j_$eq(int)
34 changes: 34 additions & 0 deletions tests/run/i12753/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
trait C[This <: C[This]]

trait COps[This <: C[This]] {
def t: This
def foo(x: Int): This = t
def bar: Object = "Cbr"
}

class D extends C[D] {
def x = 1
}
object D extends COps[D] {
def t = new D
override def foo(x: Int): D = super.foo(x)
override def bar: String = "Dbr"
}

trait T {
val a = 1
var b = 1
lazy val c = 1
def d = 1

val i: Int
var j: Int
lazy val k: Int = 1
def l: Int
}
object O extends T {
val i: Int = 1
var j: Int = 1
override lazy val k: Int = 1
def l: Int = 1
}
36 changes: 36 additions & 0 deletions tests/run/i12753/Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
public class Test {
public static void s(Object s) {
System.out.println(s);
}

public static void statics(Class<?> c) {
java.lang.reflect.Method[] ms = c.getDeclaredMethods();
java.util.Arrays.sort(ms, (a, b) -> a.toString().compareTo(b.toString()));
for (java.lang.reflect.Method a : ms) {
if (java.lang.reflect.Modifier.isStatic(a.getModifiers()))
s((a.isSynthetic() ? "synthetic " : "") + a);
}
}

public static void main(String[] args) {
s(D.foo(1).x());
s(D.bar().trim());

s(O.a());
s(O.b());
O.b_$eq(2);
s(O.b());
s(O.c());
s(O.d());

s(O.i());
s(O.j());
O.j_$eq(2);
s(O.j());
s(O.k());
s(O.l());

statics(D.class);
statics(O.class);
}
}

0 comments on commit 0ed163e

Please sign in to comment.