I propose to move the PyFrameObject structure to the internal C API.


--


Between Python 3.10 and Python 3.11, the work on optimizing ceval.c modified deeply the PyFrameObject structure. Examples:

* The f_code member has been removed by in bpo-44032 by the commit b11a951f16f0603d98de24fee5c023df83ea552c.
* The f_frame member has been added in bpo-44590 by the commit ae0a2b756255629140efcbe57fc2e714f0267aa3.

Most members have been moved to a new PyFrameObject.f_frame member which has the type "struct _interpreter_frame*". Problem: this type is only part of the *internal* C API.

Moreover, accessing the few remaining members which "didn't change" became dangerous. For example, f_back can be NULL even if the frame has a previous frame: the PyFrame_GetBack() function *must* now be called. See bpo-46356 "[C API] Enforce usage of PyFrame_GetBack()".

Reading directly f_lineno was already dangerous since Python 2.3: the value is only valid if the value is greater than 0. It's way safer to use the clean PyFrame_GetLineNumber() API instead.

PyFrame_GetBack() was added to Python 3.9. You can use the pythoncapi_compat project to get this function on Python 3.8 and older:

=> https://pythoncapi-compat.readthedocs.io/

PyFrame_GetLineNumber() was added to the limited API in Python 3.10.

=> Documentation: https://docs.python.org/dev/c-api/reflection.html#c.PyFrame_GetBack


--


There *are* projects accessing directly PyFrameObject like the gevent project which sets the f_code member (moved to f_frame.f_code in Python 3.11). It's broken on Python 3.11:
https://bugs.python.org/issue40421#msg413719

Debuggers and profilers also want to read PyFrameObject directly. IMO for these *specific* use cases, using the *internal* C API is a legit use case and it's fine.

Moving PyFrameObject to the internal C API would clarify the situation. Currently, What's New in Python 3.11 documents the change this with warning:

"While the documentation notes that the fields of PyFrameObject are subject to change at any time, they have been stable for a long time and were used in several popular extensions. "


--


I'm mostly worried about Cython which still get and set many PyFrameObject members directly (ex: f_lasti, f_lineno, f_localsplus, f_trace), since there are no public functions for that.

=> https://bugs.python.org/issue40421#msg367550

Right now, I would suggest Cython to use the internal C API, and *later* consider adding new getter and setter functions. I don't think that we can solve all problems at once: it takes take to design clean API and use them in Cython.

Python 3.11 already broke Cython since most PyFrameObject members moved into the new "internal" PyFrameObject.f_frame API which requires using the internal C API to get "struct _interpreter_frame".

=> https://github.com/cython/cython/issues/4500


--


Using a frame using the *public* C API was and remains supported. Short example:
--
PyThreadState *tstate = PyThreadState_Get();
PyFrameObject* frame = PyThreadState_GetFrame(tstate);
int lineno = PyFrame_GetLineNumber(frame);
---

The PyFrameObject structure is opaque and members are not accessed directly: it's fine.

By the way, Include/cpython/ceval.h uses the "struct _interpreter_frame*" type whereas this type is part of the internal C API:

PyAPI_FUNC(PyObject *) _PyEval_EvalFrameDefault(PyThreadState *tstate, struct _interpreter_frame *f, int exc);

Maybe we should move this defintion to the internal C API pycore_ceval.h.

See also bpo-40421 "[C API] Add getter functions for PyFrameObject and maybe move PyFrameObject to the internal C API". I added getter functions in recent Python versions:

* PyFrame_GetBack(): Python 3.9
* PyFrame_GetCode(): Python 3.9
* PyFrame_GetLineNumber() added to the limited C API version 3.10

See also bpo-44800 "Code readability: rename InterpreterFrame to _Py_framedata".

See also bpo-45247: [C API] Add explicit support for Cython to the C API.

I marked my PR as a draft since this change is an incompatible change. Even if PyFrameObject structure is excluded from the limited C API and not documented, it's used by a few projects. I plan to check how this change impacts these projects before merging the change.

For example, test this change on:

* Cython: https://github.com/cython/cython/issues/4500
* gevent: https://github.com/gevent/gevent/issues/1867
* coverage uses f_lasti: https://bugs.python.org/issue40421#msg403814

See also the bpo-39947: "[C API] Make the PyThreadState structure opaque (move it to the internal C API)". Recently, I also added helper functions:

* PyThreadState_GetFrame(): Python 3.9 and limited C API version 3.10
* PyThreadState_GetID(): Python 3.9 and limited C API version 3.10
* PyThreadState_GetInterpreter(): Python 3.9 and limited C API version 3.10
* PyThreadState_EnterTracing(), PyThreadState_LeaveTracing(): Python 3.11

See also pending GH-29121 of bpo-39947: "Add PyThreadState_SetTrace() function".

