-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
gh-120642: Move private PyCode APIs to the internal C API #120643
Conversation
vstinner
commented
Jun 17, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Move _Py_BackoffCounter to pycore_backoff.h.
- Move _Py_CODEUNIT and related functions to pycore_code.h.
- Move Include/cpython/optimizer.h content to pycore_optimizer.h.
- Remove Include/cpython/optimizer.h.
- Issue: [C API] Move private PyCode and PyOptimizer API to the internal C API #120642
* Move _Py_BackoffCounter to pycore_backoff.h. * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h.
@markshannon @iritkatriel @encukou: What do you think of this change? The main effect is that 6 PyUnstable optimizer functions are moved to the internal C API (pycore_optimizer.h):
An alternative is to modify structures to add names to unions and structures, and then modify functions using it. |
We already went through and added union names to some of these, but I guess whichever developer keeps adding them didn't notice. Looking at the number of CI failures, possibly adding the names is going to be less disruptive! 😆 |
I just forgot to push a local change, I'm ashamed :-( |
test_cext and test_cppext are supposed to catch such issue, but we don't suppot "strict ISO C99" for now. We can update these tests once a decision will be taken: capi-workgroup/decisions#30 |
Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetOptimizerAPI() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer()
If possible, I would like to backport this change to 3.13 to solve a C99 compatibility issue in the Python C API. |
Since this is removing (unstable) C API functions added in 3.13 that everyone agrees we should remove, we should backport this to 3.13, yes. |
Python/optimizer.c
Outdated
@@ -195,7 +182,7 @@ _Py_SetOptimizer(PyInterpreterState *interp, _PyOptimizerObject *optimizer) | |||
} | |||
|
|||
int | |||
PyUnstable_SetOptimizer(_PyOptimizerObject *optimizer) | |||
_Py_SetOptimizerAPI(_PyOptimizerObject *optimizer) |
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.
Why the API
suffix?
Maybe Py_SetTier2Optimizer
(to distinguish it from the AST, bytecode or tier 1 optimizers)
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.
There is another _Py_SetOptimizer() function with a different API. This one adds more boilerplate code and calls the first one.
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 renamed the function to _Py_SetTier2Optimizer()
(I added an underscore prefix).
static inline _Py_CODEUNIT | ||
_py_make_codeunit(uint8_t opcode, uint8_t oparg) | ||
{ | ||
// No designated initialisers because of C++ compat |
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.
This is an internal header. Why does it need to be C++ compatible?
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.
No idea, I just moved code. For now, I prefer to leave it as it is. I can be reworked later.
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @vstinner, I could not cleanly backport this to
|
…on#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer() (cherry picked from commit 9e4a81f)
…on#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer() (cherry picked from commit 9e4a81f)
GH-121043 is a backport of this pull request to the 3.13 branch. |
…0643) (#121043) gh-120642: Move private PyCode APIs to the internal C API (#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer() (cherry picked from commit 9e4a81f)
…on#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer()
…on#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer()
FYI there are two mention of #define _PyCode_CODE(CO) _Py_RVALUE((_Py_CODEUNIT *)(CO)->co_code_adaptive)
#define _PyCode_NBYTES(CO) (Py_SIZE(CO) * (Py_ssize_t)sizeof(_Py_CODEUNIT)) This seems out of place, since those macros cannot work unless one also includes pycore_code.h. |
Oh correct, I wrote PR gh-121644 to move these 2 macros to internal C API. |
…on#120643) * Move _Py_CODEUNIT and related functions to pycore_code.h. * Move _Py_BackoffCounter to pycore_backoff.h. * Move Include/cpython/optimizer.h content to pycore_optimizer.h. * Remove Include/cpython/optimizer.h. * Remove PyUnstable_Replace_Executor(). Rename functions: * PyUnstable_GetExecutor() => _Py_GetExecutor() * PyUnstable_GetOptimizer() => _Py_GetOptimizer() * PyUnstable_SetOptimizer() => _Py_SetTier2Optimizer() * PyUnstable_Optimizer_NewCounter() => _PyOptimizer_NewCounter() * PyUnstable_Optimizer_NewUOpOptimizer() => _PyOptimizer_NewUOpOptimizer()