Skip to content

bpo-46836: Add Doc/c-api/frame.rst #32051

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

Merged
merged 5 commits into from
Mar 23, 2022
Merged

bpo-46836: Add Doc/c-api/frame.rst #32051

merged 5 commits into from
Mar 23, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Mar 22, 2022

Reorganize the documentation of the PyFrameObject C API.

https://bugs.python.org/issue46836

Reorganize the documentation of the PyFrameObject C API.
@vstinner
Copy link
Member Author

My previous attempt to document PyFrameObject, in 2013: https://bugs.python.org/issue19431

@gvanrossum
Copy link
Member

@markshannon ^^

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it makes sense to more all the Frame API docs to one place.

I'd really like discourage extension code from accessing the internals of the frame, as the integrity of the VM relies on it in subtle ways.

.. versionchanged:: 3.11
The structure moved to the internal C API headers.

Public members:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be much safer to just say "don't access fields directly".

For example, f_lineno is not valid most of the time, we use it as an internal cache when tracing. f_back can be NULL even if the Python attribute is not None. Setting f_trace_lines or f_trace_opcodes may not do anything, as the change won't visible to the VM.

Copy link
Member Author

@vstinner vstinner Mar 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to clarify that these members are part of the Python API. My idea is just to explain that the Python API can be accessed in C as well.

Setting f_trace_lines or f_trace_opcodes may not do anything, as the change won't visible to the VM.

Well, it's technically possible to do that in pure Python. If there is a bug, it should be either documented or fixed. For example, disallow setting f_trace_lines and f_trace_opcodes.

Currently, f_trace_lines and f_trace_opcodes are not documented. IMO it's better to document them. If there are limits, we can document them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct fields cannot be safely accessed by C code. The limit is that you can't do it.
The Python attributes can be safely modified by PyObject_GetAttr etc.

The structure moved to the internal C API headers.

Public members of the Python API can be get and set with the
:c:func:`PyObject_GetAttrString` and :c:func:`PyObject_SetAttrString`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is the C API docs, any reference to Python attributes is likely to be misread as referring to C struct fields, so it is best not to list the Python attributes here.

Could you list the C API functions, PyFrame_GetLineNumber, etc, first?
I'll add docs to the new accessors here, as I add them.

Then add a comment that Python-level attributes can be accessed by PyObject_GetAttr[String] and PyObject_SetAttr[String] with an example:

E.g. PyObject_GetAttrString(frame, "f_lineno") is equivalent to PyFrame_GetLineNumber(frame)

@markshannon
Copy link
Member

@vstinner
I'd like to get at least a minimal version of this merged, so I have somewhere to put the docs for new PyFrame C-API functions.

@markshannon
Copy link
Member

Thanks.

@vstinner
Copy link
Member Author

@kumaraditya303: Can you please propose a PR for your changes?

Mark needed this PR for his PR #32055 and so I merged my PR.

I removed the whole section about "public members" since they are confusing. I will propose a second PR to find a way to document them.

@kumaraditya303
Copy link
Contributor

@kumaraditya303: Can you please propose a PR for your changes?

Sure, seems like github removed my comments as I was still adding suggestions and the PR got merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants