-
-
Notifications
You must be signed in to change notification settings - Fork 33k
gh-134786: Py_TPFLAGS_MANAGED_WEAKREF and Py_TPFLAGS_MANAGED_DICT must be used only if Py_TPFLAGS_HAVE_GC set #135863
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
base: main
Are you sure you want to change the base?
gh-134786: Py_TPFLAGS_MANAGED_WEAKREF and Py_TPFLAGS_MANAGED_DICT must be used only if Py_TPFLAGS_HAVE_GC set #135863
Conversation
…hecking of it presence
@markshannon Please take a look. |
As the person who reported #134786: I would suggest making the documentation more explicit and replacing should with must here, especially since the code now errors out if |
+1 Additionally, this PR puts this into a WhatsNew entry for 3.15. This is not a new thing, this is a documentation fix + fix for the runtime to not allow these invalid types to be created. So I think the added whatsnew is misleading. |
I'm not sure if we should support I think the current version needs some updates and clarification in the docs. First, though, I would like to get some feedback on the overall approach. Is this the right way to do after all? Please take a look if someone interested. |
@markshannon Please take a look. |
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 we should rename this PR?
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` and :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF` | ||
flags are set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too. |
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.
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` and :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF` | |
flags are set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too. | |
* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` or :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF` | |
flag is set then :c:macro:`Py_TPFLAGS_HAVE_GC` must be set too. |
Maybe it should be "or", not "and"?
I forgot to apply changes :( |
According to discussion
Py_TPFLAGS_MANAGED_WEAKREF
andPy_TPFLAGS_MANAGED_DICT
must be used only ifPy_TPFLAGS_HAVE_GC
is set.📚 Documentation preview 📚: https://cpython-previews--135863.org.readthedocs.build/