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

feat(#376): Towards Agent Base Acrhitecture #379

Merged
merged 11 commits into from
Aug 13, 2024

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Aug 12, 2024

In this PR I has made an attempt to change an acritecture from "Handlers" to "Agents".
Agents are independent and try do decompile only those instructions, they understand.
This changes might make the architecture a bit less coupled.

Closes: #376.
History:


PR-Codex overview

This PR adds a new DecompilationAgent interface and updates various classes to implement it. It also changes the package name and imports for consistency.

Detailed summary

  • Added DecompilationAgent interface
  • Renamed and updated classes to implement DecompilationAgent
  • Changed package name from handlers to agents
  • Updated imports for consistency

The following files were skipped due to too many changes: src/main/java/org/eolang/opeo/ast/Opcode.java, src/main/java/org/eolang/opeo/decompilation/handlers/BipushHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/NewHandler.java, src/main/java/org/eolang/opeo/SelectiveDecompiler.java, src/main/java/org/eolang/opeo/decompilation/handlers/LabelHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/DupHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/GetStaticHnadler.java, src/test/java/org/eolang/opeo/decompilation/DecompilerMachineTest.java, src/main/java/org/eolang/opeo/decompilation/handlers/AddHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/CheckCastHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/SubstractionHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/NewArrayHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/IfHandler.java, src/main/java/org/eolang/opeo/decompilation/DecompilerMachine.java, src/main/java/org/eolang/opeo/decompilation/handlers/PutFieldHnadler.java, src/main/java/org/eolang/opeo/decompilation/handlers/MulHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/InvokedynamicHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/InvokestaticHander.java, src/main/java/org/eolang/opeo/decompilation/handlers/InvokevirtualHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/InvokeinterfaceHandler.java, src/main/java/org/eolang/opeo/decompilation/agents/PutFieldAgent.java, src/main/java/org/eolang/opeo/decompilation/handlers/ReturnHandler.java, src/main/java/org/eolang/opeo/decompilation/handlers/StoreToArrayHandler.java, src/main/java/org/eolang/opeo/decompilation/agents/LoadAgent.java, src/main/java/org/eolang/opeo/decompilation/handlers/ConstHandler.java, src/main/java/org/eolang/opeo/decompilation/agents/CastAgent.java, src/test/java/org/eolang/opeo/ast/AddTest.java, src/main/java/org/eolang/opeo/decompilation/agents/StoreAgent.java, src/main/java/org/eolang/opeo/decompilation/DecompilerState.java, src/main/java/org/eolang/opeo/decompilation/handlers/InvokespecialHandler.java, src/main/java/org/eolang/opeo/decompilation/agents/AllAgents.java

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

@volodya-lombrozo
Copy link
Member Author

@yegor256 Could you take a look, please? There are lot's of changes, but they are rather similar.

@volodya-lombrozo
Copy link
Member Author

@yegor256 friendly reminder

@yegor256
Copy link
Member

yegor256 commented Aug 13, 2024

@volodya-lombrozo looks good! However, it's rather hard to understand what exactly happens when those agents meet certain instructions. If you can create a set of tests (maybe in YAML files), where I can see something like this:

opcodes:
  - ILOAD 423
  - ILOAD 7
  - IADD
agents:
  - AdditionAgent
  - SomeOtherAgent
eo:
  432.add 7

Maybe you can merge this PR and then create such tests in additional PRs. They will help us understand how agents work, by illustrative examples.

Also, in the opcodes section we may want to have EO objects too, not only opcodes. Maybe it should be input section, instead of opcodes.

@volodya-lombrozo
Copy link
Member Author

@yegor256 Agree. Added a new issue for that tests. If all the rest is fine, could you accept this changes through a PR review, please? It seems, right now you leave only comments.

Copy link
Member

@yegor256 yegor256 left a comment

Choose a reason for hiding this comment

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

@volodya-lombrozo looks good, but integration tests are very much needed

@volodya-lombrozo
Copy link
Member Author

@yegor256 Thank you for the review! I will try to implement this tests as soon as possible.

@volodya-lombrozo
Copy link
Member Author

@rultor merge

@rultor
Copy link
Contributor

rultor commented Aug 13, 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 3bb47a6 into objectionary:master Aug 13, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Aug 13, 2024

@rultor merge

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

@0crat
Copy link

0crat commented Aug 13, 2024

@volodya-lombrozo Thanks for your contribution! You've earned +5 points: +20 base, +30 for hits-of-code (capped at 30 for 2155 HOC), -7 for exceeding 200 HOC, -15 for exceeding 800 HOC, -15 for no code review, -10 for no reviewer comments. While the large code size impacted your score, we appreciate your effort. Keep the contributions coming, but consider smaller, more focused changes. Your running balance is +196.

@0crat
Copy link

0crat commented Aug 13, 2024

@yegor256 🎉 Fantastic review! You've earned a whopping +53 points: +15 base, +30 for reviewing 2155 hits-of-code, and +8 for your 8 insightful comments. Your dedication is paying off - your balance now stands at +145. Keep up the great work! 💪

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.

Refactoring: Agent Base Architecture
4 participants