From 4e284c5bae57788764c1b7ba704c7d6f829028da Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Fri, 28 Nov 2025 15:32:38 +0900 Subject: [PATCH] Fix generic signatures for mixin forwarders conflicting type parameter names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #24134 https://github.com/scala/scala3/commit/f99dba239f9f876ab62cd8d870e3e56e1ed7a5d7 started emitting Java generic signature for mixin forwarders. However, when generating Java generic signatures for mixin forwarders, method-level type parameters could shadow class-level type parameters with the same name. For example, when `JavaPartialFunction[A, B]` implements `PartialFunction1[A, B]`, ```scala trait Function1[-T1, +R]: def compose[A](g: A => T1): A => R = ??? trait PartialFunction[-A, +B] extends Function1[A, B]: def compose[R](k: PartialFunction[R, A]): PartialFunction[R, B] = ??? abstract class JavaPartialFunction[A, B] extends PartialFunction[A, B] ``` the generated mixin forwarder for `compose` in Function1 was like: ```java public abstract class JavaPartialFunction implements scala.PartialFunction { public scala.Function1 compose(scala.Function1); ``` which is obviously incorrect, the type parameter `A` of `compose[A]` is shadowed by the `A` in `JavaPartialFunction`. The `compose`'s type parameter should use an unique name like: ```java public abstract class JavaPartialFunction implements scala.PartialFunction { public scala.Function1 compose(scala.Function1); ``` This commit fix the problem by - Tracks class-level type parameter names when generating method signatures - Renames conflicting method-level type parameters (A → A1, A2, etc.) --- .../dotc/transform/GenericSignatures.scala | 53 +++++++++++++++---- .../test/dotc/pos-test-pickling.excludelist | 1 + tests/pos/i24134/JavaPartialFunction.scala | 20 +++++++ tests/pos/i24134/JavaTestServer.java | 18 +++++++ 4 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 tests/pos/i24134/JavaPartialFunction.scala create mode 100644 tests/pos/i24134/JavaTestServer.java diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index e7a852745e4c..cf5386ea65e2 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala @@ -41,9 +41,36 @@ object GenericSignatures { @noinline private final def javaSig0(sym0: Symbol, info: Type)(using Context): Option[String] = { + // This works as long as mangled names are always valid valid Java identifiers, + // if we change our name encoding, we'll have to `throw new UnknownSig` here for + // names which are not valid Java identifiers (see git history of this method). + def sanitizeName(name: Name): String = name.mangledString + val builder = new StringBuilder(64) val isTraitSignature = sym0.enclosingClass.is(Trait) + // Collect class-level type parameter names to avoid conflicts with method-level type parameters + val usedNames = collection.mutable.Set.empty[String] + if(sym0.is(Method)) { + sym0.enclosingClass.typeParams.foreach { tp => + usedNames += sanitizeName(tp.name) + } + } + val methodTypeParamRenaming = collection.mutable.Map.empty[String, String] + def freshTypeParamName(sanitizedName: String): String = { + if !usedNames.contains(sanitizedName) then sanitizedName + else { + var i = 1 + var newName = sanitizedName + i + while usedNames.contains(newName) do + i += 1 + newName = sanitizedName + i + methodTypeParamRenaming(sanitizedName) = newName + usedNames += newName + newName + } + } + def superSig(cls: Symbol, parents: List[Type]): Unit = { def isInterfaceOrTrait(sym: Symbol) = sym.is(PureInterface) || sym.is(Trait) @@ -133,15 +160,16 @@ object GenericSignatures { else Right(parent)) - def paramSig(param: TypeParamInfo): Unit = { - builder.append(sanitizeName(param.paramName.lastPart)) + def tparamSig(param: TypeParamInfo): Unit = { + val freshName = freshTypeParamName(sanitizeName(param.paramName.lastPart)) + builder.append(freshName) boundsSig(hiBounds(param.paramInfo.bounds)) } def polyParamSig(tparams: List[TypeParamInfo]): Unit = if (tparams.nonEmpty) { builder.append('<') - tparams.foreach(paramSig) + tparams.foreach(tparamSig) builder.append('>') } @@ -151,6 +179,12 @@ object GenericSignatures { builder.append(';') } + def typeParamSigWithName(sanitizedName: String): Unit = { + builder.append(ClassfileConstants.TVAR_TAG) + builder.append(sanitizedName) + builder.append(';') + } + def methodResultSig(restpe: Type): Unit = { val finalType = restpe.finalResultType val sym = finalType.typeSymbol @@ -160,11 +194,6 @@ object GenericSignatures { jsig(finalType) } - // This works as long as mangled names are always valid valid Java identifiers, - // if we change our name encoding, we'll have to `throw new UnknownSig` here for - // names which are not valid Java identifiers (see git history of this method). - def sanitizeName(name: Name): String = name.mangledString - // Anything which could conceivably be a module (i.e. isn't known to be // a type parameter or similar) must go through here or the signature is // likely to end up with Foo.Empty where it needs Foo.Empty$. @@ -244,7 +273,11 @@ object GenericSignatures { // don't emit type param name if the param is upper-bounded by a primitive type (including via a value class) if erasedUnderlying.isPrimitiveValueType then jsig(erasedUnderlying, toplevel = toplevel, unboxedVCs = unboxedVCs) - else typeParamSig(ref.paramName.lastPart) + else { + val name = sanitizeName(ref.paramName.lastPart) + val nameToUse = methodTypeParamRenaming.getOrElse(name, name) + typeParamSigWithName(nameToUse) + } case ref: TermRef if ref.symbol.isGetter => // If the type of a val is a TermRef to another val, generating the generic signature @@ -258,7 +291,7 @@ object GenericSignatures { case ref: SingletonType => // Singleton types like `x.type` need to be widened to their underlying type - // For example, `def identity[A](x: A): x.type` should have signature + // For example, `def identity[A](x: A): x.type` should have signature // with return type `A` (not `java.lang.Object`) jsig(ref.underlying, toplevel = toplevel, unboxedVCs = unboxedVCs) diff --git a/compiler/test/dotc/pos-test-pickling.excludelist b/compiler/test/dotc/pos-test-pickling.excludelist index 9706f95cdfb9..c9a5b86eb0fe 100644 --- a/compiler/test/dotc/pos-test-pickling.excludelist +++ b/compiler/test/dotc/pos-test-pickling.excludelist @@ -33,6 +33,7 @@ i17186b.scala i11982a.scala i17255 i17735.scala +i24134 # Tree is huge and blows stack for printing Text i7034.scala diff --git a/tests/pos/i24134/JavaPartialFunction.scala b/tests/pos/i24134/JavaPartialFunction.scala new file mode 100644 index 000000000000..4fdda74e853c --- /dev/null +++ b/tests/pos/i24134/JavaPartialFunction.scala @@ -0,0 +1,20 @@ +final class Flow[In, Out]: + def collect[T](pf: PartialFunction[Out, T]): Flow[In, T] = ??? + +object Flow: + def create[T](): Flow[T, T] = ??? + + +abstract class Message: + def asTextMessage: TextMessage + +abstract class TextMessage extends Message + +abstract class JavaPartialFunction[A, B] extends PartialFunction[A, B]: + @throws(classOf[Exception]) // required, compiles without annotation + def apply(x: A, isCheck: Boolean): B + + final def isDefinedAt(x: A): Boolean = ??? + final override def apply(x: A): B = ??? + final override def applyOrElse[A1 <: A, B1 >: B](x: A1, default: A1 => B1): B1 = ??? + diff --git a/tests/pos/i24134/JavaTestServer.java b/tests/pos/i24134/JavaTestServer.java new file mode 100644 index 000000000000..05f5b61ef4b0 --- /dev/null +++ b/tests/pos/i24134/JavaTestServer.java @@ -0,0 +1,18 @@ +public class JavaTestServer { + public static Flow greeter() { + return Flow.create() + .collect( + new JavaPartialFunction() { + @Override + public Message apply(Message msg, boolean isCheck) throws Exception { + if (isCheck) throw new RuntimeException(); + else return handleTextMessage(msg.asTextMessage()); + } + }); + } + + public static TextMessage handleTextMessage(TextMessage msg) { + return null; + } +} +