Skip to content
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

Inefficiency and random unexpected exclusions in Renamer #97

Open
ItzSomebody opened this issue Apr 26, 2021 · 1 comment
Open

Inefficiency and random unexpected exclusions in Renamer #97

ItzSomebody opened this issue Apr 26, 2021 · 1 comment
Labels
radon issue Radon isn't functioning within normal parameters suggestion New feature or request top priority Extra attention is needed

Comments

@ItzSomebody
Copy link
Owner

ItzSomebody commented Apr 26, 2021

From convo on discord (huge thanks to @MrExplode for spotting!): https://canary.discord.com/channels/495471385351553024/495481816128421899/831649568336117780

The current renamer design (both radon 2 and 3) attempts to check all subclasses of a parent class for inheritance via className + '.' + methodName + methodDesc. Radon achieves this through recursion and has a nicety of being easy to understand. Unfortunately, what this does is search potentially unrelated classes -- in fact, the entire inheritance graph is traversed. Since I am on Windows (disgusting) at the moment, here is a visual aid I whipped up in mspaint.exe:

Untitled

With the current algorithm, all classes in this graph will be traversed when checking inheritance for some virtual method in MyParentClass. This is very likely the cause of the random reports given to me about methods being excluded by renaming unintentionally.

@ItzSomebody ItzSomebody added radon issue Radon isn't functioning within normal parameters top priority Extra attention is needed labels Apr 26, 2021
@ItzSomebody
Copy link
Owner Author

Also, since this requires a little bit of rethinking of the renamer, perhaps this would be a good time to

  1. Fix remapping for a specific annoying case of lambdas (see: https://github.com/videogame-hacker/oof-jvm/tree/master/src/anonymous_lambda) and

  2. Add the much requested feature of remapping hardcoded reflections/methodhandles

@ItzSomebody ItzSomebody added the suggestion New feature or request label Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
radon issue Radon isn't functioning within normal parameters suggestion New feature or request top priority Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant