-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
GH-96803: Add three C-API functions to make _PyInterpreterFrame less opaque for users of PEP 523. #96849
GH-96803: Add three C-API functions to make _PyInterpreterFrame less opaque for users of PEP 523. #96849
Conversation
markshannon
commented
Sep 15, 2022
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Expose _PyInterpreterFrame_GetLine in the private API #96803
… users of PEP 523.
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.
LGTM
@@ -0,0 +1,9 @@ | |||
Expose C-API functions to get the code object, lasti and line number from |
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.
Not sure if this needs a news entry since the functions are exposed technically on a private header.
Include/cpython/frameobject.h
Outdated
|
||
/* Returns the currently executing line number, or -1 if there is no line number. | ||
* Does not raise an exception. */ | ||
PyAPI_FUNC(int) _PyInterpreterFrame_GetLine(struct _PyInterpreterFrame *frame); |
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.
Using struct
here is confusing the Windows compiler
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.
You can add struct _PyInterpreterFrame;
somewhere to declare that "this structure exists" without having to declare the structure members (so it's declared as an opaque structure), to avoid such compiler warning. For example, at the top of this file.
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 added these functions to cpython/pyframe.h instead.
I don't know why frameobject.h is not included by Python.h: you have to include it explicitly, which goes against the advice of only including Python.h in C extensions. So I recently added pyframe.h which is included by Python.h. Maybe we should just move frameobject.h content into pyframe.h.
In Python 3.11, I moved multiple functions from frameobject.h to pyframe.h: 27b9894
Python/frame.c
Outdated
@@ -118,6 +118,20 @@ _PyFrame_Clear(_PyInterpreterFrame *frame) | |||
Py_DECREF(frame->f_code); | |||
} | |||
|
|||
/* Limited API functions */ |
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.
These functions are excluded from the limited API, they are declared in cpython/.
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.
What is the part of the API that isn't portable called? "unlimited", "non-limited"?
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.
The Limited API is supposed to be the most portable. The "not portable" API is called the "Public API".
For me, the reference documentation about that is now: https://devguide.python.org/developer-workflow/c-api/index.html#changing-python-s-c-api
See also: Include/README.rst.
Misc/NEWS.d/next/C API/2022-09-15-15-21-34.gh-issue-96803.ynBKIS.rst
Outdated
Show resolved
Hide resolved
* implementing custom frame evaluators with PEP 523. */ | ||
|
||
/* Returns the code object of the frame. | ||
* Does not raise an exception. */ |
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.
Somewhere I saw the expression: "This function cannot fail" but I can no longer find it in the doc.
…IS.rst Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
(I'm working my way through some PRs which have been approved and are labeled "awaiting merge", hence my seemingly bolt from the blue comment. Why? Read here.) This PR has been idle for awhile. Can it be merged or is there more to do? |
Is this supposed to be in 3.12? |
We might as well. I'm unsure how people use PEP 523, but this should help. I'll add the |
67a5450
to
7fbece8
Compare
* main: pythongh-99113: Add PyInterpreterConfig.own_gil (pythongh-104204) pythongh-104146: Remove unused var 'parser_body_declarations' from clinic.py (python#104214) pythongh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (pythongh-104205) pythongh-104108: Add the Py_mod_multiple_interpreters Module Def Slot (pythongh-104148) pythongh-99113: Share the GIL via PyInterpreterState.ceval.gil (pythongh-104203) pythonGH-100479: Add `pathlib.PurePath.with_segments()` (pythonGH-103975) pythongh-69152: Add _proxy_response_headers attribute to HTTPConnection (python#26152) pythongh-103533: Use PEP 669 APIs for cprofile (pythonGH-103534) pythonGH-96803: Add three C-API functions to make _PyInterpreterFrame less opaque for users of PEP 523. (pythonGH-96849)
… less opaque for users of PEP 523. (pythonGH-96849)