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

Fix confusion of enum _framestate with enum _frameowner #106156

Closed
wants to merge 1 commit into from

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Jun 27, 2023

The owner field of _PyInterpreterFrame is supposed to be a member of enum _frameowner, but FRAME_CLEARED is a member of enum _framestate. Add FRAME_OWNED_BY_ACCIDENT to enum _frameowner and use that instead. (It happens to have the same numerical value, but we shouldn’t rely on that anywhere.)

The owner field of _PyInterpreterFrame is supposed to be a member of
enum _frameowner, but FRAME_CLEARED is a member of enum _framestate.
Add FRAME_OWNED_BY_ACCIDENT to enum _frameowner and use that instead.
(It happens to have the same numerical value, but we shouldn’t rely on
that anywhere.)

Signed-off-by: Anders Kaseorg <[email protected]>
@@ -52,6 +52,7 @@ enum _frameowner {
FRAME_OWNED_BY_GENERATOR = 1,
FRAME_OWNED_BY_FRAME_OBJECT = 2,
FRAME_OWNED_BY_CSTACK = 3,
FRAME_OWNED_BY_ACCIDENT = 4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you should add a NEWS describing that you added a new value to the enum_frameowner enumeration in pycore_frame.h.

I think that making modifications to the interpreter core requires adding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not part of the public API; it’s an internal fix that has no direct relevance to Python users or extension authors.

However, the surrounding code has changed, so I’m closing this in favor of

@andersk andersk closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants