[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +30140
pull_request: https://github.com/python/cpython/pull/32051

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Guido van Rossum]

Change by Guido van Rossum :


--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Brandt Bucher]

Brandt Bucher  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Brandt Bucher]

Brandt Bucher  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Brandt Bucher]

Change by Brandt Bucher :


--
nosy: +brandtbucher

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +29706
pull_request: https://github.com/python/cpython/pull/31583

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests:  -29656

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Petr Viktorin]

Petr Viktorin  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Stefan Behnel]

Stefan Behnel  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[Petr Viktorin]

Petr Viktorin  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +29656
pull_request: https://github.com/python/cpython/pull/31532

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +Mark.Shannon, corona10, erlendaasland, petr.viktorin, scoder

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +29654
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/31530

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46836] [C API] Move PyFrameObject to the internal C API

[STINNER Victor]

New submission from STINNER Victor :

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.

--
components: C API
messages: 413795
nosy: vstinner
priority: normal
severity: normal
status: open
title: [C API] Move PyFrameObject to the internal C API
versions: Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com