This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Code readability: rename InterpreterFrame to `_Py_framedata`
Type: enhancement Stage: patch review
Components: Versions: Python 3.11
process
Status: open Resolution:
Dependencies: 44590 Superseder:
Assigned To: ncoghlan Nosy List: Mark.Shannon, brandtbucher, ncoghlan, pablogsal, vstinner
Priority: normal Keywords: patch

Created on 2021-08-01 01:48 by ncoghlan, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 27525 closed ncoghlan, 2021-08-01 08:30
PR 3640 ncoghlan, 2021-08-21 13:31
PR 31987 closed ncoghlan, 2022-03-19 11:21
PR 32024 closed ncoghlan, 2022-03-21 10:58
PR 32281 merged ncoghlan, 2022-04-03 05:23
PR 32301 merged Mark.Shannon, 2022-04-04 09:57
Messages (10)
msg398675 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-01 01:48
When merging the bpo-44590 changes into my PEP 558 implementation branch, I found it very hard to follow when the code was referring to the new interpreter frames rather than the existing Python frame objects that were historically used for both execution and introspection.

The "interpreter frame" name was also a little confusing, since the introspection frames are still associated with a specific interpreter, they're just not required for code execution anymore, only for code introspection APIs that call for a full Python object.

So, inspired by the "gi_xframe" (etc) attributes added in https://github.com/python/cpython/pull/27077, I'm proposing the following internal refactoring:

* Rename "pycore_frame.h" to "pycore_xframe.h"
* Rename the _interpreter_frame struct to _execution_frame
* Rename the type from InterpreterFrame to ExecFrame
* Use "xf_" rather than "f_" as the struct field prefix on execution frames
* Use "xframe" and "xf" rather than "frame" and "f" for execution frame variables
* Consistently use _PyExecFrame as the access function prefix, rather than a confusing mixture of _PyFrame and _PyInterpreterFrame
* Rename _PyThreadState_PushFrame to _PyThreadState_PushExecFrame
* Rename _PyThreadState_PopFrame to _PyThreadState_PopExecFrame
msg398681 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-01 05:01
As a side effect of working on this, I noticed some changes that are needed to adapt the GDB integration hooks to the new frame state layout.
msg398690 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-01 08:48
PR for this proposed refactoring is now up, with a review requested from Mark: https://github.com/python/cpython/pull/27525/

The PR mostly follows what I originally posted, except that I went with _Py_execution_frame and _PyExecFrame for the struct and typedef names, since these are present in a public header.

The outdated GDB hooks I found were in the gdbinit example file, rather than the actual libpython.py file that test_gdb covers. I suspect the gdbinit example will need further adjustments to actually work with Python 3.11, so rather than fully updating the implementation dependent pieces, I just added a comment suggesting it be checked after the interpreter optimisation development for 3.11 settles down.
msg399381 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-11 10:25
Mark raised some valid concerns with the proposed naming convention over on the PR:

* the proposed names make it sound like there are genuinely two kinds of frame, when the actual relationship is between a frame's data storage and a Python object providing an interface to that storage
* _PyExecFrame looks like an actual Python type name, so we probably want something more like "Py_buffer" (where the lowercase name is intended to indicate that the struct isn't a full Python type)

Mark offered "activation record" as a possible name, but I'm going to see how "_Py_framedata" looks first (with "fdata" as the abbreviated form, since "fd" is so frequently used to mean "file descriptor")

I'm also going to see how the PR looks if both the frame and frame data struct keep their existing field prefixes - it may be that changing variable names will be enough to avoid ambiguity, in which case leaving the field names alone genuinely reduces code churn.
msg399581 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-14 05:10
PR has been updated with a new API proposal prompted by Mark's review comments on the original proposal.

* Rename "pycore_frame.h" to "pycore_framedata.h"
* Rename the _interpreter_frame struct to _Py_execution_frame
* Rename the type from InterpreterFrame to _Py_framedata
* Rather than 6 fields with no prefix, and 6 fields with the "f_" prefix, _Py_framedata fields now consistently have no prefix (globals, builtins, locals, code, lasti, and stack are affected by this)
* Use "fdata" rather than a mixture of "frame" and "f" for frame data variables
* Generators and coroutines use ``gi_fdata`` (etc) rather than ``gi_xframe`` as their field name
* Frame objects use ``f_fdata`` rather than ``f_frame`` as their field name
* Thread states use ``fdata`` rather than ``frame`` as their field name
* Consistently use _Py_frameobject as the access function prefix, rather than a confusing mixture of _PyFrame and _PyInterpreterFrame
* Leave the name of _PyThreadState_PushFrame alone
* Leave the name of _PyThreadState_PopFrame alone
msg399582 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-08-14 05:13
From a naming convention perspective, the code comments and NEWS entry in the PR now refer to "full frame objects" (``PyFrameObject``) and "frame data storage structs" (``_Py_framedata``) to avoid giving the misleading impression that introspection and execution frames are different Python types.
msg414446 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-03-03 15:15
Oh. I didn't know this issue. I recently made changes around PyFrameObject:

* Move PyFrameObject to the internal C API (see bpo-46836 for the rationale)
* Rename CFrame to _PyCFrame
* Rename InterpreterFrame to _PyInterpreterFrame

I prepared PRs for Cython, greenlet and gevent to use the internal C API pycore_frame.h to get the PyFrameObject structure:

https://bugs.python.org/issue46836#msg414283

For the short term, these projects should use the internal C API. But I sent a call to add getter and setter functions:

https://mail.python.org/archives/list/capi-sig@python.org/thread/RCT4SB5LY5UPRRRALEOHWEQHIXFNTHYF/

If possible, it would be great to have a public C API so these projects don't use the internal C API at all, that's being discussed at:

* https://github.com/faster-cpython/ideas/issues/309
* https://bugs.python.org/issue40421

In terms of backward compatibility, since PyFrameObject is now part of the internal C API, we can break things. In practice... it's better to not break 3rd party code too often. See for example Brandt Bucher who is directly impacted by these changes:

https://bugs.python.org/issue46836#msg414279
msg416610 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2022-04-03 05:27
Core dev forum thread: https://discuss.python.org/t/proposal-rename-pyinterpreterframe-struct-as-py-framedata/14213

The conclusion from the forum thread and associated PRs was that any of the further renaming proposed didn't provide sufficient additional clarity to justify further code churn in a semi public API.

 As a result, the final PR just documents the status quo in the internal C API frame header (since the conventions arising from the code-churn-minimising migration to a split data structure is hard to infer just from reading the code): https://github.com/python/cpython/pull/32281
msg416613 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2022-04-03 06:56
New changeset 124227c95f310d2ecd4b567271ab1919fc7000cb by Nick Coghlan in branch 'main':
bpo-44800: Document internal frame naming conventions (GH-32281)
https://github.com/python/cpython/commit/124227c95f310d2ecd4b567271ab1919fc7000cb
msg416671 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2022-04-04 14:09
New changeset 8a349eb30b54bab9a7146fc10e3379c3cacaa19e by Mark Shannon in branch 'main':
Revert "bpo-44800: Document internal frame naming conventions (GH-32281)" (#32301)
https://github.com/python/cpython/commit/8a349eb30b54bab9a7146fc10e3379c3cacaa19e
History
Date User Action Args
2022-04-11 14:59:48adminsetgithub: 88963
2022-04-04 14:09:53Mark.Shannonsetmessages: + msg416671
2022-04-04 09:57:18Mark.Shannonsetpull_requests: + pull_request30363
2022-04-03 06:56:04ncoghlansetmessages: + msg416613
2022-04-03 05:27:28ncoghlansetmessages: + msg416610
2022-04-03 05:23:03ncoghlansetpull_requests: + pull_request30342
2022-03-21 10:58:13ncoghlansetpull_requests: + pull_request30113
2022-03-19 11:21:26ncoghlansetpull_requests: + pull_request30078
2022-03-03 20:20:47brandtbuchersetnosy: + brandtbucher
2022-03-03 15:15:22vstinnersetnosy: + vstinner
messages: + msg414446
2021-08-21 13:31:00ncoghlansetpull_requests: + pull_request26326
2021-08-14 05:13:26ncoghlansetmessages: + msg399582
2021-08-14 05:10:43ncoghlansetmessages: + msg399581
title: Code readability: rename interpreter frames to execution frames -> Code readability: rename InterpreterFrame to `_Py_framedata`
2021-08-11 10:25:50ncoghlansetmessages: + msg399381
2021-08-01 08:48:35ncoghlansetmessages: + msg398690
2021-08-01 08:30:36ncoghlansetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request26040
2021-08-01 05:01:39ncoghlansetnosy: + pablogsal
messages: + msg398681
2021-08-01 01:50:29ncoghlansetassignee: ncoghlan
2021-08-01 01:48:47ncoghlansetdependencies: + Create frame objects lazily when needed
2021-08-01 01:48:31ncoghlancreate