wip: replace use of PyCell_GET/SET

nascheme · nascheme · commit b91d59bd7045 · 2024-11-25T13:59:42.000-08:00
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
@@ -8,6 +8,7 @@
 #include "pycore_moduleobject.h"  // _PyModule_GetDict()
 #include "pycore_modsupport.h"    // _PyArg_CheckPositional()
 #include "pycore_object.h"        // _PyObject_GC_UNTRACK()
+#include "pycore_cell.h"          // PyCell_GetRef() PyCell_SetTakeRef()
 #include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches
 
 
@@ -187,11 +188,8 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value)
             }
         }
         if (cell != NULL) {
-            PyObject *oldvalue_o = PyCell_GET(cell);
-            if (value != oldvalue_o) {
-                PyCell_SET(cell, Py_XNewRef(value));
-                Py_XDECREF(oldvalue_o);
-            }
+            Py_XINCREF(value);
+            PyCell_SetTakeRef((PyCellObject *)cell, value);
         } else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {
             PyStackRef_XCLOSE(fast[i]);
             fast[i] = PyStackRef_FromPyObjectNew(value);
@@ -1987,19 +1985,22 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i,
         if (kind & CO_FAST_FREE) {
             // The cell was set by COPY_FREE_VARS.
             assert(value != NULL && PyCell_Check(value));
-            value = PyCell_GET(value);
+            value = PyCell_GetRef((PyCellObject *)value);
         }
         else if (kind & CO_FAST_CELL) {
             if (value != NULL) {
                 if (PyCell_Check(value)) {
                     assert(!_PyFrame_IsIncomplete(frame));
-                    value = PyCell_GET(value);
+                    value = PyCell_GetRef((PyCellObject *)value);
                 }
                 // (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL),
                 // with the initial value set when the frame was created...
                 // (unlikely) ...or it was set via the f_locals proxy.
             }
         }
+        else {
+            Py_INCREF(value);
+        }
     }
     *pvalue = value;
     return 1;
@@ -2076,14 +2077,14 @@ PyFrame_GetVar(PyFrameObject *frame_obj, PyObject *name)
             continue;
         }
 
-        PyObject *value;  // borrowed reference
+        PyObject *value;
         if (!frame_get_var(frame, co, i, &value)) {
             break;
         }
         if (value == NULL) {
             break;
         }
-        return Py_NewRef(value);
+        return value;
     }
 
     PyErr_Format(PyExc_NameError, "variable %R does not exist", name);
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -19,6 +19,7 @@
 #include "pycore_typeobject.h"    // struct type_cache
 #include "pycore_unionobject.h"   // _Py_union_type_or
 #include "pycore_weakref.h"       // _PyWeakref_GET_REF()
+#include "pycore_cell.h"          // PyCell_GetRef()
 #include "opcode.h"               // MAKE_CELL
 
 #include <stddef.h>               // ptrdiff_t
@@ -11658,23 +11659,28 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p,
 
     assert(_PyFrame_GetCode(cframe)->co_nlocalsplus > 0);
     PyObject *firstarg = PyStackRef_AsPyObjectBorrow(_PyFrame_GetLocalsArray(cframe)[0]);
+    if (firstarg == NULL) {
+        PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted");
+        return -1;
+    }
     // The first argument might be a cell.
-    if (firstarg != NULL && (_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL)) {
-        // "firstarg" is a cell here unless (very unlikely) super()
-        // was called from the C-API before the first MAKE_CELL op.
-        if (_PyInterpreterFrame_LASTI(cframe) >= 0) {
-            // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need
-            // to use _PyOpcode_Deopt here:
-            assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL ||
-                   _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS);
-            assert(PyCell_Check(firstarg));
-            firstarg = PyCell_GET(firstarg);
+    // "firstarg" is a cell here unless (very unlikely) super()
+    // was called from the C-API before the first MAKE_CELL op.
+    if ((_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL) &&
+            (_PyInterpreterFrame_LASTI(cframe) >= 0)) {
+        // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need
+        // to use _PyOpcode_Deopt here:
+        assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL ||
+                _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS);
+        assert(PyCell_Check(firstarg));
+        firstarg = PyCell_GetRef((PyCellObject *)firstarg);
+        if (firstarg == NULL) {
+            PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted");
+            return -1;
         }
     }
