-
Notifications
You must be signed in to change notification settings - Fork 11
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
use MethodParameter attribute if code attribute is missing #19
base: master
Are you sure you want to change the base?
Conversation
for (long i = 0; i < count; i++) { | ||
int nameIndex = raw.getU2At(offset); | ||
int accessFlags = raw.getU2At(offset + 2); | ||
parameters.add(new MethodParameterEntry(cp.getEntry(nameIndex), accessFlags)); |
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.
nameIndex
may be 0, indicating null
. getEntry
does not accept 0 arguments:
if (index == 0) throw new ConfusedCFRException("Attempt to fetch element 0 from constant pool"); |
|
||
@Override | ||
public boolean isGoodName() { | ||
return true; |
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.
Same here, no good if it's 0-name; it's totally legal to have null name in MethodParameters (storing parameter flags only) but valid name in LocalVariableTable/LocalVariableTypeTable
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.
not sure what exactly isGoodName is supposed to represent, but I don't see a very easy way to fix this currently since this doesn't know which parameter it is until dumpParameter is called...
|
||
@Override | ||
public Dumper dumpParameter(Dumper d, MethodPrototype prototype, int index, boolean defines) { | ||
return d.parameterName(forced ? name : parameters.get(index).name.getValue(), this, prototype, index, defines); |
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.
Same, will NPE on .getValue
On a side note, I don't know why cfr's author doesn't just implement the length calculation and attribute name reading in the base |
Also as a side note: javac actually does NOT generate In Java 20: public interface Apple {
int my(int argumentName);
} compiles with
No |
yeah that's expected, but this is mostly for decompiling remapped jars, and the remapping process (intermediary -> named) does emit the MethodParameters attributes if parameters are present, at least in loom |
this applies parameter names for abstract methods and interface methods