-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-142913: Add test case for interpreter generator w/ overridden opcodes #142911
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
base: main
Are you sure you want to change the base?
Conversation
e25709a to
40d5d70
Compare
85c6f7e to
506d4b6
Compare
80a2701 to
8fec3b0
Compare
abb5095 to
c5caac6
Compare
mpage
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| #define Py_BUILD_CORE_MODULE | ||
| #endif | ||
|
|
||
| #include "../../Python/ceval.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Third-party interpreter loops will still need to borrow the contents of Python/ceval.h and Python/ceval_macros.h, right? At some point it'd be nice if we could avoid borrowing these as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think depends on what you mean by "borrow". They'll need to include them (which should require a little extra work as the files aren't in Include). For ceval.h we could potentially do better but that's in-avoidable for ceval_macros.h as they are in fact (mostly) macros.
But this does put them in a place where rather than needing to do any actual copying they can build and re-use those source files. A re-generated interpreter could have some minor drift between patch versions of Python but because the bytecodes.c + ceval.h are all cohesive they should just either be missing or have extra bug fixes vs the patch version they're running on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm an extension author how would I set up my build so that I can include these files? I was assuming that since they aren't in Include I would probably need to copy them into my own source tree...
The interpreter generator has long supported giving it multiple bytecodes files where the latter files override opcodes defined in the earlier opcodes. This enables PEP 523 hook users to generate an interpreter loop using the existing opcodes and their own set of customizations. But there've been no test cases that validate this actually works.
This adds such a test case and fixes things up so this practically works. static functions used directly in the interpreter loop are moved into ceval.h. APIs which are used but not exported are marked as exported.
The generated test case is updated as mark of
mark regen-casesso it'll stay in sync as the bytecodes change.