Skip to content

Commit

Permalink
gh-106572: Deprecate PyObject_SetAttr(v, name, NULL)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
vstinner committed Jul 10, 2023
1 parent 51ea664 commit d414fec
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 18 deletions.
11 changes: 6 additions & 5 deletions Doc/c-api/object.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions Doc/data/stable_abi.dat

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------

Expand Down
6 changes: 2 additions & 4 deletions Include/abstract.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,23 @@ 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);
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_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:
Expand Down
2 changes: 2 additions & 0 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/test_stable_abi_ctypes.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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.
4 changes: 4 additions & 0 deletions Misc/stable_abi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
48 changes: 44 additions & 4 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
8 changes: 7 additions & 1 deletion Objects/weakrefobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions PC/python3dll.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Python/bltinmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d414fec

Please sign in to comment.