From 0a8543f30be0f07e58b28445476cba259969a00a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sun, 9 Jul 2023 17:40:11 +0200 Subject: [PATCH] gh-105373: Deprecate PyObject_SelAttr(v, name, NULL) * Convert PyObject_DelAttr() and PyObject_DelAttrString() macros to functions. * Add PyObject_DelAttr() and PyObject_DelAttrString() functions to the stable ABI. * Replace PyObject_SetAttr(obj, name, NULL) with PyObject_DelAttr(obj, name). * weakref proxy_setattr() calls PyObject_DelAttr() if value is NULL. --- Doc/c-api/object.rst | 6 ++- Doc/data/stable_abi.dat | 2 + Doc/whatsnew/3.13.rst | 14 ++++++ Include/abstract.h | 6 +-- Include/object.h | 2 + Lib/test/test_stable_abi_ctypes.py | 2 + ...-07-09-17-50-34.gh-issue-105373.eYj3wa.rst | 7 +++ Misc/stable_abi.toml | 4 ++ Objects/object.c | 48 +++++++++++++++++-- Objects/weakrefobject.c | 8 +++- PC/python3dll.c | 2 + Python/bltinmodule.c | 3 +- Python/bytecodes.c | 2 +- Python/executor_cases.c.h | 2 +- Python/generated_cases.c.h | 2 +- 15 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-07-09-17-50-34.gh-issue-105373.eYj3wa.rst diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 22e7458013fb3db..1079907e6a020f8 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -97,8 +97,10 @@ 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. It will + become an error in Python 3.15. .. 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 a01fc3b34b6fe8b..63f0f140f56472b 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -778,6 +778,15 @@ 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. It will become an error in + Python 3.15. 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:`105373`.) + Removed ------- @@ -947,6 +956,11 @@ Pending Removal in Python 3.15 * :c:func:`PyWeakref_GetObject`: use :c:func:`PyWeakref_GetRef` instead. * :c:type:`!Py_UNICODE_WIDE` type: use ``wchar_t`` instead. * :c:type:`Py_UNICODE` type: use ``wchar_t`` instead. +* Calling ``PyObject_SetAttr(v, name, NULL)`` and + ``PyObject_SetAttrString(v, name, NULL)`` will become an error: + use ``PyObject_DelAttr(v, name)`` and ``PyObject_DelAttrString(v, name)`` + instead. + (Contributed by Victor Stinner in :gh:`105373`.) * Python initialization functions: * :c:func:`PySys_ResetWarnOptions`: clear :data:`sys.warnoptions` and 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-09-17-50-34.gh-issue-105373.eYj3wa.rst b/Misc/NEWS.d/next/C API/2023-07-09-17-50-34.gh-issue-105373.eYj3wa.rst new file mode 100644 index 000000000000000..4f3db8be8fc97ac --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-07-09-17-50-34.gh-issue-105373.eYj3wa.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. It will become an error in +Python 3.15. 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 bac3e79bb7c250e..0836f822b18aa3a 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 89b077ac4283459..59a915b69a2ef37 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 32efeb099d9b3e6..b8512e3c3955598 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 eb8b50e5c905d4c..d7630c2cca1fd6f 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"