MatthieuDartiailh / bytecode

Python module to modify bytecode
https://bytecode.readthedocs.io/
MIT License
302 stars 38 forks source link

chore: handle LOAD_METHOD opcode in CPython 3.12 #129

Closed P403n1x87 closed 1 year ago

P403n1x87 commented 1 year ago

Allow handling the LOAD_METHOD pseudo-instruction in CPython 3.12 by converting it to the equivalent LOAD_ATTR instruction.

Resolves #128.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 60.00% and project coverage change: -0.09% :warning:

Comparison is base (e14a0a2) 95.60% compared to head (bdc6775) 95.51%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #129 +/- ## ========================================== - Coverage 95.60% 95.51% -0.09% ========================================== Files 6 6 Lines 2000 2005 +5 Branches 481 484 +3 ========================================== + Hits 1912 1915 +3 - Misses 53 54 +1 - Partials 35 36 +1 ``` | [Files Changed](https://app.codecov.io/gh/MatthieuDartiailh/bytecode/pull/129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthieu+Dartiailh) | Coverage Δ | | |---|---|---| | [src/bytecode/instr.py](https://app.codecov.io/gh/MatthieuDartiailh/bytecode/pull/129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthieu+Dartiailh#diff-c3JjL2J5dGVjb2RlL2luc3RyLnB5) | `97.17% <0.00%> (-0.51%)` | :arrow_down: | | [src/bytecode/concrete.py](https://app.codecov.io/gh/MatthieuDartiailh/bytecode/pull/129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthieu+Dartiailh#diff-c3JjL2J5dGVjb2RlL2NvbmNyZXRlLnB5) | `96.68% <100.00%> (+0.01%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MatthieuDartiailh commented 1 year ago

I am extremely hesitant to allow this kind of things. In particular I fear that "legalizing" the bytecode when converting to concrete may be too late since we need a CFG to compute the stack size and I would not bet on dis.stack_effect being accurate for pseudo instructions.

This and your request about TryBegin/End to me sound more like we could have a more abstract builder for bytecode that we could then safely compile before doing any computation on it. Food for though.

P403n1x87 commented 1 year ago

I've had this idea for a while now and finally had a chance to experiment with it https://github.com/P403n1x87/spasmlang. Perhaps these sorts of transformations can be handled at this higher level.

P403n1x87 commented 1 year ago

I'm closing this PR since my use cases are now covered by https://github.com/P403n1x87/spasmlang/pull/2.