Skip to content
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-131117: Update tp_finalize example to use PyErr_GetRaisedException #131118

Merged
merged 5 commits into from
Mar 19, 2025

Conversation

cmaloney
Copy link
Contributor

@cmaloney cmaloney commented Mar 11, 2025

The tp_finalize C API doc currently uses the older PyErr_Fetch + PyErr_Restore API to stash the current exception in its sample implementation. That API was deprecated in 3.12.

Update to point to the suggested replacement function PyErr_GetRaisedException which has a sample usage.


📚 Documentation preview 📚: https://cpython-previews--131118.org.readthedocs.build/

The tp_finalize C API doc used PyErr_Fetch and PyErr_Restore in its
example code. That API was deprecated in 3.12.

Update to point to the suggested replacement function
PyErr_GetRaisedException which has a sample usage.
@cmaloney cmaloney changed the title gh-131117: Use PyErr_GetRaisedException in tp_finalize doc gh-131117: Update tp_finalize example to use PyErr_GetRaisedException Mar 12, 2025
:c:member:`~PyTypeObject.tp_finalize` should not mutate the current exception status;
therefore, a recommended way to write a non-trivial finalizer is::
:c:member:`~PyTypeObject.tp_finalize` should not mutate the current exception
status. Non-trivial finalizers should use :c:func:`PyErr_GetRaisedException`,
Copy link
Member

Choose a reason for hiding this comment

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

"non-trivial" is not accurate. You can have a lot of code which doesn't raise exceptions, and short code which raises exceptions. I prefer to keep the previous sentence.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@cmaloney
Copy link
Contributor Author

CI / Github Actions workflow seems to have failed due to some restructuring (https://github.com/python/cpython/actions/runs/13864492893), looks like needs an update branch, should I do that?

@vstinner
Copy link
Member

I clicked on Update Branch. Let's see if it helps.

@vstinner vstinner merged commit a4832f6 into python:main Mar 19, 2025
24 checks passed
@vstinner vstinner added the needs backport to 3.13 bugs and security fixes label Mar 19, 2025
@miss-islington-app
Copy link

Thanks @cmaloney for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 19, 2025
…eption (pythonGH-131118)

The tp_finalize C API doc used PyErr_Fetch() and PyErr_Restore() in
its example code. That API was deprecated in 3.12.

Update to point to the suggested replacement function
PyErr_GetRaisedException() which has a sample usage.
(cherry picked from commit a4832f6)

Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Mar 19, 2025

GH-131476 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Mar 19, 2025
vstinner pushed a commit that referenced this pull request Mar 19, 2025
…ception (GH-131118) (#131476)

gh-131117: Update tp_finalize example to use PyErr_GetRaisedException (GH-131118)

The tp_finalize C API doc used PyErr_Fetch() and PyErr_Restore() in
its example code. That API was deprecated in 3.12.

Update to point to the suggested replacement function
PyErr_GetRaisedException() which has a sample usage.
(cherry picked from commit a4832f6)

Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com>
@cmaloney cmaloney deleted the tp_finalize_doc branch March 19, 2025 18:03
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
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants