8000 Revert "gh-98724: Fix Py_CLEAR() macro side effects (#99100) (#99288)" · python/cpython@f728554 · GitHub
[go: up one dir, main page]
Skip to content

Commit f728554

Browse files
vstinnervstinner
authored
Revert "gh-98724: Fix Py_CLEAR() macro side effects (#99100) (#99288)"
This reverts commit 1082890.
1 parent bbac9a8 commit f728554

File tree

4 files changed

+27
-126
lines changed

4 files changed

+27
-126
lines changed

‎Include/cpython/object.h

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -309,41 +309,37 @@ _PyObject_GenericSetAttrWithDict(PyObject *, PyObject *,
309309

310310
PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
311311

312-
/* Safely decref `dst` and set `dst` to `src`.
312+
/* Safely decref `op` and set `op` to `op2`.
313313
*
314314
* As in case of Py_CLEAR "the obvious" code can be deadly:
315315
*
316-
* Py_DECREF(dst);
317-
* dst = src;
316+
* Py_DECREF(op);
317+
* op = op2;
318318
*
319319
* The safe way is:
320320
*
321-
* Py_SETREF(dst, src);
321+
* Py_SETREF(op, op2);
322322
*
323-
* That arranges to set `dst` to `src` _before_ decref'ing, so that any code
324-
* triggered as a side-effect of `dst` getting torn down no longer believes
325-
* `dst` points to a valid object.
323+
* That arranges to set `op` to `op2` _before_ decref'ing, so that any code
324+
* triggered as a side-effect of `op` getting torn down no longer believes
325+
* `op` points to a valid object.
326326
*
327-
* gh-98724: Use the _tmp_dst_ptr variable to evaluate the 'dst' macro argument
328-
* exactly once, to prevent the duplication of side effects in this macro.
327+
* Py_XSETREF is a variant of Py_SETREF that uses Py_XDECREF instead of
328+
* Py_DECREF.
329329
*/
330-
#define Py_SETREF(dst, src) \
331-
do { \
332-
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
333-
PyObject *_tmp_dst = (*_tmp_dst_ptr); \
334-
*_tmp_dst_ptr = _PyObject_CAST(src); \
335-
Py_DECREF(_tmp_dst); \
330+
331+
#define Py_SETREF(op, op2) \
332+
do { \
333+
PyObject *_py_tmp = _PyObject_CAST(op); \
334+
(op) = (op2); \
335+
Py_DECREF(_py_tmp); \
336336
} while (0)
337337

338-
/* Py_XSETREF() is a variant of Py_SETREF() that uses Py_XDECREF() instead of
339-
* Py_DECREF().
340-
*/
341-
#define Py_XSETREF(dst, src) \
342-
do { \
343-
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
344-
PyObject *_tmp_dst = (*_tmp_dst_ptr); \
345-
*_tmp_dst_ptr = _PyObject_CAST(src); \
346-
Py_XDECREF(_tmp_dst); \
338+
#define Py_XSETREF(op, op2) \
339+
do { \
340+
PyObject *_py_tmp = _PyObject_CAST(op); \
341+
(op) = (op2); \
342+
Py_XDECREF(_py_tmp); \
347343
} while (0)
348344

349345

‎Include/object.h

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -575,21 +575,16 @@ static inline void Py_DECREF(PyObject *op)
575575
* one of those can't cause problems -- but in part that relies on that
576576
* Python integers aren't currently weakly referencable. Best practice is
577577
* to use Py_CLEAR() even if you can't think of a reason for why you need to.
578-
*
579-
* gh-98724: Use the _py_tmp_ptr variable to evaluate the macro argument
580-
* exactly once, to prevent the duplication of side effects in this macro.
581578
*/
582-
#define Py_CLEAR(op) \
583-
do { \
584-
PyObject **_py_tmp_ptr = _Py_CAST(PyObject**, &(op)); \
585-
if (*_py_tmp_ptr != NULL) { \
586-
PyObject* _py_tmp = (*_py_tmp_ptr); \
587-
*_py_tmp_ptr = NULL; \
588-
Py_DECREF(_py_tmp); \
589-
} \
579+
#define Py_CLEAR(op) \
580+
do { \
581+
PyObject *_py_tmp = _PyObject_CAST(op); \
582+
if (_py_tmp != NULL) { \
583+
(op) = NULL; \
584+
Py_DECREF(_py_tmp); \
585+
} \
590586
} while (0)
591587

592-
593588
/* Function to use in case the object pointer can be NULL: */
594589
static inline void Py_XINCREF(PyObject *op)
595590
{

‎Misc/NEWS.d/next/C API/2022-11-04-16-13-35.gh-issue-98724.p0urWO.rst

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎Modules/_testcapimodule.c

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -5684,91 +5684,6 @@ test_set_type_size(PyObject *self, PyObject *Py_UNUSED(ignored))
56845684
}
56855685

56865686

5687-
// Test Py_CLEAR() macro
5688-
static PyObject*
5689-
test_py_clear(PyObject *self, PyObject *Py_UNUSED(ignored))
5690-
{
5691-
// simple case with a variable
5692-
PyObject *obj = PyList_New(0);
5693-
if (obj == NULL) {
5694-
return NULL;
5695-
}
5696-
Py_CLEAR(obj);
5697-
assert(obj == NULL);
5698-
5699-
// gh-98724: complex case, Py_CLEAR() argument has a side effect
5700-
PyObject* array[1];
5701-
array[0] = PyList_New(0);
5702-
if (array[0] == NULL) {
5703-
return NULL;
5704-
}
5705-
5706-
PyObject **p = array;
5707-
Py_CLEAR(*p++);
5708-
assert(array[0] == NULL);
5709-
assert(p == array + 1);
5710-
5711-
Py_RETURN_NONE;
5712-
}
5713-
5714-
5715-
// Test Py_SETREF() and Py_XSETREF() macros, similar to test_py_clear()
5716-
static PyObject*
5717-
test_py_setref(PyObject *self, PyObject *Py_UNUSED(ignored))
5718-
{
5719-
// Py_SETREF() simple case with a variable
5720-
PyObject *obj = PyList_New(0);
5721-
if (obj == NULL) {
5722-
return NULL;
5723-
}
5724-
Py_SETREF(obj, NULL);
5725-
assert(obj == NULL);
5726-
5727-
// Py_XSETREF() simple case with a variable
5728-
PyObject *obj2 = PyList_New(0);
5729-
if (obj2 == NULL) {
5730-
return NULL;
5731-
}
5732-
Py_XSETREF(obj2, NULL);
5733-
assert(obj2 == NULL);
5734-
// test Py_XSETREF() when the argument is NULL
5735-
Py_XSETREF(obj2, NULL);
5736-
assert(obj2 == NULL);
5737-
5738-
// gh-98724: complex case, Py_SETREF() argument has a side effect
5739-
PyObject* array[1];
5740-
array[0] = PyList_New(0);
5741-
if (array[0] == NULL) {
5742-
return NULL;
5743-
}
5744-
5745-
PyObject **p = array;
5746-
Py_SETREF(*p++, NULL);
5747-
assert(array[0] == NULL);
5748-
assert(p == array + 1);
5749-
5750-
// gh-98724: complex case, Py_XSETREF() argument has a side effect
5751-
PyObject* array2[1];
5752-
array2[0] = PyList_New(0);
5753-
if (array2[0] == NULL) {
5754-
return NULL;
5755-
}
5756-
5757-
PyObject **p2 = array2;
5758-
Py_XSETREF(*p2++, NULL);
5759-
assert(array2[0] == NULL);
5760-
assert(p2 == array2 + 1);
5761-
5762-
// test Py_XSETREF() when the argument is NULL
5763-
p2 = array2;
5764-
Py_XSETREF(*p2++, NULL);
5765-
assert(array2[0] == NULL);
5766-
assert(p2 == array2 + 1);
5767-
5768-
Py_RETURN_NONE;
5769-
}
5770-
5771-
57725687
#define TEST_REFCOUNT() \
57735688
do { \
57745689
PyObject *obj = PyList_New(0); \
@@ -6560,8 +6475,6 @@ static PyMethodDef TestMethods[] = {
65606475
{"pynumber_tobase", pynumber_tobase, METH_VARARGS},
65616476
{"without_gc", without_gc, METH_O},
65626477
{"test_set_type_size", test_set_type_size, METH_NOARGS},
6563-
{"test_py_clear", test_py_clear, METH_NOARGS},
6564-
{"test_py_setref", test_py_setref, METH_NOARGS},
65656478
{"test_refcount_macros", test_refcount_macros, METH_NOARGS},
65666479
{"test_refcount_funcs", test_refcount_funcs, METH_NOARGS},
65676480
{"test_py_is_macros", test_py_is_macros, METH_NOARGS},

0 commit comments

Comments
 (0)
0