ForgeFlower icon indicating copy to clipboard operation
ForgeFlower copied to clipboard

Local variable names in inner class methods aren't fixed when they conflict with outer names

Open DenWav opened this issue 5 years ago • 3 comments

This is due to https://github.com/MinecraftForge/ForgeFlower/blob/master/FernFlower-Patches/0037-Do-not-rebuild-variable-names-in-lambdas.patch

For background on what that code is doing (re: the patch message): That allows the decompiler to fix variable names in a nested class or lambda expression which clashes with the name of an outer variable.

What's happening is it's checking the names for the current method against the variable names defined in the outer scope. That's why setNewOuterNames is passed to VarNamesCollector, then checked for in VarNamesCollector.getFreeName() - the outer names are compared against the names for the nested method.

What FernFlower's code originally did was append x to local variable names when conflicts came up - but now it doesn't do that because of that patch. Considering that's the only thing that method is actually doing I don't know what the patch is actually trying to fix. The commit which added the patch said it fixes #88, but there is no issue 88 yet.

This comes up relatively infrequently, and I guess never in MCP due to how LVT is handled, but it causes an issue with my usage. This quick and dirty fix is because of this issue - previous versions of FernFlower would rename the inner i variable to ix. https://github.com/PaperMC/Paper/blob/mappings/mojang/Spigot-Server-Patches/0266-Optimize-BlockPosition-helper-methods.patch#L113-L134

DenWav avatar Dec 29 '20 08:12 DenWav

See the comment on the commit, he meant to reference #80

ichttt avatar Dec 29 '20 09:12 ichttt

Ah, so this is easy to fix just by excluding this and keeping the method call. I'll PR that tomorrow.

DenWav avatar Dec 29 '20 11:12 DenWav

Okay patch 9 totally changes how variables are named and so it actually makes patch 37 a little odd considering it's not doing anything. But I cannot follow the new system, and can't figure out how to backport this feature onto it. I'll just leave this issue here and hope someone smarter than me can figure it out.

DenWav avatar Dec 31 '20 01:12 DenWav