Revert "gh-98724: Fix Py_CLEAR() macro side effects" (#99737)

Revert "gh-98724: Fix Py_CLEAR() macro side effects (#99100)"

This reverts commit c03e05c2e7.

Doc/c-api/refcounting.rst

@@ -7,8 +7,8 @@
 Reference Counting
 ******************
 
-The functions and macros in this section are used for managing reference counts
+The macros in this section are used for managing reference counts of Python
-of Python objects.
+objects.
 
 
 .. c:function:: Py_ssize_t Py_REFCNT(PyObject *o)
@@ -129,11 +129,6 @@ of Python objects.
    It is a good idea to use this macro whenever decrementing the reference
    count of an object that might be traversed during garbage collection.
 
-   .. versionchanged:: 3.12
-      The macro argument is now only evaluated once. If the argument has side
-      effects, these are no longer duplicated.
-
-
 .. c:function:: void Py_IncRef(PyObject *o)
 
    Increment the reference count for object *o*. A function version of :c:func:`Py_XINCREF`.
@@ -144,40 +139,3 @@ of Python objects.
 
    Decrement the reference count for object *o*. A function version of :c:func:`Py_XDECREF`.
    It can be used for runtime dynamic embedding of Python.
-
-
-.. c:macro:: Py_SETREF(dst, src)
-
-   Macro safely decrementing the `dst` reference count and setting `dst` to
-   `src`.
-
-   As in case of :c:func:`Py_CLEAR`, "the obvious" code can be deadly::
-
-       Py_DECREF(dst);
-       dst = src;
-
-   The safe way is::
-
-        Py_SETREF(dst, src);
-
-   That arranges to set `dst` to `src` _before_ decrementing reference count of
-   *dst* old value, so that any code triggered as a side-effect of `dst`
-   getting torn down no longer believes `dst` points to a valid object.
-
-   .. versionadded:: 3.6
-
-   .. versionchanged:: 3.12
-      The macro arguments are now only evaluated once. If an argument has side
-      effects, these are no longer duplicated.
-
-
-.. c:macro:: Py_XSETREF(dst, src)
-
-   Variant of :c:macro:`Py_SETREF` macro that uses :c:func:`Py_XDECREF` instead
-   of :c:func:`Py_DECREF`.
-
-   .. versionadded:: 3.6
-
-   .. versionchanged:: 3.12
-      The macro arguments are now only evaluated once. If an argument has side
-      effects, these are no longer duplicated.

Doc/whatsnew/3.12.rst

@@ -822,11 +822,6 @@ Porting to Python 3.12
   :class:`bytes` type is accepted for bytes strings.
   (Contributed by Victor Stinner in :gh:`98393`.)
 
-* The :c:macro:`Py_CLEAR`, :c:macro:`Py_SETREF` and :c:macro:`Py_XSETREF`
-  macros now only evaluate their argument once. If the argument has side
-  effects, these side effects are no longer duplicated.
-  (Contributed by Victor Stinner in :gh:`98724`.)
-
 Deprecated
 ----------
 

Include/cpython/object.h

@@ -305,41 +305,37 @@ _PyObject_GenericSetAttrWithDict(PyObject *, PyObject *,
 
 PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
 
-/* Safely decref `dst` and set `dst` to `src`.
+/* Safely decref `op` and set `op` to `op2`.
  *
  * As in case of Py_CLEAR "the obvious" code can be deadly:
  *
- *     Py_DECREF(dst);
+ *     Py_DECREF(op);
- *     dst = src;
+ *     op = op2;
  *
  * The safe way is:
  *
- *      Py_SETREF(dst, src);
+ *      Py_SETREF(op, op2);
  *
- * That arranges to set `dst` to `src` _before_ decref'ing, so that any code
+ * That arranges to set `op` to `op2` _before_ decref'ing, so that any code
- * triggered as a side-effect of `dst` getting torn down no longer believes
+ * triggered as a side-effect of `op` getting torn down no longer believes
- * `dst` points to a valid object.
+ * `op` points to a valid object.
  *
- * gh-98724: Use the _tmp_dst_ptr variable to evaluate the 'dst' macro argument
+ * Py_XSETREF is a variant of Py_SETREF that uses Py_XDECREF instead of
- * exactly once, to prevent the duplication of side effects in this macro.
+ * Py_DECREF.
  */
-#define Py_SETREF(dst, src)                                     \
+
-    do {                                                        \
+#define Py_SETREF(op, op2)                      \
-        PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
+    do {                                        \
-        PyObject *_tmp_dst = (*_tmp_dst_ptr);                   \
+        PyObject *_py_tmp = _PyObject_CAST(op); \
-        *_tmp_dst_ptr = _PyObject_CAST(src);                    \
+        (op) = (op2);                           \
-        Py_DECREF(_tmp_dst);                                    \
+        Py_DECREF(_py_tmp);                     \
     } while (0)
 
-/* Py_XSETREF() is a variant of Py_SETREF() that uses Py_XDECREF() instead of
+#define Py_XSETREF(op, op2)                     \
- * Py_DECREF().
+    do {                                        \
- */
+        PyObject *_py_tmp = _PyObject_CAST(op); \
-#define Py_XSETREF(dst, src)                                    \
+        (op) = (op2);                           \
-    do {                                                        \
+        Py_XDECREF(_py_tmp);                    \
-        PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
-        PyObject *_tmp_dst = (*_tmp_dst_ptr);                   \
-        *_tmp_dst_ptr = _PyObject_CAST(src);                    \
-        Py_XDECREF(_tmp_dst);                                   \
     } while (0)
 
 

Include/object.h

@@ -598,21 +598,16 @@ static inline void Py_DECREF(PyObject *op)
  * one of those can't cause problems -- but in part that relies on that
  * Python integers aren't currently weakly referencable.  Best practice is
  * to use Py_CLEAR() even if you can't think of a reason for why you need to.
- *
- * gh-98724: Use the _py_tmp_ptr variable to evaluate the macro argument
- * exactly once, to prevent the duplication of side effects in this macro.
  */
-#define Py_CLEAR(op)                                          \
+#define Py_CLEAR(op)                            \
-    do {                                                      \
+    do {                                        \
-        PyObject **_py_tmp_ptr = _Py_CAST(PyObject**, &(op)); \
+        PyObject *_py_tmp = _PyObject_CAST(op); \
-        if (*_py_tmp_ptr != NULL) {                           \
+        if (_py_tmp != NULL) {                  \
-            PyObject* _py_tmp = (*_py_tmp_ptr);               \
+            (op) = NULL;                        \
-            *_py_tmp_ptr = NULL;                              \
+            Py_DECREF(_py_tmp);                 \
-            Py_DECREF(_py_tmp);                               \
+        }                                       \
-        }                                                     \
     } while (0)
 
-
 /* Function to use in case the object pointer can be NULL: */
 static inline void Py_XINCREF(PyObject *op)
 {

Modules/_testcapimodule.c

@@ -2589,91 +2589,6 @@ test_set_type_size(PyObject *self, PyObject *Py_UNUSED(ignored))
 }
 
 
-// Test Py_CLEAR() macro
-static PyObject*
-test_py_clear(PyObject *self, PyObject *Py_UNUSED(ignored))
-{
-    // simple case with a variable
-    PyObject *obj = PyList_New(0);
-    if (obj == NULL) {
-        return NULL;
-    }
-    Py_CLEAR(obj);
-    assert(obj == NULL);
-
-    // gh-98724: complex case, Py_CLEAR() argument has a side effect
-    PyObject* array[1];
-    array[0] = PyList_New(0);
-    if (array[0] == NULL) {
-        return NULL;
-    }
-
-    PyObject **p = array;
-    Py_CLEAR(*p++);
-    assert(array[0] == NULL);
-    assert(p == array + 1);
-
-    Py_RETURN_NONE;
-}
-
-
-// Test Py_SETREF() and Py_XSETREF() macros, similar to test_py_clear()
-static PyObject*
-test_py_setref(PyObject *self, PyObject *Py_UNUSED(ignored))
-{
-    // Py_SETREF() simple case with a variable
-    PyObject *obj = PyList_New(0);
-    if (obj == NULL) {
-        return NULL;
-    }
-    Py_SETREF(obj, NULL);
-    assert(obj == NULL);
-
-    // Py_XSETREF() simple case with a variable
-    PyObject *obj2 = PyList_New(0);
-    if (obj2 == NULL) {
-        return NULL;
-    }
-    Py_XSETREF(obj2, NULL);
-    assert(obj2 == NULL);
-    // test Py_XSETREF() when the argument is NULL
-    Py_XSETREF(obj2, NULL);
-    assert(obj2 == NULL);
-
-    // gh-98724: complex case, Py_SETREF() argument has a side effect
-    PyObject* array[1];
-    array[0] = PyList_New(0);
-    if (array[0] == NULL) {
-        return NULL;
-    }
-
-    PyObject **p = array;
-    Py_SETREF(*p++, NULL);
-    assert(array[0] == NULL);
-    assert(p == array + 1);
-
-    // gh-98724: complex case, Py_XSETREF() argument has a side effect
-    PyObject* array2[1];
-    array2[0] = PyList_New(0);
-    if (array2[0] == NULL) {
-        return NULL;
-    }
-
-    PyObject **p2 = array2;
-    Py_XSETREF(*p2++, NULL);
-    assert(array2[0] == NULL);
-    assert(p2 == array2 + 1);
-
-    // test Py_XSETREF() when the argument is NULL
-    p2 = array2;
-    Py_XSETREF(*p2++, NULL);
-    assert(array2[0] == NULL);
-    assert(p2 == array2 + 1);
-
-    Py_RETURN_NONE;
-}
-
-
 #define TEST_REFCOUNT() \
     do { \
         PyObject *obj = PyList_New(0); \
@@ -3337,8 +3252,6 @@ static PyMethodDef TestMethods[] = {
     {"pynumber_tobase", pynumber_tobase, METH_VARARGS},
     {"without_gc", without_gc, METH_O},
     {"test_set_type_size", test_set_type_size, METH_NOARGS},
-    {"test_py_clear", test_py_clear, METH_NOARGS},
-    {"test_py_setref", test_py_setref, METH_NOARGS},
     {"test_refcount_macros", test_refcount_macros, METH_NOARGS},
     {"test_refcount_funcs", test_refcount_funcs, METH_NOARGS},
     {"test_py_is_macros", test_py_is_macros, METH_NOARGS},