Add a new callback decompilation_changed in decompilers#180
Add a new callback decompilation_changed in decompilers#180NishTheFish-dev wants to merge 23 commits intomainfrom
decompilation_changed in decompilers#180Conversation
|
Sanity check for test case in latest CI: https://github.com/binsync/libbs/actions/runs/21654130527/job/62425263591?pr=180#step:9:414 |
tests/test_decompilers.py
Outdated
| deci.artifact_change_callbacks[Decompilation].append(on_decompilation_change) | ||
|
|
||
| # mock objects to simulate IDA's HexRays structures | ||
| class MockCfunc: |
There was a problem hiding this comment.
hehe I can tell this part was Claude written.
libbs/decompilers/ida/hooks.py
Outdated
| return | ||
|
|
||
| dec = Decompilation( | ||
| # only IDA support for now |
There was a problem hiding this comment.
A pointless comment. This code is already in IDA.
|
|
||
| deci.shutdown() | ||
|
|
||
| def test_ida_hook_decompilation_event(self): |
There was a problem hiding this comment.
High-level comments:
This is a bad way to test if a feature works. I understand the logic (or the AI's logic?) here, but it's too contrived. Why directly grab the HexraysHook, cause a fake trigger, and then observe if the new hook works?
Why not just get an IDA deci, do some action that forces a refresh (like renaming a variable or calling the refresh API), then seeing if your new hook gets hit. An example of a place I tested a callback implicitly:
libbs/tests/test_decompilers.py
Line 799 in 6959aba
tests/test_decompilers.py
Outdated
| func_addr = ida_deci.art_lifter.lift_addr(0x40071d) | ||
| dec = ida_deci.decompile(func_addr) | ||
| assert dec is not None, "Failed to decompile main" | ||
| ida_deci.decompilation_changed(dec) |
There was a problem hiding this comment.
Arn't you calling the event directly here? You should trigger it indirectly to prove that the callback works.
There was a problem hiding this comment.
function rename trigger
NishTheFish-dev
force-pushed
the
feat/new_dcmpchanged_callback
branch
from
5e2b99d to
c231021
Compare
…nsync/libbs into feat/new_dcmpchanged_callback
2 participants
Closes #176
Adds a new hook for IDA that detects when the decompilation has changed. Also includes a test case to verify this works correctly (now implemented!)