-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
Correct synthetic parameter handling in inner classes #98
base: master
Are you sure you want to change the base?
Conversation
@@ -463,7 +463,7 @@ index ebb845dff674965ff3e0e8011db33dae5270e24d..a02c40a8a8a2ad1a1888d8a5370625c2 | |||
+ | |||
+ int j = 0; | |||
+ for (int i = start; i < lstParameters.size(); ++i) { | |||
+ if (mask == null || mask.get(i) != null) { | |||
+ if (mask == null || mask.get(i) == null) { |
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.
Huh.. wonder how this ever worked. Also probably need to change isNew to isGenNew a few lines above where the mask is set so that we can make sure desc.getSignature is not null. Err.. nevermind its inside a desc.getSignature() != null if. But that should probably be changed for proper fix.
+ // However, synthetic parameters are added at the beginnings of method signatures (at least usually) | ||
+ // so we really want to start from the beginning | ||
+ // if it ends up that this assumption is not true, we'd have to do some sort of fuzzy matching here, or earlier | ||
+ // on traverse multiple steps in the graph in getEnclosingVarField |
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.
As discussed in Discord, this assumption is incorrect.
Would need a deeper dive into the compiler to find all cases where synthetics are added to the beginning or end.
index fe372662be3fbd310f94831b47f21b92033a851c..f50e767ab81de739cad2059b5a8279fcd7cdba25 100644 | ||
--- a/src/org/jetbrains/java/decompiler/main/rels/NestedClassProcessor.java | ||
+++ b/src/org/jetbrains/java/decompiler/main/rels/NestedClassProcessor.java | ||
@@ -35,6 +35,8 @@ import java.util.*; |
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.
I've tested this and it does not seem to effect decompiled code at all. So i'm gunna split this out and we can discuss it in more depth later.
Pulled in the mask fix in e661013 |
Awaiting discussion & rebasing. |
Broken out of #96
It appears that a lot of inner classes have gained a Signature attribute, which causes generic signature processing to activate. This exposed some issues in its handling of synthetic constructor parameters, which I think I've taken care of enough to correctly
(not sure how these changes would react if someone disabled enum handling though)
Comparisons (via VanillaGradle)
1.16.5
note: There are some new warnings in the decompiler output here, which appear to be classes where PG has stripped the constructor. This is due to the change to now calculate synthetic parameter masks for inner classes that are marked
static
. This change is required because starting with 21w19a, Mojang's tooling has corrupted Minecraft's inner class metadata and made some non-static inner classes be marked as static.The warnings:
21w19a