AbstractClassMappingImpl.complete(...) infers wrong method name
CheaterCodes opened this issue · 0 comments
Context
I have three classes:
abstract class A {
private void foo() {
[...]
}
}
interface B {
void foo();
}
class C extends A implements B {
public void foo() {
[...]
}
}
I want to remap these classes using the following mapping set:
A.foo()V -> A.methodA()V
B.foo()V -> B.methodB()V
Problem
Expected result:
C.foo()V
is overriding B.foo()
and should therefore be remapped to C.methodB()V
Actual result:
C.foo()V
gets remapped to C.methodA()V
even though A.foo()V
is private and can't be overriden.
Note that the erronous result is also ordering dependent, so it might not occur in all situations.
Origin
The reason for this happening are the changes in commit 2a7c0b6.
The changes were made in response to issue CadixDev/Mercury#14.
The argument made in the code comments doesn't make sense to me:
Check if there are any methods here that override the return type of a parent method.
While the issue in Mercury is correct in saying that
Java allows overriding methods with different return types, if the type is a subclass of the OG return type.
, the same is not applicable to the JVM, as the spec says:
An instance method mC can override another instance method mA iff all of the following are true:
- mC has the same name and descriptor as mA.
[...]
Suggested Solution
The minimum fix would be to add accessibility checks to the erronous code, preventing it from inheriting a name from a private method. However, keep in mind that the JVM requires strictly identical method descriptors, so this solution could still cause issues (A method String foo()
can not override the method Object foo()
in the JVM, and inferring the names that way is still erronous).
My preferred fix would be to revert the referenced commit in Lorenz and instead fix the issue in a tool specifically made for source remapping if possible (e.g. Mercury).
If separating asm remapping from source remapping is too difficult, maybe there could be a flag of some sort to change that behaviour.