gh-106572: Deprecate PyObject_SetAttr(v, name, NULL)

If the value is NULL, PyObject_SetAttr() and PyObject_SetAttrString()
emit a DeprecationWarning in Python Development Mode or if Python is
built in debug mode.

weakref proxy_setattr() calls PyObject_DelAttr() if value is NULL.

Author: vstinner

Doc/c-api/object.rst

@@ -122,9 +122,13 @@ 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.
+   If *v* is ``NULL``, emit a :exc:`DeprecationWarning` in :ref:`Python
+   Development Mode <devmode>` or if :ref:`Python is built in debug mode
+   <debug-build>`.
+
+   .. 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)
@@ -134,8 +138,13 @@ 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`.
+   If *v* is ``NULL``, emit a :exc:`DeprecationWarning` in :ref:`Python
+   Development Mode <devmode>` or if :ref:`Python is built in debug mode
+   <debug-build>`.
+
+   .. 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)

Doc/whatsnew/3.13.rst

@@ -795,6 +795,17 @@ Deprecated
   :c:func:`PyWeakref_GetRef` on Python 3.12 and older.
   (Contributed by Victor Stinner in :gh:`105927`.)
 
+* Deprecate :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` to
+  remove an attribute (if *value* is ``NULL``): ``PyObject_DelAttr(v, name)``
+  and ``PyObject_DelAttrString(v, name)`` must be used instead. If the
+  development mode is enabled or if Python is built in debug mode, these
+  deprecated functions now emit a :exc:`DeprecationWarning` if *value* is
+  ``NULL``.  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
 -------
 

Lib/test/test_capi/test_misc.py

@@ -1115,6 +1115,24 @@ class Data(_testcapi.ObjExtraData):
         del d.extra
         self.assertIsNone(d.extra)
 
+    def test_setattr(self):
+        class MyType:
+            pass
+
+        PyObject_SetAttr = _testcapi.PyObject_SetAttr
+
+        obj = MyType()
+        PyObject_SetAttr(obj, "attr", 123)
+        self.assertEqual(obj.attr, 123)
+        # PyObject_SetAttr(obj, name, NULL) emits a DeprecationWarning
+        # in the Python Development Mode or if Python is built in debug mode
+        if support.Py_DEBUG or sys.flags.dev_mode:
+            with self.assertWarns(DeprecationWarning):
+                PyObject_SetAttr(obj, "attr")
+        else:
+            PyObject_SetAttr(obj, "attr")
+        self.assertFalse(hasattr(obj, "attr"))
+
 
 @requires_limited_api
 class TestHeapTypeRelative(unittest.TestCase):

Misc/NEWS.d/next/C API/2023-07-10-13-39-56.gh-issue-106572.3ZQjCa.rst

@@ -0,0 +1,9 @@
+Deprecate :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` to
+remove an attribute (if *value* is ``NULL``): ``PyObject_DelAttr(v, name)`` and
+``PyObject_DelAttrString(v, name)`` must be used instead. If the development
+mode is enabled or if Python is built in debug mode, these deprecated functions
+now emit a :exc:`DeprecationWarning` if *value* is ``NULL``.  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.

Modules/_testcapimodule.c

@@ -3464,6 +3464,20 @@ test_weakref_capi(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
 }
 
 
+static PyObject *
+test_pyobject_setattr(PyObject *Py_UNUSED(module), PyObject *args)
+{
+    PyObject *obj, *name, *value = NULL;
+    if (!PyArg_ParseTuple(args, "OO|O", &obj, &name, &value)) {
+        return NULL;
+    }
+    if (PyObject_SetAttr(obj, name, value) < 0) {
+        return NULL;
+    }
+    Py_RETURN_NONE;
+}
+
+
 static PyMethodDef TestMethods[] = {
     {"set_errno",               set_errno,                       METH_VARARGS},
     {"test_config",             test_config,                     METH_NOARGS},
@@ -3609,6 +3623,7 @@ static PyMethodDef TestMethods[] = {
     {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL},
     {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS},
     {"test_weakref_capi", test_weakref_capi, METH_NOARGS},
+    {"PyObject_SetAttr", test_pyobject_setattr, METH_VARARGS},
     {NULL, NULL} /* sentinel */
 };
 

Objects/object.c

@@ -926,8 +926,11 @@ PyObject_HasAttrString(PyObject *v, const char *name)
     return ok;
 }
 
-int
-PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w)
+// Forward declaration
+static int object_set_attr(PyObject *v, PyObject *name, PyObject *value);
+
+static int
+object_set_attr_string(PyObject *v, const char *name, PyObject *w)
 {
     PyObject *s;
     int res;
@@ -937,15 +940,32 @@ PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w)
     s = PyUnicode_InternFromString(name);
     if (s == NULL)
         return -1;
-    res = PyObject_SetAttr(v, s, w);
+    res = object_set_attr(v, s, w);
     Py_XDECREF(s);
     return res;
 }
 
+int
+PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w)
+{
+    if (w == NULL
+#ifndef Py_DEBUG
+        && _Py_GetConfig()->dev_mode
+#endif
+        && 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 PyObject_SetAttrString(v, name, NULL);
+    return object_set_attr_string(v, name, NULL);
 }
 
 int
@@ -1156,8 +1176,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;
@@ -1205,10 +1225,27 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
     return -1;
 }
 
+int
+PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
+{
+    if (value == NULL
+#ifndef Py_DEBUG
+        && _Py_GetConfig()->dev_mode
+#endif
+        && 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 PyObject_SetAttr(v, name, NULL);
+    return object_set_attr(v, name, NULL);
 }
 
 PyObject **

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;
 }