So, this will break Cython and gevent, but (unlike the optimization work that broke f_code/f_frame) it won't provide any value to users?

I don't see how that's a good idea.

Petr Viktorin:
> So, this will break Cython and gevent,

This change doesn't break Cython and gevent: they are already broken.


> but (unlike the optimization work that broke f_code/f_frame) it won't provide any value to users?

The problem is that the C API changed silently: existing code which gets directly PyFrameObject.f_back still compiles successfully, but it will no longer work in some cases.

See bpo-46356 "[C API] Enforce usage of PyFrame_GetBack()" for more details.

The intent of moving the structure to the internal C API is to clarify its status: we provide no backward compatibility warranty, you are on our own if you use it.

It's also a way to promote the usage of the new clean public C API: it is now reliable, whereas accessing directly PyFrameObject members break at each Python version.

The internal C API cannot be used easily on purpose: you have to opt-in for this API by defining the Py_BUILD_CORE_MODULE macro and you need to use different #include. It's a way to enforce the usage of the clean public C API.

I haven't looked fully into this yet, but I *think* that Cython can get rid of most of the direct usages of PyFrameObject by switching to the new InterpreterFrame struct instead. It looks like the important fields have now been moved over to that.

That won't improve the situation regarding the usage of CPython internals, but it's probably worth keeping in mind before we start adding new API functions that work on frame objects.

Stefan Behnel:
> I haven't looked fully into this yet, but I *think* that Cython can get rid of most of the direct usages of PyFrameObject by switching to the new InterpreterFrame struct instead. It looks like the important fields have now been moved over to that.

InterpreterFrame is part of the internal C API. As I wrote, IMO it's fine for now that Cython uses the internal C API.

> That won't improve the situation regarding the usage of CPython internals, but it's probably worth keeping in mind before we start adding new API functions that work on frame objects.

Right.

My hope is also that making the structure internal should help to identify which members should be exposed with getter functions, or even setter functions (Mark would prefer to no add setter functions).

OK, looking at it more carefully, it makes sense to do the change.

> PyAPI_FUNC(PyObject *) _PyEval_EvalFrameDefault(PyThreadState *tstate, struct _interpreter_frame *f, int exc);

I created bpo-46850 "[C API] Move _PyEval_EvalFrameDefault() to the internal C API" for this issue.

New changeset 18b5dd68c6b616257ae243c0b6bb965ffc885a23 by Victor Stinner in branch 'main':
bpo-46836: Move PyFrameObject to pycore_frame.h (GH-31530)
https://github.com/python/cpython/commit/18b5dd68c6b616257ae243c0b6bb965ffc885a23

I announced the change on the capi-sig mailing list:
https://mail.python.org/archives/list/capi-sig@python.org/thread/RCT4SB5LY5UPRRRALEOHWEQHIXFNTHYF/

New changeset 87af12bff33b3e7546fa26158b7d8680ecb6ecec by Victor Stinner in branch 'main':
bpo-46836: Rename InterpreterFrame to _PyInterpreterFrame (GH-31583)
https://github.com/python/cpython/commit/87af12bff33b3e7546fa26158b7d8680ecb6ecec

Victor, can we please revert these changes? They broke Greenlet, a required dependency for three of our performance benchmarks:

https://github.com/python-greenlet/greenlet/issues/288#issuecomment-1055632607

I've already spent considerable effort contributing workarounds for other 3.11 breakages in Greenlet and coaxing them to put out a compatible release:

https://github.com/python-greenlet/greenlet/pull/280

These changes, however, just seem like needless breakage that are now also impacting our performance work.

I'm also very uncomfortable with the lack of review on these PRs. The most recent one (https://github.com/python/cpython/pull/31583) was open for less than 30 minutes before merging, from 6:57 to 7:22 am in my local time zone.

I plan to update Cython, greenlet and gevent for this change.

Draft PR for greenlet: https://github.com/python-greenlet/greenlet/pull/294

I made these changes close to the Python 3.11 alpha 6 release to be able to test "#if PY_VERSION_HEX < 0x30B00A6" to have code compatible with Python 3.11 alpha 5 and older.

Draft PR for Cython: https://github.com/cython/cython/pull/4671

Draft PR for gevent: https://github.com/gevent/gevent/pull/1872

> Draft PR for Cython: https://github.com/cython/cython/pull/4671

Notes on how Cython access PyFrameObject fields: https://bugs.python.org/issue40421#msg414314

New changeset b0f886d1bca499db1118a60b707923fa8e157073 by Victor Stinner in branch 'main':
bpo-46836: Add Doc/c-api/frame.rst (GH-32051)
https://github.com/python/cpython/commit/b0f886d1bca499db1118a60b707923fa8e157073