Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 73 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems name shadowing can also happen for nested classes, though that's a lot more exotic

abstract class C[T] {
  val x: T
  class I[T <: x.type]
}

the class C$I has signautre <T:TT;>Ljava/lang/Object;, which I guess is also wrong? Either way, I think we don't need to fix this in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. In that case, the renaming strategy for methods in nested classes will be more complicated. I won't fix it in this PR. Instead, I gonna rebase #24621 on top of this change and address it there.

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(')')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we have

abstract class TypeTraverser(using Context) extends TypeAccumulator[Unit] {
def traverse(tp: Type): Unit
def apply(x: Unit, tp: Type): Unit = traverse(tp)
protected def traverseChildren(tp: Type): Unit = foldOver((), tp)
}
👍

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
}
7 changes: 7 additions & 0 deletions tests/generic-java-signatures/shadowedTypeParam.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
bar: public <T> T Foo.bar(T)
baz: public <T> T Foo.baz(T,Foo<T>)
qux: public <U> U Foo.qux(U,Foo<T>)
quux: public <T,U> scala.Tuple2<T, U> Foo.quux(T,U,Foo<T>)
copy: public <T> Bar<T> Bar.copy(T)
compose: public <A1> scala.Function1<A1, B> JavaPartialFunction.compose(scala.Function1<A1, A>)
compose: public <R> scala.PartialFunction<R, B> JavaPartialFunction.compose(scala.PartialFunction<R, A>)
43 changes: 43 additions & 0 deletions tests/generic-java-signatures/shadowedTypeParam.scala
Original file line number Diff line number Diff line change
@@ -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 `<T1> T Bar.copy$default$1` rather than `<T> 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")
Copy link
Member Author

@tanishiking tanishiking Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still MiMa won't be happy with <T1> T Bar.copy$default$1 rather than <T> T Bar.copy$default$1 (but the original signature was wrong, the return type T is shadowed by method type parameter.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I didn't pay attention. A small reproducer is

abstract class C[T] {
  def x: T
  def m[T] = x
}

We have that wrong in Scala 2, I created scala/bug#13144.

Copy link
Member Author

@tanishiking tanishiking Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! that's a good reproducer :)
It seems like renaming to <T1:Ljava/lang/Object;>()TT; is right thing to do 👍


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}")
}
Loading