diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 22e7458013fb3db..a951dd575ad2425 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -85,9 +85,9 @@ Object Protocol return ``0`` on success. This is the equivalent of the Python statement ``o.attr_name = v``. - If *v* is ``NULL``, the attribute is deleted. This behaviour is deprecated - in favour of using :c:func:`PyObject_DelAttr`, but there are currently no - plans to remove it. + .. deprecated:: 3.13 + Calling ``PyObject_SetAttrString(o, attr_name, NULL)`` is deprecated: + ``PyObject_DelAttr(o, attr_name)`` must be used instead. .. c:function:: int PyObject_SetAttrString(PyObject *o, const char *attr_name, PyObject *v) @@ -97,8 +97,9 @@ Object Protocol return ``0`` on success. This is the equivalent of the Python statement ``o.attr_name = v``. - If *v* is ``NULL``, the attribute is deleted, but this feature is - deprecated in favour of using :c:func:`PyObject_DelAttrString`. + .. deprecated:: 3.13 + Calling ``PyObject_SetAttrString(o, attr_name, NULL)`` is deprecated: + ``PyObject_DelAttrString(o, attr_name)`` must be used instead. .. c:function:: int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value) diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 7fb002cd80369b6..9a61ddc39a353fd 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -490,6 +490,8 @@ function,PyObject_Calloc,3.7,, function,PyObject_CheckBuffer,3.11,, function,PyObject_ClearWeakRefs,3.2,, function,PyObject_CopyData,3.11,, +function,PyObject_DelAttr,3.13,, +function,PyObject_DelAttrString,3.13,, function,PyObject_DelItem,3.2,, function,PyObject_DelItemString,3.2,, function,PyObject_Dir,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 8fc809deca139b3..7cfa1c4b2690865 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -779,6 +779,14 @@ Deprecated :c:func:`PyWeakref_GetRef` on Python 3.12 and older. (Contributed by Victor Stinner in :gh:`105927`.) +* Deprecate ``PyObject_SetAttr(v, name, NULL)`` and + ``PyObject_SetAttrString(v, name, NULL)``: use ``PyObject_DelAttr(v, name)`` + and ``PyObject_DelAttrString(v, name)`` instead. When + :c:func:`PyObject_SetAttr` is called with a ``NULL`` value, it's unclear if + the caller wants to remove the attribute on purpose, or if the value is + ``NULL`` because of an error. Such API is error prone and should be avoided. + (Contributed by Victor Stinner in :gh:`106572`.) + Removed ------- diff --git a/Include/abstract.h b/Include/abstract.h index 016ace9bc89e964..c84d2c704e96051 100644 --- a/Include/abstract.h +++ b/Include/abstract.h @@ -80,7 +80,7 @@ extern "C" { This is the equivalent of the Python statement o.attr_name=v. */ -/* Implemented as a macro: +/* Implemented elsewhere: int PyObject_DelAttrString(PyObject *o, const char *attr_name); @@ -88,17 +88,15 @@ extern "C" { -1 on failure. This is the equivalent of the Python statement: del o.attr_name. */ -#define PyObject_DelAttrString(O, A) PyObject_SetAttrString((O), (A), NULL) -/* Implemented as a macro: +/* Implemented elsewhere: int PyObject_DelAttr(PyObject *o, PyObject *attr_name); Delete attribute named attr_name, for object o. Returns -1 on failure. This is the equivalent of the Python statement: del o.attr_name. */ -#define PyObject_DelAttr(O, A) PyObject_SetAttr((O), (A), NULL) /* Implemented elsewhere: diff --git a/Include/object.h b/Include/object.h index 3ef64511399c66f..dccab07e5f2c6fe 100644 --- a/Include/object.h +++ b/Include/object.h @@ -391,9 +391,11 @@ PyAPI_FUNC(PyObject *) PyObject_RichCompare(PyObject *, PyObject *, int); PyAPI_FUNC(int) PyObject_RichCompareBool(PyObject *, PyObject *, int); PyAPI_FUNC(PyObject *) PyObject_GetAttrString(PyObject *, const char *); PyAPI_FUNC(int) PyObject_SetAttrString(PyObject *, const char *, PyObject *); +PyAPI_FUNC(int) PyObject_DelAttrString(PyObject *v, const char *name); PyAPI_FUNC(int) PyObject_HasAttrString(PyObject *, const char *); PyAPI_FUNC(PyObject *) PyObject_GetAttr(PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_SetAttr(PyObject *, PyObject *, PyObject *); +PyAPI_FUNC(int) PyObject_DelAttr(PyObject *v, PyObject *name); PyAPI_FUNC(int) PyObject_HasAttr(PyObject *, PyObject *); PyAPI_FUNC(PyObject *) PyObject_SelfIter(PyObject *); PyAPI_FUNC(PyObject *) PyObject_GenericGetAttr(PyObject *, PyObject *); diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 038c978e7bbd02b..20bc2624c813611 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -509,6 +509,8 @@ def test_windows_feature_macros(self): "PyObject_CheckReadBuffer", "PyObject_ClearWeakRefs", "PyObject_CopyData", + "PyObject_DelAttr", + "PyObject_DelAttrString", "PyObject_DelItem", "PyObject_DelItemString", "PyObject_Dir", diff --git a/Misc/NEWS.d/next/C API/2023-07-10-13-39-56.gh-issue-106572.3ZQjCa.rst b/Misc/NEWS.d/next/C API/2023-07-10-13-39-56.gh-issue-106572.3ZQjCa.rst new file mode 100644 index 000000000000000..eea855d9dee4647 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-07-10-13-39-56.gh-issue-106572.3ZQjCa.rst @@ -0,0 +1,7 @@ +Deprecate ``PyObject_SetAttr(v, name, NULL)`` and +``PyObject_SetAttrString(v, name, NULL)``: use ``PyObject_DelAttr(v, name)`` +and ``PyObject_DelAttrString(v, name)`` instead. When +:c:func:`PyObject_SetAttr` is called with a ``NULL`` value, it's unclear if the +caller wants to remove the attribute on purpose, or if the value is ``NULL`` +because of an error. Such API is error prone and should be avoided. Patch by +Victor Stinner. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index bc7259f11816f3d..c61fedf8390e283 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2432,3 +2432,7 @@ added = '3.13' [function.PyWeakref_GetRef] added = '3.13' +[function.PyObject_DelAttr] + added = '3.13' +[function.PyObject_DelAttrString] + added = '3.13' diff --git a/Objects/object.c b/Objects/object.c index c27b13e9e0c31ac..b07baeee83ae1c9 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -926,8 +926,8 @@ PyObject_HasAttrString(PyObject *v, const char *name) return ok; } -int -PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w) +static int +object_set_attr_string(PyObject *v, const char *name, PyObject *w) { PyObject *s; int res; @@ -942,6 +942,26 @@ PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w) return res; } +int +PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w) +{ + if (w == NULL + && PyErr_WarnEx(PyExc_DeprecationWarning, + "PyObject_SetAttrString(v, name, NULL) is deprecated: " + "use PyObject_DelAttrString(v, name) instead", + 1) < 0) + { + return -1; + } + return object_set_attr_string(v, name, w); +} + +int +PyObject_DelAttrString(PyObject *v, const char *name) +{ + return object_set_attr_string(v, name, NULL); +} + int _PyObject_IsAbstract(PyObject *obj) { @@ -1136,8 +1156,8 @@ PyObject_HasAttr(PyObject *v, PyObject *name) return 1; } -int -PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) +static int +object_set_attr(PyObject *v, PyObject *name, PyObject *value) { PyTypeObject *tp = Py_TYPE(v); int err; @@ -1185,6 +1205,26 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) return -1; } +int +PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) +{ + if (value == NULL + && PyErr_WarnEx(PyExc_DeprecationWarning, + "PyObject_SetAttr(v, name, NULL) is deprecated: " + "use PyObject_DelAttr(v, name) instead", + 1) < 0) + { + return -1; + } + return object_set_attr(v, name, value); +} + +int +PyObject_DelAttr(PyObject *v, PyObject *name) +{ + return object_set_attr(v, name, NULL); +} + PyObject ** _PyObject_ComputedDictPointer(PyObject *obj) { diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index e9563729bf82ba6..913564ed9379f61 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -485,7 +485,13 @@ proxy_setattr(PyObject *proxy, PyObject *name, PyObject *value) if (!proxy_check_ref(obj)) { return -1; } - int res = PyObject_SetAttr(obj, name, value); + int res; + if (value != NULL) { + res = PyObject_SetAttr(obj, name, value); + } + else { + res = PyObject_DelAttr(obj, name); + } Py_DECREF(obj); return res; } diff --git a/PC/python3dll.c b/PC/python3dll.c index 65bdf326ffbc7f0..a7173911c7c1e84 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -447,6 +447,8 @@ EXPORT_FUNC(PyObject_CheckBuffer) EXPORT_FUNC(PyObject_CheckReadBuffer) EXPORT_FUNC(PyObject_ClearWeakRefs) EXPORT_FUNC(PyObject_CopyData) +EXPORT_FUNC(PyObject_DelAttr) +EXPORT_FUNC(PyObject_DelAttrString) EXPORT_FUNC(PyObject_DelItem) EXPORT_FUNC(PyObject_DelItemString) EXPORT_FUNC(PyObject_Dir) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 49efafc07f4245f..20a86fc6f583d3d 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1567,8 +1567,9 @@ static PyObject * builtin_delattr_impl(PyObject *module, PyObject *obj, PyObject *name) /*[clinic end generated code: output=85134bc58dff79fa input=164865623abe7216]*/ { - if (PyObject_SetAttr(obj, name, (PyObject *)NULL) != 0) + if (PyObject_DelAttr(obj, name) < 0) { return NULL; + } Py_RETURN_NONE; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 0848bbfd203ec49..88444293aa228b9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1240,7 +1240,7 @@ dummy_func( inst(DELETE_ATTR, (owner --)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - int err = PyObject_SetAttr(owner, name, (PyObject *)NULL); + int err = PyObject_DelAttr(owner, name); DECREF_INPUTS(); ERROR_IF(err, error); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 0ef752249ccb762..facaef83e64fa20 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1018,7 +1018,7 @@ PyObject *owner = stack_pointer[-1]; #line 1242 "Python/bytecodes.c" PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - int err = PyObject_SetAttr(owner, name, (PyObject *)NULL); + int err = PyObject_DelAttr(owner, name); #line 1023 "Python/executor_cases.c.h" Py_DECREF(owner); #line 1245 "Python/bytecodes.c" diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 11823cf9cd293c0..ecbf8ee59da1da1 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1696,7 +1696,7 @@ PyObject *owner = stack_pointer[-1]; #line 1242 "Python/bytecodes.c" PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - int err = PyObject_SetAttr(owner, name, (PyObject *)NULL); + int err = PyObject_DelAttr(owner, name); #line 1701 "Python/generated_cases.c.h" Py_DECREF(owner); #line 1245 "Python/bytecodes.c"