-    if (firstarg == NULL) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "super(): arg[0] deleted");
-        return -1;
+    else {
+        Py_INCREF(firstarg);
     }
 
     // Look for __class__ in the free vars.
@@ -11689,18 +11695,22 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p,
             if (cell == NULL || !PyCell_Check(cell)) {
                 PyErr_SetString(PyExc_RuntimeError,
                   "super(): bad __class__ cell");
+                Py_DECREF(firstarg);
                 return -1;
             }
-            type = (PyTypeObject *) PyCell_GET(cell);
+            type = (PyTypeObject *) PyCell_GetRef((PyCellObject *)cell);
             if (type == NULL) {
                 PyErr_SetString(PyExc_RuntimeError,
                   "super(): empty __class__ cell");
+                Py_DECREF(firstarg);
                 return -1;
             }
             if (!PyType_Check(type)) {
                 PyErr_Format(PyExc_RuntimeError,
                   "super(): __class__ is not a type (%s)",
                   Py_TYPE(type)->tp_name);
+                Py_DECREF(type);
+                Py_DECREF(firstarg);
                 return -1;
             }
             break;
@@ -11709,6 +11719,7 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p,
     if (type == NULL) {
         PyErr_SetString(PyExc_RuntimeError,
                         "super(): __class__ cell not found");
+        Py_DECREF(firstarg);
         return -1;
     }
 
@@ -11755,16 +11766,23 @@ super_init_impl(PyObject *self, PyTypeObject *type, PyObject *obj) {
             return -1;
         }
     }
+    else {
+        Py_INCREF(type);
+        Py_XINCREF(obj);
+    }
 
-    if (obj == Py_None)
+    if (obj == Py_None) {
+        Py_DECREF(obj);
         obj = NULL;
+    }
     if (obj != NULL) {
         obj_type = supercheck(type, obj);
-        if (obj_type == NULL)
+        if (obj_type == NULL) {
+            Py_DECREF(obj);
             return -1;
-        Py_INCREF(obj);
+        }
     }
-    Py_XSETREF(su->type, (PyTypeObject*)Py_NewRef(type));
+    Py_XSETREF(su->type, (PyTypeObject*)type);
     Py_XSETREF(su->obj, obj);
     Py_XSETREF(su->obj_type, obj_type);
     return 0;
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
@@ -13,6 +13,7 @@
 #include "pycore_pythonrun.h"     // _Py_SourceAsString()
 #include "pycore_sysmodule.h"     // _PySys_GetAttr()
 #include "pycore_tuple.h"         // _PyTuple_FromArray()
+#include "pycore_cell.h"          // PyCell_GetRef()
 
 #include "clinic/bltinmodule.c.h"
 
