From a93c849e7d64bcbb5f5c8d45c82339c75e844ed0 Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Sun, 7 Dec 2025 02:13:01 +0900 Subject: [PATCH 1/3] Only rename method type params shadowing enclosing class type params Previously, method type parameters were always renamed if they had the same name as any class type parameter, regardless of whether the class type parameter was actually used in the method signature. As a result, we rename methods's type params for unnecessary names like: https://github.com/scala/scala3/issues/24671 (Note that renaming actually doesn't cause binary incompatibility, and this change is just for make generic signature a bit clear and silence false-positive MIMA reporting). For example, in an example below, the Scala compiler rename the generic signature of `bar` to something like `bar[T1](x: T1): T1` because `T` is used by `Foo[T]`. However, this is unnessary rename because none of T in method signature refer the `T` of `Foo[T]`. ```scala class Foo[T]: def bar[T](x: T): T = ??? ``` This commit makes the renaming conditional: Method type parameters are only renamed when - (1) A class type parameter is referenced in the method signature, and - (2) That class type parameter is shadowed by a method type parameter --- .../dotc/transform/GenericSignatures.scala | 84 ++++++++++++++++--- .../shadowedTypeParam.check | 7 ++ .../shadowedTypeParam.scala | 43 ++++++++++ 3 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 tests/generic-java-signatures/shadowedTypeParam.check create mode 100644 tests/generic-java-signatures/shadowedTypeParam.scala diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index cf5386ea65e2..ae828e1b1cb8 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala @@ -49,24 +49,21 @@ object GenericSignatures { 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) - } - } + // Track class type parameter names that are shadowed by method type parameters + // Used to trigger renaming of method type parameters to avoid conflicts + val shadowedClassTypeParamNames = collection.mutable.Set.empty[String] val methodTypeParamRenaming = collection.mutable.Map.empty[String, String] + def freshTypeParamName(sanitizedName: String): String = { - if !usedNames.contains(sanitizedName) then sanitizedName + if !shadowedClassTypeParamNames.contains(sanitizedName) then sanitizedName else { var i = 1 var newName = sanitizedName + i - while usedNames.contains(newName) do + while shadowedClassTypeParamNames.contains(newName) do i += 1 newName = sanitizedName + i methodTypeParamRenaming(sanitizedName) = newName - usedNames += newName + shadowedClassTypeParamNames += newName newName } } @@ -347,7 +344,22 @@ object GenericSignatures { case mtd: MethodOrPoly => val (tparams, vparams, rte) = collectMethodParams(mtd) - if (toplevel && !sym0.isConstructor) polyParamSig(tparams) + if (toplevel && !sym0.isConstructor) { + if (sym0.is(Method)) { + val (usedMethodTypeParamNames, usedClassTypeParams) = collectUsedTypeParams(vparams :+ rte, sym0) + val methodTypeParamNames = tparams.map(tp => sanitizeName(tp.paramName.lastPart)).toSet + // Only add class type parameters to shadowedClassTypeParamNames if they are: + // 1. Referenced in the method signature, AND + // 2. Shadowed by a method type parameter with the same name + // This will trigger renaming of the method type parameter + usedClassTypeParams.foreach { classTypeParam => + val classTypeParamName = sanitizeName(classTypeParam.name) + if methodTypeParamNames.contains(classTypeParamName) then + shadowedClassTypeParamNames += classTypeParamName + } + } + polyParamSig(tparams) + } builder.append('(') for vparam <- vparams do jsig1(vparam) builder.append(')') @@ -448,6 +460,12 @@ object GenericSignatures { (initialSymbol.is(Method) && initialSymbol.typeParams.contains(sym)) ) + private def isTypeParameterInMethSig(sym: Symbol, initialSymbol: Symbol)(using Context) = + !sym.maybeOwner.isTypeParam && + sym.isTypeParam && ( + (initialSymbol.is(Method) && initialSymbol.typeParams.contains(sym)) + ) + // @M #2585 when generating a java generic signature that includes // a selection of an inner class p.I, (p = `pre`, I = `cls`) must // rewrite to p'.I, where p' refers to the class that directly defines @@ -528,4 +546,48 @@ object GenericSignatures { val rte = recur(mtd) (tparams.toList, vparams.toList, rte) end collectMethodParams + + /** Collect type parameters that are actually used in the given types. */ + private def collectUsedTypeParams(types: List[Type], initialSymbol: Symbol)(using Context): (Set[Name], Set[Symbol]) = + val usedMethodTypeParamNames = collection.mutable.Set.empty[Name] + val usedClassTypeParams = collection.mutable.Set.empty[Symbol] + + def collect(tp: Type): Unit = tp.dealias match + case ref @ TypeParamRef(_: PolyType, _) => + usedMethodTypeParamNames += ref.paramName + case TypeRef(pre, _) => + val sym = tp.typeSymbol + if isTypeParameterInMethSig(sym, initialSymbol) then + usedMethodTypeParamNames += sym.name + else if sym.isTypeParam && sym.isContainedIn(initialSymbol.topLevelClass) then + usedClassTypeParams += sym + else + collect(pre) + case AppliedType(tycon, args) => + collect(tycon) + args.foreach(collect) + case AndType(tp1, tp2) => + collect(tp1) + collect(tp2) + case OrType(tp1, tp2) => + collect(tp1) + collect(tp2) + case RefinedType(parent, _, refinedInfo) => + collect(parent) + collect(refinedInfo) + case TypeBounds(lo, hi) => + collect(lo) + collect(hi) + case ExprType(res) => + collect(res) + case AnnotatedType(tpe, _) => + collect(tpe) + case defn.ArrayOf(elemtp) => + collect(elemtp) + case _ => + () + + types.foreach(collect) + (usedMethodTypeParamNames.toSet, usedClassTypeParams.toSet) + end collectUsedTypeParams } diff --git a/tests/generic-java-signatures/shadowedTypeParam.check b/tests/generic-java-signatures/shadowedTypeParam.check new file mode 100644 index 000000000000..63ee165beb4f --- /dev/null +++ b/tests/generic-java-signatures/shadowedTypeParam.check @@ -0,0 +1,7 @@ +bar: public T Foo.bar(T) +baz: public T Foo.baz(T,Foo) +qux: public U Foo.qux(U,Foo) +quux: public scala.Tuple2 Foo.quux(T,U,Foo) +copy: public Bar Bar.copy(T) +compose: public scala.Function1 JavaPartialFunction.compose(scala.Function1) +compose: public scala.PartialFunction JavaPartialFunction.compose(scala.PartialFunction) diff --git a/tests/generic-java-signatures/shadowedTypeParam.scala b/tests/generic-java-signatures/shadowedTypeParam.scala new file mode 100644 index 000000000000..846546bc0dff --- /dev/null +++ b/tests/generic-java-signatures/shadowedTypeParam.scala @@ -0,0 +1,43 @@ +// All of type params shouldn't rename +class Foo[T]: + def bar[T](x: T): T = x + def baz[T](x: T, y: Foo[T]): T = x + def qux[U](x: U, y: Foo[T]): U = x + def quux[T, U](x: T, y: U, z: Foo[T]): (T, U) = (x, y) + +// https://github.com/scala/scala3/issues/24671 +final case class Bar[+T](t: T) + +// https://github.com/scala/scala3/issues/24134 +// The mixin forwarders for compose in Function1 method has signature +// `def compose[A](g: A => T1): A => R` +// Where the JavaPartialFunction[A, B] has type parameter A (name clash), +// The type parameter A in method should be renamed to avoid name duplication. +abstract class JavaPartialFunction[A, B] extends PartialFunction[A, B] + +@main def Test(): Unit = + val fooMethods = classOf[Foo[_]].getDeclaredMethods() + printMethodSig(fooMethods, "bar") + printMethodSig(fooMethods, "baz") + printMethodSig(fooMethods, "qux") + printMethodSig(fooMethods, "quux") + + val barMethods = classOf[Bar[_]].getDeclaredMethods() + printMethodSig(barMethods, "copy") + // copy$default$1 still have ` T Bar.copy$default$1` rather than ` T Bar.copy$default$1` + // as reported in https://github.com/scala/scala3/issues/24671 + // The type parameter rename occurs because the return type T refers the enclosing class's type param T. + // printMethodSig(barMethods, "copy$default$1") + + val jpfMethods = classOf[JavaPartialFunction[_, _]].getDeclaredMethods() + printMethodSigs(jpfMethods, "compose") + +def printMethodSig(methods: Array[java.lang.reflect.Method], name: String): Unit = + methods.find(_.getName.endsWith(name)).foreach { m => + println(s"$name: ${m.toGenericString}") + } + +def printMethodSigs(methods: Array[java.lang.reflect.Method], name: String): Unit = + methods.filter(_.getName == name).sortBy(_.toGenericString).foreach { m => + println(s"$name: ${m.toGenericString}") + } From 3d4d3974f44bbb5d55d52379443270e19497a8ef Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Mon, 8 Dec 2025 22:03:54 +0900 Subject: [PATCH 2/3] refactor: use TypeTraverser to collect used type params --- .../dotc/transform/GenericSignatures.scala | 51 ++++++------------- 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index ae828e1b1cb8..7c7b1b7a6098 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala @@ -551,43 +551,22 @@ object GenericSignatures { private def collectUsedTypeParams(types: List[Type], initialSymbol: Symbol)(using Context): (Set[Name], Set[Symbol]) = val usedMethodTypeParamNames = collection.mutable.Set.empty[Name] val usedClassTypeParams = collection.mutable.Set.empty[Symbol] + val collector = new TypeTraverser: + def traverse(tp: Type) = tp.dealias match + case ref @ TypeParamRef(_: PolyType, _) => + usedMethodTypeParamNames += ref.paramName + case TypeRef(pre, _) => + val sym = tp.typeSymbol + if isTypeParameterInMethSig(sym, initialSymbol) then + usedMethodTypeParamNames += sym.name + else if sym.isTypeParam && sym.isContainedIn(initialSymbol.topLevelClass) then + usedClassTypeParams += sym + else + traverse(pre) + case _ => + traverseChildren(tp) - def collect(tp: Type): Unit = tp.dealias match - case ref @ TypeParamRef(_: PolyType, _) => - usedMethodTypeParamNames += ref.paramName - case TypeRef(pre, _) => - val sym = tp.typeSymbol - if isTypeParameterInMethSig(sym, initialSymbol) then - usedMethodTypeParamNames += sym.name - else if sym.isTypeParam && sym.isContainedIn(initialSymbol.topLevelClass) then - usedClassTypeParams += sym - else - collect(pre) - case AppliedType(tycon, args) => - collect(tycon) - args.foreach(collect) - case AndType(tp1, tp2) => - collect(tp1) - collect(tp2) - case OrType(tp1, tp2) => - collect(tp1) - collect(tp2) - case RefinedType(parent, _, refinedInfo) => - collect(parent) - collect(refinedInfo) - case TypeBounds(lo, hi) => - collect(lo) - collect(hi) - case ExprType(res) => - collect(res) - case AnnotatedType(tpe, _) => - collect(tpe) - case defn.ArrayOf(elemtp) => - collect(elemtp) - case _ => - () - - types.foreach(collect) + types.foreach(collector.traverse) (usedMethodTypeParamNames.toSet, usedClassTypeParams.toSet) end collectUsedTypeParams } From 9b85e447f352e1df9233350f3803fe50e0245d80 Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Mon, 8 Dec 2025 22:16:16 +0900 Subject: [PATCH 3/3] Add another test case that shadows class's type param https://github.com/scala/bug/issues/13144 --- .../shadowedTypeParam.check | 2 ++ .../shadowedTypeParam.scala | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/generic-java-signatures/shadowedTypeParam.check b/tests/generic-java-signatures/shadowedTypeParam.check index 63ee165beb4f..7882a0d479f7 100644 --- a/tests/generic-java-signatures/shadowedTypeParam.check +++ b/tests/generic-java-signatures/shadowedTypeParam.check @@ -3,5 +3,7 @@ baz: public T Foo.baz(T,Foo) qux: public U Foo.qux(U,Foo) quux: public scala.Tuple2 Foo.quux(T,U,Foo) copy: public Bar Bar.copy(T) +copy$default$1: public T Bar.copy$default$1() +m: public T C.m() compose: public scala.Function1 JavaPartialFunction.compose(scala.Function1) compose: public scala.PartialFunction JavaPartialFunction.compose(scala.PartialFunction) diff --git a/tests/generic-java-signatures/shadowedTypeParam.scala b/tests/generic-java-signatures/shadowedTypeParam.scala index 846546bc0dff..8c6a644c7421 100644 --- a/tests/generic-java-signatures/shadowedTypeParam.scala +++ b/tests/generic-java-signatures/shadowedTypeParam.scala @@ -8,6 +8,13 @@ class Foo[T]: // https://github.com/scala/scala3/issues/24671 final case class Bar[+T](t: T) +// `m: public T C.m()` rather than `m: public T C.m()` +// that was wrong and could be crashed with +// `val c = C[String]; String x = c.m();`. +abstract class C[T]: + def x: T + def m[T] = x + // https://github.com/scala/scala3/issues/24134 // The mixin forwarders for compose in Function1 method has signature // `def compose[A](g: A => T1): A => R` @@ -24,10 +31,13 @@ abstract class JavaPartialFunction[A, B] extends PartialFunction[A, B] val barMethods = classOf[Bar[_]].getDeclaredMethods() printMethodSig(barMethods, "copy") - // copy$default$1 still have ` T Bar.copy$default$1` rather than ` T Bar.copy$default$1` + // copy$default$1 have ` T Bar.copy$default$1` rather than ` T Bar.copy$default$1` // as reported in https://github.com/scala/scala3/issues/24671 // The type parameter rename occurs because the return type T refers the enclosing class's type param T. - // printMethodSig(barMethods, "copy$default$1") + printMethodSig(barMethods, "copy$default$1") + + val cMethods = classOf[C[_]].getDeclaredMethods() + printMethodSig(cMethods, "m") val jpfMethods = classOf[JavaPartialFunction[_, _]].getDeclaredMethods() printMethodSigs(jpfMethods, "compose")