gh-105481: Generate the opcode lists in dis from data extracted from bytecodes.c#106758
gh-105481: Generate the opcode lists in dis from data extracted from bytecodes.c#106758iritkatriel merged 30 commits into
Conversation
… opcode.py with the functions in _opcode.py
Maybe in this case actually the new list is incorrect - all the opcodes with DEREF seem to belong to hasfree rather than haslocal: |
Sorry, something went wrong.
Ugh. Worse, now LOAD_CLOSURE is gone, it’s ambiguous. Who uses this? How? (Only dis.py?) |
Sorry, something went wrong.
LOAD_CLOSURE is just an alias (pseudo op) for LOAD_FAST. It's in the haslocal list in the old and new version, no change there. |
Sorry, something went wrong.
|
Now it's like this: Old: New: The "<148>" is because in opcode.py in main we have In the haslocal list, we see that the old list is missing the super instructions. |
Sorry, something went wrong.
|
For the other lists: Old: New: old is missing the Old: New: Old is missing Old: New: New has For hasarg, the old list contained a few that the new list does not:
|
Sorry, something went wrong.
|
It mostly looks right, except for |
Sorry, something went wrong.
My point is that LOAD_CLOSURE used to be separate because it was in the hasfree list. Now some LOAD_FAST instances refer to a local and others to a free variable. Example: The LOAD_FAST (a) at offset 26 used to be a LOAD_CLOSURE in 3.12 and before (and in 3.13 until LOAD_CLOSURE was made a pseudo-op). It's harmless, because we have I don't think |
Sorry, something went wrong.
Looks like a relic. In 3.11, 148 was LOAD_CLASSDEREF. That opcode no longer exists in 3.12 (it was removed in gh-103764 for PEP 695). So let's get rid of it. |
Sorry, something went wrong.
So we merge hasfree into haslocal and remove hasfree? Or leave it empty and soft deprecate it? |
Sorry, something went wrong.
Hm, I was just writing up a rationale for this when I realized that we should just keep |
Sorry, something went wrong.
|
Ok, then this PR is probably fine w.r.t. hasfree and haslocal. The only list in oplists now is hascompare, which contains only COMPARE_OP: It is used by dis to identify that its op (>=, ==, etc) needs to be displayed. |
Sorry, something went wrong.
Since there's only one opcode like this, maybe dis can just check whether it's that opcode? Or do you anticipate that in the future there will be multiple ones? (Were there ever?) |
Sorry, something went wrong.
It's just this one all the way back to 3.8. dis can stop using the list, but it's documented in the dis module so can we remove it? |
Sorry, something went wrong.
I guess deprecate it like hasjabs/hasjrel. |
Sorry, something went wrong.
…ode_metadata rather than via opcode
…ad of opcode for _specialized_instructions
|
I think I need to add in whatsnew that the dis module's opcode lists are no longer also defined in the opcode module. The opcode module is not documented, but if you look on GitHub you can see some places where people import it. |
Sorry, something went wrong.
Or maybe this is a breaking change that I can't make without deprecation? I'm not sure what the status of the opcode module is. I can put the lists back in opcode, and instead work to remove the dependency of the build on opcode. |
Sorry, something went wrong.
|
I have a feeling that opcode is actually a better place than dis for the official public API. And that's probably why people have been importing it despite its undocumented status. |
Sorry, something went wrong.
|
OK, I reverted the last few commits so we leave everything in opcode. We will rid the build of opcode in a future PR once the opcode.h file is generated from bytecodes.c. |
Sorry, something went wrong.
The generated lists are more correct than the (old) hard-coded ones, which were not maintained properly.
For example:
Old:
New: