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

Unroll PHI/UNPHI Transformations #720

Merged
merged 19 commits into from
Sep 24, 2024

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Sep 23, 2024

In this PR I unroll all the changes made by phi/unphi transformations.
When these transformations are applied to the xmir genereated by jeo-maven-plugin, the xmir structure is changed significantly.
We apply this "back" transformations to make the xmir undestandable by jeo-maven-plugin after phi/unphi transformations.

Related to #705.


PR-Codex overview

This PR primarily focuses on enhancing the Bytecode handling classes by adding @ToString and @EqualsAndHashCode annotations for better object representation and equality checks, along with updates to the UnrollMojo class for handling PHI/UNPHI transformations.

Detailed summary

  • Added @ToString and @EqualsAndHashCode annotations to:
    • BytecodeHandler
    • BytecodeMethodBuilder
    • BytecodeDefaultValue
    • BytecodeInstructionEntry
  • Updated BytecodeClass and BytecodeMethodProperties to use List instead of Collection.
  • Modified UnrollMojo to include new unroll-phi functionality.
  • Added new XML transformation files for handling data and method calls.
  • Implemented tests in CanonicalXmirTest to verify bytecode integrity after transformations.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@volodya-lombrozo
Copy link
Member Author

Blocked by #715

@volodya-lombrozo
Copy link
Member Author

Blocked by objectionary/eo#3383

* @throws IOException If fails.
*/
private static XML parse(final String eoprog) throws IOException {
return new EoSyntax("name", new InputOf(eoprog)).parsed();
Copy link
Member

@maxonfjvipon maxonfjvipon Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo I think this is the problem of objectionary/eo#3373 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxonfjvipon Please, read this issue objectionary/eo#3380 it's a bug in eo

Copy link
Member

@maxonfjvipon maxonfjvipon Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo I think the default constructor won't help you here because the name should taken from somewhere. Can't you set it manually from the file name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxonfjvipon Please, extend this issue with your knowledge about file names objectionary/eo#3380

@volodya-lombrozo volodya-lombrozo marked this pull request as ready for review September 24, 2024 14:27
@volodya-lombrozo
Copy link
Member Author

@rultor merge

@rultor
Copy link
Contributor

rultor commented Sep 24, 2024

@rultor merge

@volodya-lombrozo OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 23abedb into objectionary:master Sep 24, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Sep 24, 2024

@rultor merge

@volodya-lombrozo Done! FYI, the full log is here (took me 32min)

@0crat
Copy link

0crat commented Sep 24, 2024

@volodya-lombrozo Thanks for your contribution! Your effort is appreciated, but there's room for improvement. Here's the breakdown: +4 base points, -8 for 783 hits-of-code (exceeding 200), -16 for missing code review, and +24 to meet the minimum reward. Remember, our policy values quality over quantity. Aim for smaller, focused contributions and ensure code reviews for better rewards. Your current balance is +109. Keep refining your approach!

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.

5 participants