bpo-44800: rename _PyInterpreterFrame to _Py_frame #31987
Conversation
|
Significantly scaled back from the original proposal in #27525, as per the Discourse thread at https://discuss.python.org/t/proposal-rename-pyinterpreterframe-struct-as-py-framedata/14213 |
There was a problem hiding this comment.
Honestly, I'm not convinced by the value of this large rename changes. I renamed _PyInterpreterFrame and _PyCFrame just before Python 3.11 alpha6 release which moves the PyFrameObject structure to the pycore_frame.h internal header file. The plan is now to updated 3rd party code projects to make them compatible with these changes (compatible with Python 3.11 alpha6): https://bugs.python.org/issue46836#msg414283
If we change this internal C API once more time (Python 3.11 alpha7 or another release), it would require one more compatibility layer to the compatibility sandwich of projects like Cython, greenlet and gevent which currently require to access to these members until faster-cpython/ideas#309 is implemented.
IMO investing time on implementing faster-cpython/ideas#309 first would make more sense, since right now there are still too many 3rd party code which require to access PyFrameObject members.
Once 3rd party code will no longer consume this internal C API, we will have more freedom to change it without annoying users. As I wrote, core devs are directly impacted since these changes often break pyperformance: https://bugs.python.org/issue46836#msg414279
|
When you're done making the requested changes, leave the comment: |
|
I'm not saying that these changes should not be done. The current API can be enhanced as you do. The problem is really a practical problem impacting users right now. |
|
@vstinner If you're willing to volunteer to sync the PEP 558 working branch with CPython's main branch for me without these changes being made first, then I'll drop the proposal. I didn't file the issue out of idle whim, I filed it as a result of trying to do that merge and failing abjectly because I couldn't easily tell which conflicting snippets were using full frame objects and needed to add a level of indirection to get to the frame data object and which should now be using the lower level frame data struct and should hence change their type declarations. I know this code pretty well (both the old and the new versions), so if I can't follow it well enough to merge it with other code, that doesn't bode well for future maintainability. If you want to make the case that it's already too late to fix this and we're now stuck with it forever just because I broke my arm last November and didn't push the point until I'd recovered, then the Discourse thread is the place to do it: https://discuss.python.org/t/proposal-rename-pyinterpreterframe-struct-as-py-framedata/14213 |
|
PEP 558 is still a draft, no? Is it important to have an up to date implementation? There is also PEP 667. What's the status of these PEPs?
That's not what I wrote. Once Cython, greenlet and gevent will be able to use a sane public C API, changing PyFrameObject internals will impact less projects. |
|
And as I wrote on Discourse, if we don't change this internal layout for 3.11, we won't be able to change it in the future due to the problems it would create for bqckports. |
|
PEP 558 is supposed to be in 3.11. The 3 months lost to my broken arm means it is likely that it isn't going to make it, but the readability problem revealed by attempting a merge of that magnitude is a separate issue (hence why I filed it as one) |
|
I realised there's a macro hack we can use to keep code written for 3.11a6 compiling until a proper frame stack access API is available (presumably before 3.11b1). The old field and type names aren't used anywhere else, so the C preprocessor can be used to automatically translate them at compile time. |
Added a workaround that keeps 3.11a6 API compatibility by default
|
Thanks to C11, we're able to use Temporarily adding |
|
I'm going to scale back the proposal again to just the struct rename (keeping the old name around as a compatibility alias until there's a proper public API), and documenting the 7 frame data struct fields that have the "f_" prefix. |
|
Scaled back PR is at #32024 |
_PyInterpreterFrameto_Py_frameto help indicate that_Py_frameis an implementation optimisation for frame objects rather than a distinct concept
f_prefix from_Py_framefields that currently have it so presence orabsence of the prefix disambiguates
PyFrameObjectand_Py_framepointerswhen reading code snippets without their type declarations in code diffs
f_frametof_fdatato avoid the oddframe->f_frameconstruct_f_frame_datato_f_owned_fdatato avoid confusion withf_fdatacframeas a variable name to justframepycore_frame.hcovering the naming conventionshttps://bugs.python.org/issue44800