diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index d8a38e263970..cf254a3f39b8 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -506,7 +506,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, synthetic: Boolean, skip: Set[(String, String)]): Option[(String, String)] = { val moduleName = internalName(module) val methodInfo = module.thisType.memberInfo(m) val paramJavaTypes: List[BType] = methodInfo.firstParamTypes map toTypeKind @@ -516,10 +516,10 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * as the JVM will not allow redefinition of a final static method, * 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 (synthetic) asm.Opcodes.ACC_SYNTHETIC else 0 ) // TODO needed? for(ann <- m.annotations) { ann.symbol.initialize } @@ -530,39 +530,42 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { val jReturnType = toTypeKind(methodInfo.resultType) val mdesc = MethodBType(paramJavaTypes, jReturnType).descriptor val mirrorMethodName = m.javaSimpleName - val mirrorMethod: asm.MethodVisitor = jclass.visitMethod( - flags, - mirrorMethodName, - mdesc, - jgensig, - mkArrayS(thrownExceptions) - ) - - emitAnnotations(mirrorMethod, others) - val params: List[Symbol] = Nil // backend uses this to emit annotations on parameter lists of forwarders - // to static methods of companion class - // Old assumption: in Dotty this link does not exists: there is no way to get from method type - // to inner symbols of DefDef - // TODO: now we have paramSymss and could use it here. - emitParamAnnotations(mirrorMethod, params.map(_.annotations)) - - mirrorMethod.visitCode() - - mirrorMethod.visitFieldInsn(asm.Opcodes.GETSTATIC, moduleName, str.MODULE_INSTANCE_FIELD, symDescriptor(module)) - - var index = 0 - for(jparamType <- paramJavaTypes) { - mirrorMethod.visitVarInsn(jparamType.typedOpcode(asm.Opcodes.ILOAD), index) - assert(!jparamType.isInstanceOf[MethodBType], jparamType) - index += jparamType.size - } - - mirrorMethod.visitMethodInsn(asm.Opcodes.INVOKEVIRTUAL, moduleName, mirrorMethodName, asmMethodType(m).descriptor, false) - mirrorMethod.visitInsn(jReturnType.typedOpcode(asm.Opcodes.IRETURN)) + val nameDesc = (mirrorMethodName, mdesc) + if skip(nameDesc) then None + else + val mirrorMethod: asm.MethodVisitor = jclass.visitMethod( + flags, + mirrorMethodName, + mdesc, + jgensig, + mkArrayS(thrownExceptions) + ) + + emitAnnotations(mirrorMethod, others) + val params: List[Symbol] = Nil // backend uses this to emit annotations on parameter lists of forwarders + // to static methods of companion class + // Old assumption: in Dotty this link does not exists: there is no way to get from method type + // to inner symbols of DefDef + // TODO: now we have paramSymss and could use it here. + emitParamAnnotations(mirrorMethod, params.map(_.annotations)) + + mirrorMethod.visitCode() + + mirrorMethod.visitFieldInsn(asm.Opcodes.GETSTATIC, moduleName, str.MODULE_INSTANCE_FIELD, symDescriptor(module)) + + var index = 0 + for(jparamType <- paramJavaTypes) { + mirrorMethod.visitVarInsn(jparamType.typedOpcode(asm.Opcodes.ILOAD), index) + assert(!jparamType.isInstanceOf[MethodBType], jparamType) + index += jparamType.size + } - mirrorMethod.visitMaxs(0, 0) // just to follow protocol, dummy arguments - mirrorMethod.visitEnd() + mirrorMethod.visitMethodInsn(asm.Opcodes.INVOKEVIRTUAL, moduleName, mirrorMethodName, asmMethodType(m).descriptor, false) + mirrorMethod.visitInsn(jReturnType.typedOpcode(asm.Opcodes.IRETURN)) + mirrorMethod.visitMaxs(0, 0) // just to follow protocol, dummy arguments + mirrorMethod.visitEnd() + Some(nameDesc) } /* Add forwarders for all methods defined in `module` that don't conflict @@ -574,43 +577,54 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { */ def addForwarders(jclass: asm.ClassVisitor, jclassName: String, moduleClass: Symbol): Unit = { assert(moduleClass.is(ModuleClass), moduleClass) - report.debuglog(s"Dumping mirror class for object: $moduleClass") val linkedClass = moduleClass.companionClass - lazy val conflictingNames: Set[Name] = { + lazy val conflictingNames: Set[Name] = (linkedClass.info.allMembers.collect { case d if d.name.isTermName => d.name }).toSet - } - 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 - 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)) - report.debuglog(s"No forwarder for '$m' from $jclassName to '$moduleClass'") - else if (conflictingNames(m.name)) - report.log(s"No forwarder for $m due to conflict with ${linkedClass.info.member(m.name)}") - else if (m.accessBoundary(defn.RootClass) ne defn.RootClass) - 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) - } - } + + // There's a bit of a dance to fix lampepfl/dotty#12753 in a binary compatible way (see lampepfl/dotty#12767). + // The binary-incompatible solution would be to look up members at erasure phase, but we want to continue + // emitting the excess forwarders and mark them `synthetic`. + // Because additional members are generated after erasure (mixin) it's difficult to compare lists of symbols + // at different phases and know which ones to mark synthetic. + // So we first get the forwarders we want (at erasure) and remember the emitted (name, signature) pairs. Then we + // get the forwarders as they were done in 3.0.0 and emit only new ones, as synthetic. + // In the member search at erasure phase we keep `deferred` members. This is needed for + // trait T { val x: Int = 1 }; object O extends T + // the accessor `T.x` is abstract, only mixin adds a concrete one to `O`. + + def exclude(m: Symbol, keepDeferred: Boolean) = + m == NoSymbol || + m.isType || m.is(Deferred) && !keepDeferred || (m.owner eq defn.ObjectClass) || m.isConstructor || m.name.is(ExpandedName) || + conflictingNames(m.name) || + (m.accessBoundary(defn.RootClass) ne defn.RootClass) + + val members = + sortedMembersBasedOnFlags(moduleClass.info, required = Method, excluded = ExcludedForwarder, erasurePhase) + .filterNot(m => exclude(m, keepDeferred = true)) + + val addedSignatures = members.flatMap(m => addForwarder(jclass, moduleClass, m, synthetic = false, skip = Set.empty)) + + val binCompatMembers = + sortedMembersBasedOnFlags(moduleClass.info, required = Method, excluded = ExcludedForwarder, ctx.phase) + .map(m => if (m.is(Bridge)) m.nextOverriddenSymbol else m) + .filterNot(m => exclude(m, keepDeferred = false)) + + binCompatMembers.foreach(m => addForwarder(jclass, moduleClass, m, synthetic = true, skip = addedSignatures.toSet)) } /** The members of this type that have all of `required` flags but none of `excluded` flags set. * The members are sorted by name and signature to guarantee a stable ordering. */ - private def sortedMembersBasedOnFlags(tp: Type, required: Flag, excluded: FlagSet): List[Symbol] = { + private def sortedMembersBasedOnFlags(tp: Type, required: Flag, excluded: FlagSet, phase: Phase): List[Symbol] = { // The output of `memberNames` is a Set, sort it to guarantee a stable ordering. val names = tp.memberNames(takeAllFilter).toSeq.sorted - val buffer = mutable.ListBuffer[Symbol]() + val members = mutable.ListBuffer[Symbol]() names.foreach { name => - buffer ++= tp.memberBasedOnFlags(name, required, excluded) + members ++= atPhase(phase)(tp.memberBasedOnFlags(name, required, excluded)) .alternatives.sortBy(_.signature)(Signature.lexicographicOrdering).map(_.symbol) } - buffer.toList + members.toList } /* 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); + } +}