From 1a6ec3e1e0435b5091e6bbcc77318b5db6febbeb Mon Sep 17 00:00:00 2001 From: Solal Pirelli Date: Mon, 20 Apr 2026 14:46:58 +0200 Subject: [PATCH 1/2] Only generate generic signature if it differs from descriptor --- .../tools/backend/jvm/BCodeHelpers.scala | 69 ++++++++----------- .../tools/backend/jvm/BCodeSkelBuilder.scala | 11 +-- .../backend/jvm/DottyBytecodeTests.scala | 6 +- 3 files changed, 38 insertions(+), 48 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index a845df1eb561..d88c286799d8 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -308,11 +308,12 @@ trait BCodeHelpers(val backendUtils: BackendUtils)(using ctx: Context) extends B * * @param sym The symbol for which to generate a signature. * @param owner The owner of `sym`. + * @param descriptor The descriptor of the symbol; the signature is unnecessary if they are equal. * @return The generic signature of `sym` before erasure, as specified in the Java Virtual * Machine Specification, ยง4.3.4, or `null` if `sym` doesn't need a generic signature. * @see https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.3.4 */ - def getGenericSignature(sym: Symbol, owner: Symbol): String | Null = { + def getGenericSignature(sym: Symbol, owner: Symbol, descriptor: String | Null): String | Null = { atPhase(erasurePhase) { // Finding the member's type is nontrivial because of erasure and how it interacts with other phases. def computeMemberType(): Type = { @@ -345,7 +346,11 @@ trait BCodeHelpers(val backendUtils: BackendUtils)(using ctx: Context) extends B owner.denot.thisType.memberInfo(sym) } - getGenericSignatureHelper(sym, owner, computeMemberType()).orNull + if ctx.base.settings.XnoGenericSig.value then null + else + val genSig = getGenericSignatureHelper(sym, owner, computeMemberType()).orNull + if genSig == descriptor then null + else genSig } } @@ -375,12 +380,12 @@ trait BCodeHelpers(val backendUtils: BackendUtils)(using ctx: Context) extends B ) // TODO needed? for(ann <- m.annotations) { ann.symbol.initialize } - val jgensig = getStaticForwarderGenericSignature(m, module) + val jReturnType = ts.toTypeKind(methodInfo.resultType) + val mdesc = MethodBType(paramJavaTypes, jReturnType).descriptor + val jgensig = getStaticForwarderGenericSignature(m, module, mdesc) val (throws, others) = m.annotations.partition(_.symbol eq defn.ThrowsAnnot) val thrownExceptions: List[String] = getExceptions(throws) - val jReturnType = ts.toTypeKind(methodInfo.resultType) - val mdesc = MethodBType(paramJavaTypes, jReturnType).descriptor val mirrorMethodName = m.javaSimpleName val lengthOk = if jgensig ne null then BCodeUtils.checkConstantStringLength(jgensig) else BCodeUtils.checkConstantStringLength(mirrorMethodName, mdesc) @@ -639,22 +644,17 @@ trait BCodeHelpers(val backendUtils: BackendUtils)(using ctx: Context) extends B } // end of trait JAndroidBuilder private def getGenericSignatureHelper(sym: Symbol, owner: Symbol, memberTpe: Type)(using Context): Option[String] = { - if (needsGenericSignature(sym)) { - val erasedTypeSym = TypeErasure.fullErasure(sym.denot.info).typeSymbol - if (erasedTypeSym.isPrimitiveValueClass) { - // Suppress signatures for symbols whose types erase in the end to primitive - // value types. This is needed to fix #7416. - None - } else { - val jsOpt = GenericSignatures.javaSig(sym, memberTpe) - if (ctx.settings.XverifySignatures.value) { - jsOpt.foreach(verifySignature(sym, _)) - } - - jsOpt - } - } else { + val erasedTypeSym = TypeErasure.fullErasure(sym.denot.info).typeSymbol + if (erasedTypeSym.isPrimitiveValueClass) { + // Suppress signatures for symbols whose types erase in the end to primitive + // value types. This is needed to fix #7416. None + } else { + val jsOpt = GenericSignatures.javaSig(sym, memberTpe) + if (ctx.settings.XverifySignatures.value) { + jsOpt.foreach(verifySignature(sym, _)) + } + jsOpt } } @@ -686,32 +686,21 @@ trait BCodeHelpers(val backendUtils: BackendUtils)(using ctx: Context) extends B } } - // @M don't generate java generics sigs for (members of) implementation - // classes, as they are monomorphic (TODO: ok?) - private final def needsGenericSignature(sym: Symbol): Boolean = !( - // pp: this condition used to include sym.hasexpandedname, but this leads - // to the total loss of generic information if a private member is - // accessed from a closure: both the field and the accessor were generated - // without it. This is particularly bad because the availability of - // generic information could disappear as a consequence of a seemingly - // unrelated change. - ctx.base.settings.XnoGenericSig.value - || sym.is(Artifact) - || sym.isAllOf(LiftedMethod) - || sym.is(Bridge) - ) - - private def getStaticForwarderGenericSignature(sym: Symbol, moduleClass: Symbol): String = { + private def getStaticForwarderGenericSignature(sym: Symbol, moduleClass: Symbol, descriptor: String | Null): String | Null = { // scala/bug#3452 Static forwarder generation uses the same erased signature as the method if forwards to. // By rights, it should use the signature as-seen-from the module class, and add suitable // primitive and value-class boxing/unboxing. // But for now, just like we did in mixin, we just avoid writing a wrong generic signature // (one that doesn't erase to the actual signature). See run/t3452b for a test case. - val memberTpe = atPhase(erasurePhase) { moduleClass.denot.thisType.memberInfo(sym) } - val erasedMemberType = ElimErasedValueType.elimEVT(TypeErasure.transformInfo(sym, memberTpe)) - if (erasedMemberType =:= sym.denot.info) - getGenericSignatureHelper(sym, moduleClass, memberTpe).orNull + if !ctx.base.settings.XnoGenericSig.value then + val memberTpe = atPhase(erasurePhase) { moduleClass.denot.thisType.memberInfo(sym) } + val erasedMemberType = ElimErasedValueType.elimEVT(TypeErasure.transformInfo(sym, memberTpe)) + if (erasedMemberType =:= sym.denot.info) + val gensig = getGenericSignatureHelper(sym, moduleClass, memberTpe).orNull + if gensig == descriptor then null + else gensig + else null else null } diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index fb16b9d07253..983e8bb1dbe0 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -336,7 +336,7 @@ trait BCodeSkelBuilder(using ctx: Context) extends BCodeHelpers { val flags = BCodeUtils.javaFlags(claszSymbol) - val thisSignature = getGenericSignature(claszSymbol, claszSymbol.owner) + val thisSignature = getGenericSignature(claszSymbol, claszSymbol.owner, null) val lengthOk = if thisSignature ne null then BCodeUtils.checkConstantStringLength(thisSignature) else BCodeUtils.checkConstantStringLength(thisName) if !lengthOk then @@ -423,7 +423,8 @@ trait BCodeSkelBuilder(using ctx: Context) extends BCodeHelpers { * No code is needed for this module symbol. */ for (f <- claszSymbol.info.decls.filter(p => p.isTerm && !p.is(Method))) { - val javagensig = getGenericSignature(f, claszSymbol) + val descriptor = symInfoTK(f).descriptor + val javagensig = getGenericSignature(f, claszSymbol, descriptor) val flags = javaFieldFlags(f) assert(!f.isStaticMember || !claszSymbol.is(Trait) || !f.is(Mutable), @@ -432,7 +433,7 @@ trait BCodeSkelBuilder(using ctx: Context) extends BCodeHelpers { val jfield = new asm.tree.FieldNode( flags, f.javaSimpleName, - symInfoTK(f).descriptor, + descriptor, javagensig, null // no initial value ) @@ -736,7 +737,8 @@ trait BCodeSkelBuilder(using ctx: Context) extends BCodeHelpers { */ private def initJMethod(flags: Int, params: List[Symbol]): Unit = { - val jgensig = getGenericSignature(methSymbol, claszSymbol) + val mdesc = ts.asmMethodType(methSymbol).descriptor + val jgensig = getGenericSignature(methSymbol, claszSymbol, mdesc) val (excs, others) = methSymbol.annotations.partition(_.symbol eq defn.ThrowsAnnot) val thrownExceptions: List[String] = getExceptions(excs) @@ -744,7 +746,6 @@ trait BCodeSkelBuilder(using ctx: Context) extends BCodeHelpers { if (isMethSymStaticCtor) CLASS_CONSTRUCTOR_NAME else jMethodName - val mdesc = ts.asmMethodType(methSymbol).descriptor val lengthOk = if jgensig ne null then BCodeUtils.checkConstantStringLength(jgensig) else BCodeUtils.checkConstantStringLength(bytecodeName, mdesc) if !lengthOk then diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 22d2515cd382..82f56dcea572 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -1724,9 +1724,9 @@ class DottyBytecodeTests extends DottyBytecodeTest { checkBCode(source) { dir => val clsIn = dir.lookupName("Foo.class", directory = false).input val clsNode = loadClassNode(clsIn) - def testSig(methodName: String, expectedSignature: String) = { - val signature = clsNode.methods.asScala.filter(_.name == methodName).map(_.signature) - assertEquals(List(expectedSignature), signature) + def testSig(methodName: String, expectedDescriptor: String) = { + val descriptor = clsNode.methods.asScala.filter(_.name == methodName).map(_.desc) + assertEquals(List(expectedDescriptor), descriptor) } testSig("foo", "()I") testSig("bar", "()I") From fe4f2f14c5f8d875d1b166cfbccaeba8a41989d9 Mon Sep 17 00:00:00 2001 From: Solal Pirelli Date: Wed, 29 Apr 2026 11:04:13 +0200 Subject: [PATCH 2/2] add bytecode test --- .../backend/jvm/DottyBytecodeTests.scala | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 82f56dcea572..7c3b8c9eb768 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -2059,6 +2059,35 @@ class DottyBytecodeTests extends DottyBytecodeTest { assert(clsNode.methods.asScala.exists(_.name == "test")) } } + + @Test def presenceOfGenericSignatures = { + val source = + """|object Test: + | def no0(x: Int): Unit = () + | def no1(x: Array[Int]): Array[Int] = x + | def no2(x: String): String = x + | def no3(): Array[String] = Array.empty + | + | def yes0[A](x: A): A = x + | def yes1[A, B](x: A): B = ??? + | def yes2[A](x: Int): Unit = () + | def yes3[A](x: Array[A]): A = x(0) + | + | @scala.annotation.varargs def v(x: String*): String = x(0) + |""".stripMargin + checkBCode(source) { dir => + val clsIn = dir.lookupName("Test$.class", directory = false).input + val clsNode = loadClassNode(clsIn) + for noMeth <- clsNode.methods.asScala if noMeth.name.startsWith("no") do + assert(noMeth.signature == null, s"${noMeth.name} should not have a signature but does: ${noMeth.signature}") + for yesMeth <- clsNode.methods.asScala if yesMeth.name.startsWith("yes") do + assert(yesMeth.signature != null, s"${yesMeth.name} should have a signature but does not") + // regression test for issue #10837 + val varargMeths = clsNode.methods.asScala.filter(_.name.startsWith("v")) + val bridge = varargMeths.filter(_.desc == "([Ljava/lang/String;)Ljava/lang/String;").head + assert(bridge.signature == null, "vararg bridges should not have generic signatures") + } + } } object invocationReceiversTestCode {