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

Use local var context with static position methods #14

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

Machine-Maker
Copy link
Member

Ok, so this isn't working 100%, but I don't think I know enough to figure out to accurately get the LocalVariableNode referenced by a VarInsnNode. I thought it was enough to just get the MethodNode.localVariables field, iterate through and find one with the matching index, but there seem to be duplicates. I noticed this while looking at CrashReportCategory#formatLocation L47. The sectionPosX should have an index pointing to that local var node, but there is a second one in the list that points towards the Throwable in the catch block above.

@Machine-Maker Machine-Maker changed the title Use local var context with static position methods WIP Use local var context with static position methods Jul 27, 2023
@Machine-Maker
Copy link
Member Author

Ok, the issue where it matched the wrong local var has been fixed by just walking back on the instruction chain until you find a label node that matches the start label of one of the matching local vars.

@Machine-Maker Machine-Maker changed the title WIP Use local var context with static position methods Use local var context with static position methods Jul 28, 2023
@Machine-Maker
Copy link
Member Author

Ok, this should be good to go now. Handles methods on SectionPos, QuartPos, and BlockPos and names stuff accordingly. One future optimization could be to detect simple math surrounding the parameter to a method in order to persist the var, method, field name through that logic.

@DenWav DenWav force-pushed the main branch 2 times, most recently from 42706b9 to 7976d1d Compare July 29, 2023 04:29
@Machine-Maker
Copy link
Member Author

Machine-Maker commented Jan 14, 2024

Rebased and re-tested.

It's not simple to add tests for the QuartPos, SectionPos stuff, because it uses the ContainerContext to see if it can find already named lvt (like from parameters)

@Machine-Maker Machine-Maker force-pushed the better-positions branch 3 times, most recently from 56593f4 to 7ee3b4b Compare January 14, 2024 19:39
@MiniDigger MiniDigger merged commit a6f7366 into main Jan 14, 2024
2 checks passed
@MiniDigger MiniDigger deleted the better-positions branch January 14, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants