From 0ed163e9b4b79ab9d18128a8ca2c5b74eed7a7d7 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 17 Jun 2021 15:52:20 +0200 Subject: [PATCH] Hide problematic static forwarders 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 #12767 which implements an alternate fix. Fixes #12753. Co-Authored-By: Lukas Rytz --- .../tools/backend/jvm/BCodeHelpers.scala | 17 +++++++-- .../test/dotc/run-test-pickling.blacklist | 1 + tests/run/i12753.check | 28 +++++++++++++++ tests/run/i12753/C.scala | 34 ++++++++++++++++++ tests/run/i12753/Test.java | 36 +++++++++++++++++++ 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 tests/run/i12753.check create mode 100644 tests/run/i12753/C.scala create mode 100644 tests/run/i12753/Test.java diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index 2aa75e1bd934..03d5ac13665b 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -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._ @@ -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 @@ -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 } @@ -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) } } } diff --git a/compiler/test/dotc/run-test-pickling.blacklist b/compiler/test/dotc/run-test-pickling.blacklist index 5bcca090e8c6..41821a5d4d70 100644 --- a/compiler/test/dotc/run-test-pickling.blacklist +++ b/compiler/test/dotc/run-test-pickling.blacklist @@ -34,3 +34,4 @@ typeclass-derivation3.scala varargs-abstract zero-arity-case-class.scala i12194.scala +i12753 diff --git a/tests/run/i12753.check b/tests/run/i12753.check new file mode 100644 index 000000000000..e599ced53b63 --- /dev/null +++ b/tests/run/i12753.check @@ -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) diff --git a/tests/run/i12753/C.scala b/tests/run/i12753/C.scala new file mode 100644 index 000000000000..1b62940d6ffb --- /dev/null +++ b/tests/run/i12753/C.scala @@ -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 +} diff --git a/tests/run/i12753/Test.java b/tests/run/i12753/Test.java new file mode 100644 index 000000000000..2a0bd9480b9c --- /dev/null +++ b/tests/run/i12753/Test.java @@ -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); + } +}