@@ -209,7 +210,7 @@ builtin___build_class__(PyObject *self, PyObject *const *args, Py_ssize_t nargs,
         PyObject *margs[3] = {name, bases, ns};
         cls = PyObject_VectorcallDict(meta, margs, 3, mkw);
         if (cls != NULL && PyType_Check(cls) && PyCell_Check(cell)) {
-            PyObject *cell_cls = PyCell_GET(cell);
+            PyObject *cell_cls = PyCell_GetRef((PyCellObject *)cell);
             if (cell_cls != cls) {
                 if (cell_cls == NULL) {
                     const char *msg =
@@ -221,9 +222,13 @@ builtin___build_class__(PyObject *self, PyObject *const *args, Py_ssize_t nargs,
                         "__class__ set to %.200R defining %.200R as %.200R";
                     PyErr_Format(PyExc_TypeError, msg, cell_cls, name, cls);
                 }
+                Py_XDECREF(cell_cls);
                 Py_SETREF(cls, NULL);
                 goto error;
             }
+            else {
+                Py_DECREF(cell_cls);
+            }
         }
     }
 error:

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`	`#include "pycore_moduleobject.h" // _PyModule_GetDict()`
`9`	`9`	`#include "pycore_modsupport.h" // _PyArg_CheckPositional()`
`10`	`10`	`#include "pycore_object.h" // _PyObject_GC_UNTRACK()`
	`11`	`+#include "pycore_cell.h" // PyCell_GetRef() PyCell_SetTakeRef()`
`11`	`12`	`#include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches`
`12`	`13`
`13`	`14`
`@@ -187,11 +188,8 @@ framelocalsproxy_setitem(PyObject self, PyObject key, PyObject *value)`
`187`	`188`	`}`
`188`	`189`	`}`
`189`	`190`	`if (cell != NULL) {`
`190`		`- PyObject *oldvalue_o = PyCell_GET(cell);`
`191`		`- if (value != oldvalue_o) {`
`192`		`- PyCell_SET(cell, Py_XNewRef(value));`
`193`		`- Py_XDECREF(oldvalue_o);`
`194`		`- }`
	`191`	`+ Py_XINCREF(value);`
	`192`	`+ PyCell_SetTakeRef((PyCellObject *)cell, value);`
`195`	`193`	`} else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {`
`196`	`194`	`PyStackRef_XCLOSE(fast[i]);`
`197`	`195`	`fast[i] = PyStackRef_FromPyObjectNew(value);`
`@@ -1987,19 +1985,22 @@ frame_get_var(_PyInterpreterFrame frame, PyCodeObject co, int i,`
`1987`	`1985`	`if (kind & CO_FAST_FREE) {`
`1988`	`1986`	`// The cell was set by COPY_FREE_VARS.`
`1989`	`1987`	`assert(value != NULL && PyCell_Check(value));`
`1990`		`- value = PyCell_GET(value);`
	`1988`	`+ value = PyCell_GetRef((PyCellObject *)value);`
`1991`	`1989`	`}`
`1992`	`1990`	`else if (kind & CO_FAST_CELL) {`
`1993`	`1991`	`if (value != NULL) {`
`1994`	`1992`	`if (PyCell_Check(value)) {`
`1995`	`1993`	`assert(!_PyFrame_IsIncomplete(frame));`
`1996`		`- value = PyCell_GET(value);`
	`1994`	`+ value = PyCell_GetRef((PyCellObject *)value);`
`1997`	`1995`	`}`
`1998`	`1996`	`// (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL),`
`1999`	`1997`	`// with the initial value set when the frame was created...`
`2000`	`1998`	`// (unlikely) ...or it was set via the f_locals proxy.`
`2001`	`1999`	`}`
`2002`	`2000`	`}`
	`2001`	`+ else {`
	`2002`	`+ Py_INCREF(value);`
	`2003`	`+ }`
`2003`	`2004`	`}`
`2004`	`2005`	`*pvalue = value;`
`2005`	`2006`	`return 1;`
`@@ -2076,14 +2077,14 @@ PyFrame_GetVar(PyFrameObject frame_obj, PyObject name)`
`2076`	`2077`	`continue;`
`2077`	`2078`	`}`
`2078`	`2079`
`2079`		`- PyObject *value; // borrowed reference`
	`2080`	`+ PyObject *value;`
`2080`	`2081`	`if (!frame_get_var(frame, co, i, &value)) {`
`2081`	`2082`	`break;`
`2082`	`2083`	`}`
`2083`	`2084`	`if (value == NULL) {`
`2084`	`2085`	`break;`
`2085`	`2086`	`}`
`2086`		`- return Py_NewRef(value);`
	`2087`	`+ return value;`
`2087`	`2088`	`}`
`2088`	`2089`
`2089`	`2090`	`PyErr_Format(PyExc_NameError, "variable %R does not exist", name);`