8000 [backport] Don't emit forwarder in mirror class for bridge methods · lrytz/scala@3edeaac · GitHub
[go: up one dir, main page]
Skip to content

Commit 3edeaac

Browse files
lrytzlrytz
committed
[backport] Don't emit forwarder in mirror class for bridge methods
In 2.12.6 and before, the Scala compiler emits static forwarders for bridge methods in top-level modules. These forwarders are emitted by mistake, the filter to exclude bridges did not work as expected. These bridge forwarders make the Java compiler on JDK 11 report ambiguity errors when using static forwarders (scala/bug#11061). PR scala#7035 fixed this for 2.12.7 by adding the `ACC_BRIDGE` flag to static forwarders for bridges. We decided to keep these bridges for binary compatibility. However, the new flag causes the eclipse Java compiler (and apparently also IntelliJ) to report ambiguity errors when using static forwarders (scala/bug#11271). In 2.13.x the Scala compiler no longer emits static forwarders for bridges (PR scala#6531). This PR brings the same behavior to 2.12.8. This change breaks binary compatibility. However, in the examples we tested, the Java compiler emits references to the non-bridge methods, so compiled code continues to work if a library is replaced by a new version that doesn't have forwarders for bridges: ``` $> cat T.scala class A[T] { def get: T = ??? } object T extends A[String] { override def get: String = "hi" } $> ~/scala/scala-2.12.7/bin/scalac T.scala ``` Generates two forwarders in `T.class` ``` // access flags 0x49 public static bridge get()Ljava/lang/Object; // access flags 0x9 public static get()Ljava/lang/String; ``` ``` $> javac -version javac 1.8.0_181 $> cat Test.java public class Test { public static void main(String[] args) { System.out.println(T.get()); } } $> javac Test.java ``` Generates in Test.class ``` INVOKESTATIC T.get ()Ljava/lang/String; ```
1 parent 25c7215 commit 3edeaac

File tree

3 files changed

+20
-38
lines changed

3 files changed

+20
-38
lines changed

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic {
806806
*/
807807
private def addForwarder(
808808
isRemoteClass: Boolean,
809-
isBridge: Boolean,
810809
jclass: asm.ClassVisitor,
811810
moduleClass: Symbol,
812811
m: Symbol): Unit = {
@@ -834,7 +833,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic {
834833
*/
835834
// TODO: evaluate the other flags we might be dropping on the floor here.
836835
val flags = GenBCode.PublicStatic |
837-
(if (isBridge) asm.Opcodes.ACC_BRIDGE else 0) |
838836
(if (m.isVarargsMethod) asm.Opcodes.ACC_VARARGS else 0) |
839837
(if (m.isDeprecated) asm.Opcodes.ACC_DEPRECATED else 0)
840838

@@ -887,32 +885,23 @@ abstract class BCodeHelpers extends BCodeIdiomatic {
887885
*/
888886
def addForwarders(isRemoteClass: Boolean, jclass: asm.ClassVisitor, jclassName: String, moduleClass: Symbol) {
889887
assert(moduleClass.isModuleClass, moduleClass)
890-
debuglog(s"Dumping mirror class for object: $moduleClass")
891888

892-
val linkedClass = moduleClass.companionClass
889+
val linkedClass = moduleClass.companionClass
893890
lazy val conflictingNames: Set[Name] = {
894891
(linkedClass.info.members collect { case sym if sym.name.isTermName => sym.name }).toSet
895892
}
896-
debuglog(s"Potentially conflicting names for forwarders: $conflictingNames")
897-
898-
for (m <- moduleClass.info.membersBasedOnFlags(BCodeHelpers.ExcludedForwarderFlags, symtab.Flags.METHOD)) {
899-
// Fix for scala/bug#11207, see https://github.com/scala/scala/pull/7035/files#r226274350. This makes sure that 2.12.8 generates
900-
// the same forwarder methods as in 2.12.6 (but includes bridge flags). In 2.13 we don't generate any forwarders for bridges.
901-
val bridgeImplementingAbstract = m.isBridge && m.nextOverriddenSymbol.isDeferred
902-
if (m.isType || m.isDeferred || bridgeImplementingAbstract || (m.owner eq definitions.ObjectClass) || m.isConstructor)
903-
debuglog(s"No forwarder for '$m' from $jclassName to '$moduleClass': ${m.isType} || ${m.isDeferred} || ${m.owner eq definitions.ObjectClass} || ${m.isConstructor}")
904-
else if (conflictingNames(m.name))
905-
log(s"No forwarder for $m due to conflict with ${linkedClass.info.member(m.name)}")
906-
else if (m.hasAccessBoundary)
907-
log(s"No forwarder for non-public member $m")
908-
else {
909-
log(s"Adding static forwarder for '$m' from $jclassName to '$moduleClass'")
910-
addForwarder(isRemoteClass,
911-
isBridge = m.isBridge,
912-
jclass,
913-
moduleClass,
914-
m)
915-
}
893+
894+
// Before erasure * to exclude bridge methods. Excluding them by flag doesn't work, because then
895+
// the method from the base class that the bridge overrides is included (scala/bug#10812).
896+
// * Using `exitingUncurry` (not `enteringErasure`) because erasure enters bridges in traversal,
897+
// not in the InfoTransform, so it actually modifies the type from the previous phase.
898+
// Uncurry adds java varargs, which need to be included in the mirror class.
899+
val members = exitingUncurry(moduleClass.info.membersBasedOnFlags(BCodeHelpers.ExcludedForwarderFlags, symtab.Flags.METHOD))
900+
for (m <- members) {
901+
val excl = m.isDeferred || m.isConstructor || m.hasAccessBoundary ||
902+
{ val o = m.owner; (o eq ObjectClass) || (o eq AnyRefClass) || (o eq AnyClass) } ||
903+
conflictingNames(m.name)
904+
if (!excl) addForwarder(isRemoteClass, jclass, moduleClass, m)
916905
}
917906
}
918907

@@ -1184,14 +1173,9 @@ abstract class BCodeHelpers extends BCodeIdiomatic {
11841173
}
11851174

11861175
object BCodeHelpers {
1187-
val ExcludedForwarderFlags = {
1176+
val ExcludedForwarderFlags: Long = {
11881177
import scala.tools.nsc.symtab.Flags._
1189-
// Should include DEFERRED but this breaks findMember.
1190-
// Note that BRIDGE is *not* excluded. Trying to exclude bridges by flag doesn't work, findMembers
1191-
// will then include the member from the parent (which the bridge overrides / implements).
1192-
// This caused scala/bug#11061 and scala/bug#10812. In 2.13, they are fixed by not emitting
1193-
// forwarders for bridges. But in 2.12 that's not binary compatible, so instead we continue to
1194-
// emit forwarders for bridges, but mark them with ACC_BRIDGE.
1178+
// Don't include DEFERRED but filter afterwards, see comment on `findMembers`
11951179
SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | PRIVATE | MACRO
11961180
}
11971181

‎src/library/scala/runtime/SymbolLiteral.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static CallSite bootstrap(MethodHandles.Lookup lookup, String invokedName
2222
MethodType invokedType,
2323
String value) throws Throwable {
2424
ClassLoader classLoader = lookup.lookupClass().getClassLoader();
25-
MethodType type = MethodType.fromMethodDescriptorString("(Ljava/lang/Object;)Ljava/lang/Object;", classLoader);
25+
MethodType type = MethodType.fromMethodDescriptorString("(Ljava/lang/String;)Lscala/Symbol;", classLoader);
2626
Class<?> symbolClass = Class.forName("scala.Symbol", false, classLoader);
2727
MethodHandle factoryMethod = lookup.findStatic(symbolClass, "apply", type);
2828
Object symbolValue = factoryMethod.invokeWithArguments(value);

‎test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ class BytecodeTest extends BytecodeTesting {
2323
| def g: Object
2424
|}
2525
|object B extends A {
26-
| override def f: String = "b" // "bridge" forwarder
27-
| def g: String = "b" // no "bridge" forwarder, as the overridden method is abstract, scala/bug#11207
26+
| override def f: String = "b"
27+
| def g: String = "b"
2828
|}
2929
|case class K(x: Int, s: String)
3030
""".stripMargin
3131
for (base <- List("trait", "abstract class")) {
3232
val List(a, bMirror, bModule, kClass, kModule) = compileClasses(base + code)
3333
assertEquals("B", bMirror.name)
34-
assertEquals(List("f()Ljava/lang/Object;0x49", "f()Ljava/lang/String;0x9", "g()Ljava/lang/String;0x9"),
34+
assertEquals(List("f()Ljava/lang/String;0x9", "g()Ljava/lang/String;0x9"),
3535
bMirror.methods.asScala
3636
.filter(m => m.name == "f" || m.name == "g")
3737
.map(m => m.name + m.desc + "0x" + Integer.toHexString(m.access)).toList.sorted)
@@ -42,7 +42,7 @@ class BytecodeTest extends BytecodeTesting {
4242
}
4343

4444
@Test
45-
def varArg(): Unit = {
45+
def staticForwardersVarargFlag(): Unit = {
4646
val code =
4747
""" A { @annotation.varargs def f(i: Int*): Object = null }
4848
|object B extends A { @annotation.varargs override def f(i: Int*): String = "b" }
@@ -51,9 +51,7 @@ class BytecodeTest extends BytecodeTesting {
5151
val List(a, bMirror, bModule) = compileClasses(base + code)
5252
assertEquals("B", bMirror.name)
5353
assertEquals(List(
54-
"f(Lscala/collection/Seq;)Ljava/lang/Object;0x49",
5554
"f(Lscala/collection/Seq;)Ljava/lang/String;0x9",
56-
"f([I)Ljava/lang/Object;0xc9",
5755
"f([I)Ljava/lang/String;0x89"),
5856
bMirror.methods.asScala
5957
.filter(_.name == "f")

0 commit comments

Comments
 (0)
0