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 fernflower as the default decompiler #2704

Merged
merged 5 commits into from
Jun 19, 2023

Conversation

testforstephen
Copy link
Contributor

This PR does the following:

  • It replaces the default disassembler ClassFileBytesDisassembler with fernflower decompiler, which provides better decompilation results.
  • It records the line mappings when decompiling class file, which is needed by the debugger to re-map the line numbers when a breakpoint occurs.

@testforstephen
Copy link
Contributor Author

test this please

@testforstephen
Copy link
Contributor Author

The failed test cases are random, unrelated to this PR changes.

@testforstephen testforstephen added this to the End June 2023 milestone Jun 19, 2023
@testforstephen testforstephen merged commit dfe7fff into eclipse-jdtls:master Jun 19, 2023
5 of 6 checks passed
@testforstephen testforstephen deleted the jinbo_decompiler branch June 19, 2023 08:10
@rgrunber
Copy link
Contributor

rgrunber commented Jun 19, 2023

For reference, https://github.com/JetBrains/intellij-community/tree/idea/231.9011.34/plugins/java-decompiler/engine#licence . However, I can't find anything at ClearlyDefined or https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues . We may will need to open an issue for approval on this.

Update: Additional concern from @fbricon , does this co-exist well with https://marketplace.visualstudio.com/items?itemName=dgileadi.java-decompiler (source) ? I think the extension gets priority over our "default" provider.

Filed at https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/9099 .

@testforstephen
Copy link
Contributor Author

For reference, https://github.com/JetBrains/intellij-community/tree/idea/231.9011.34/plugins/java-decompiler/engine#licence . However, I can't find anything at ClearlyDefined or https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues . We may will need to open an issue for approval on this.

@rgrunber Thanks for opening the IP issue for legal review.

Update: Additional concern from @fbricon , does this co-exist well with https://marketplace.visualstudio.com/items?itemName=dgileadi.java-decompiler (source) ? I think the extension gets priority over our "default" provider.

Yes, the built-in decompiler has the lowest priority. If you install the 3rd party decompiler extension, it will take precedence over the default one.

@rgrunber
Copy link
Contributor

@fbricon mentioned something about a clash between the fernflower settings and the ones we set, but I'm not seeing it. The settings this PR sets are at https://github.com/eclipse/eclipse.jdt.ls/pull/2704/files#diff-18fc39092640259fd06479de72a5e1b47512bbf199c9373cc2362f0846b62906R68-R81 and they don't persist beyond the FernFlower instance created, which wouldn't be used if some 3rd part provider contributed a decompiler that took priority.

@testforstephen
Copy link
Contributor Author

They don't share settings with each other. The 3rd party fernflower provider has a setting java.decompiler.fernflower for user to configure, while our built-in one does not support that but uses a default tuned settings. I believe most users don’t have knowledge on how to customize the decompiler settings. I will keep it as it is unless there is a feature request to custom the decompiler options.

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.

None yet

4 participants