-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-135075: Deprecate PyObject_SetAttr(obj, name, NULL) with exc #135082
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?
Conversation
Deprecate calling PyObject_SetAttr() and PyObject_SetAttrString() with NULL value and an exception set.
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 0, | ||
"calling PyObject_SetAttrString() with NULL value " | ||
"and an exception set is deprecated; " | ||
"use PyObject_DelAttrString() instead")) { |
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.
I think that in most cases, this is the wrong advice. PyObject_SetAttrString
will only be called with NULL
and an exception set when someone was misusing the C API, not when they were trying to delete an attribute.
Is it also possible to display the exception or chain it here?
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.
PyObject_SetAttr() does chain the exception, PyObject_SetAttrString() doesn't.
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.
An alternative is to modify PyObject_SetAttr() to fail with an exception (SystemError) if called with NULL value and an exception set.
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.
Failing with SystemError
makes more sense to me. There's already an exception present, so I don't think we'll be breaking code.
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.
I don't think we'll be breaking code.
It seems like currently, it's not possible to call PyObject_SetAttr() with an exception set: #135075 (comment) At least, for some setters.
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.
A SystemError
should be safe then.
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.
Ok, I created #136180 to raise SystemError if called with NULL value and an exception set.
Deprecate calling PyObject_SetAttr() and PyObject_SetAttrString() with NULL value and an exception set.
📚 Documentation preview 📚: https://cpython-previews--135082.org.readthedocs.build/