Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-91744: Add semi-stable C API tier#91789
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner 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.
Is PyInterpreterState_SetEvalFrameFunc() really usable in the semi-stable API? The second parameter type comes from the internal C API. There is nothing to inspect a _PyInterpreterFrame structure in the semi-stable API.
I don't know how debuggers use PyInterpreterState_SetEvalFrameFunc().
cc @markshannon
vstinner commented Apr 21, 2022
See also #91788 |
vstinner commented Apr 26, 2022
See also #91906 |
h-vetinari 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.
Some comments from taking a look out of curiosity
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Doc/c-api/stable.rst Outdated
| There are two tiers of API with different stability exepectations, | ||
| enabled by specific macros: | ||
| - :c:macro:`Py_USING_SEMI_STABLE_API` exposes API that may change | ||
| without deprecation warnings. | ||
| - :c:macro:`Py_LIMITED_API` exposes API that is compatible across | ||
| several minor releases. |
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.
exepectations -> expectations
One could also say: "there are twopublic tiers of the API" (emphasis just to show addition); underscored resp. Py_BUILD_CORE are arguably a tier as well...?
I've struck out the "two" because I think it would be clearer to expose the full hierarchy, à la:
| There are two tiers of API with different stability exepectations, | |
| enabled by specific macros: | |
| - :c:macro:`Py_USING_SEMI_STABLE_API` exposes API that may change | |
| without deprecation warnings. | |
| - :c:macro:`Py_LIMITED_API` exposes API that is compatible across | |
| several minor releases. | |
| There are three public tiers of the API with different stability | |
| expectations, two of which need to be enabled by specific macros: | |
| - :c:macro:`Py_LIMITED_API` exposes API that is compatible across | |
| several minor releases. | |
| - (no macro): default C-API covered by :pep:`387`. | |
| - :c:macro:`Py_USING_SEMI_STABLE_API` exposes API that may change | |
| without deprecation warnings between minor releases. |
I'd also suggest to lead with the most stable 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.
By “There are two tiers of API with different stability expectations” I wanted to present them as being additional, different from the default that's covered in the preceding paragraphs.
The order matches the detailed sections below, which would be somewhat tricky to reorder -- the one-paragraph semi-stable tier would feel lost if it was at the end.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Doc/c-api/code.rst Outdated
| use :c:func:`PyCode_NewEmpty` instead. | ||
| Since the definition of the bytecode changes often, calling | ||
| :c:func:`PyCode_New` directly can bind you to a precise Python version. |
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.
style: better "can directly" than "directly can" (or directly at the very end)
encukouApr 28, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
It's not “can directly bind you”, but “calling :c:func:PyCode_New directly” (as opposed to PyCode_Replace for example)
| @@ -0,0 +1,10 @@ | |||
| Defining :c:macro:`Py_USING_SEMI_STABLE_API` will expose "semi-stable API", | |||
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.
Perhaps it's worth considering to stay consistent with the spelling of "semi-stable" / "semistable". Currently the include folders are named like the latter (which I think looks good in a path), mentions in the docs like here often have the hyphen.
Even though the hyphen is grammatically slightly more correct, my personal preference would be without.
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.
Which also opens the question if the macro should be Py_USING_SEMI_STABLE_API or Py_USING_SEMISTABLE_API
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 still have hope that someone will come up with a better name. Python-dev is usually so good at bikeshedding!
I'll postpone renaming for now.
encukou commented Apr 28, 2022
Thank you for the comments! |
vstinner 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.
I expected the API and macro to mention "unstable" rather than "semi-stable" (Py_USING_SEMI_STABLE_API => Py_USING_UNSTABLE_API).
| PyObject*, PyObject*, PyObject*, int, PyObject*, | ||
| PyObject*); | ||
| /* same as struct above */ | ||
| /* See Include/semi-stable/code.h for PyCode_New* */ |
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'm not sure that this comment is useful.
| /* See Include/semi-stable/code.h for PyCode_New* */ | |
| /* See Include/unstable/code.h for PyCode_New* */ |
| // Expose unstable API when building Python | ||
| #ifdefPy_BUILD_CORE | ||
| #definePy_USING_UNSTABLE_API |
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.
| #definePy_USING_UNSTABLE_API | |
| #definePy_USING_UNSTABLE_API |
| #ifdefPy_USING_UNSTABLE_API | ||
| #define_Py_NEWLY_UNSTABLE(VERSION_UNUSED) | ||
| #else | ||
| #define_Py_NEWLY_UNSTABLE(VERSION) Py_DEPRECATED(VERSION) | ||
| #endif |
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.
| #ifdefPy_USING_UNSTABLE_API | |
| #define_Py_NEWLY_UNSTABLE(VERSION_UNUSED) | |
| #else | |
| #define_Py_NEWLY_UNSTABLE(VERSION) Py_DEPRECATED(VERSION) | |
| #endif | |
| #ifdefPy_USING_UNSTABLE_API | |
| #define_Py_NEWLY_UNSTABLE(VERSION_UNUSED) | |
| #else | |
| #define_Py_NEWLY_UNSTABLE(VERSION) Py_DEPRECATED(VERSION) | |
| #endif |
| PyObject*, PyObject*, PyObject*, PyObject*, | ||
| PyObject*, PyObject*, PyObject*, int, PyObject*, | ||
| PyObject*); | ||
| /* same as struct PyCodeObject */ |
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 suggest to move the comment before the function definition: "same parameters as PyCodeObject structure members".
| /* Frame evaluation API (added for PEP-523) */ | ||
| struct_PyInterpreterFrame; |
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.
Is this needed?
| - :c:macro:`Py_USING_SEMI_STABLE_API` exposes API that may change | ||
| without deprecation warnings. | ||
| - :c:macro:`Py_LIMITED_API` limits exposed API to API that is 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.
Is API repeated on purpose?
| - :c:macro:`Py_LIMITED_API` limits exposed API to API that is compatible | |
| - :c:macro:`Py_LIMITED_API` limits exposed API that is compatible |
| #definePy_UNSTABLE_CODE_H | ||
| #include"unstable/code.h" | ||
| #undef Py_UNSTABLE_CODE_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.
I would prefer to include it in top-level Include/code.h, as done in Include/pystate.h.
| - :c:func:`PyInterpreterState_GetEvalFrameFunc` | ||
| - :c:func:`PyInterpreterState_SetEvalFrameFunc` | ||
| - :c:type:`PyFrameEvalFunction` | ||
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.
Maybe mention also _PyFrameEvalFunction type which is renamed?
encukou commented Aug 10, 2022
I'm deferring the PEP. Thanks for the comments, I'll take them into account next time I try. |
This adds the semi-stable API tier witth a few initial functions/types. Adding more is left to another PR.