-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix generic signatures for mixin forwarders conflicting type parameter names #24567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Do we know yet why the name changed in the first place? |
|
I'll check that later today |
|
Alright, thanks @tanishiking. |
|
I confirmed that 9bd7774 already have this issue. Ever since generic signature generation for mixin forwarders was introduced in #23942, it has been producing signatures with name clashes in these cases. The problem is that the member type stored in scala3/compiler/src/dotty/tools/dotc/transform/Mixin.scala Lines 323 to 335 in 9aa08e2
|
|
Some CI checks have failed, but they are unrelated to this change:
|
…r names Fixes scala#24134 scala@f99dba2 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<A, B> implements scala.PartialFunction<A, B> { public <A> scala.Function1<A, B> compose(scala.Function1<A, A>); ``` which is obviously incorrect, the type parameter `A` of `compose[A]` is shadowed by the `A` in `JavaPartialFunction<A, B>`. The `compose`'s type parameter should use an unique name like: ```java public abstract class JavaPartialFunction<A, B> implements scala.PartialFunction<A, B> { public <T> scala.Function1<T, B> compose(scala.Function1<T, A>); ``` 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.)
| // 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 => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this enough? We should get the type parameters of all the enclosing classes.
The core of this issue has to do with substitution and the fact that we convert to String. I could substitute the enclosing of the enclosing class type parameters by something that will create conflicts (from a String point of view).
I would have suggested a more robust algorithm where we do a first pass to go over all the types referred to in the Signature and disambiguate if they have a different id but the same name (as a String).
Fixes #24134
f99dba2 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]implementsPartialFunction1[A, B],the generated mixin forwarder for
composein Function1 was like:which is obviously incorrect, the type parameter
Aofcompose[A]is shadowed by theAinJavaPartialFunction<A, B>.The
compose's type parameter should use an unique name like:This commit fix the problem by