From e94c168ca6cd90e70e0e4843ac5ecaa3057de085 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 2 Aug 2024 20:03:25 +0200 Subject: [PATCH] Address feedback & remove FIXME --- Cython/Compiler/ExprNodes.py | 28 +++++------------- Cython/Utility/ModuleSetupCode.c | 50 +++++++++++++++----------------- Cython/Utility/ObjectHandling.c | 31 +++++++++----------- 3 files changed, 46 insertions(+), 63 deletions(-) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index 1da10a49e99..eb50d810011 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -3127,18 +3127,10 @@ def generate_next_sequence_item(self, test_name, result_name, code): incref = "__Pyx_INCREF" code.putln("#if CYTHON_ASSUME_SAFE_MACROS && !CYTHON_AVOID_BORROWED_REFS") code.putln( - "%s = %s(%s, %s); %s(%s); %s%s; %s" % ( - result_name, - getitem, - self.py_result(), - self.counter_cname, - incref, - result_name, - self.counter_cname, - inc_dec, - # use the error label to avoid C compiler warnings if we only use it below - code.error_goto_if_neg('0', self.pos) - )) + f"{result_name} = {getitem}({self.py_result()}, {self.counter_cname}); " + f"{incref}({result_name}); " + f"{self.counter_cname}{inc_dec}; " + f"{code.error_goto_if_neg('0', self.pos)}") code.putln("#else") code.putln( "%s = __Pyx_PySequence_ITEM(%s, %s); %s%s; %s" % ( @@ -8382,23 +8374,19 @@ def generate_special_parallel_unpacking_code(self, code, rhs, use_loop): code.putln("if (likely(Py%s_CheckExact(sequence))) {" % sequence_types[0]) for i, item in enumerate(self.unpacked_items): if sequence_types[0] == "List": - code.putln("%s = __Pyx_PyList_GetItemRef(sequence, %d);" % ( - item.result(), i)) + code.putln(f"{item.result()} = __Pyx_PyList_GetItemRef(sequence, {i});") code.put_xgotref(item.result(), item.ctype()) else: # Tuple - code.putln("%s = PyTuple_GET_ITEM(sequence, %d);" % ( - item.result(), i)) + code.putln(f"{item.result()} = PyTuple_GET_ITEM(sequence, {i});") code.put_incref(item.result(), item.ctype()) if len(sequence_types) == 2: code.putln("} else {") for i, item in enumerate(self.unpacked_items): if sequence_types[1] == "List": - code.putln("%s = __Pyx_PyList_GetItemRef(sequence, %d);" % ( - item.result(), i)) + code.putln(f"{item.result()} = __Pyx_PyList_GetItemRef(sequence, {i});") code.put_xgotref(item.result(), item.ctype()) else: # Tuple - code.putln("%s = PyTuple_GET_ITEM(sequence, %d);" % ( - item.result(), i)) + code.putln(f"{item.result()} = PyTuple_GET_ITEM(sequence, {i});") code.put_incref(item.result(), item.ctype()) code.putln("}") diff --git a/Cython/Utility/ModuleSetupCode.c b/Cython/Utility/ModuleSetupCode.c index fec4066c415..fd922e27254 100644 --- a/Cython/Utility/ModuleSetupCode.c +++ b/Cython/Utility/ModuleSetupCode.c @@ -1054,8 +1054,14 @@ static CYTHON_INLINE PyObject * __Pyx_PyDict_GetItemStrWithError(PyObject *dict, #if CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && PY_VERSION_HEX >= 0x030d00b1 #define __Pyx_PyList_GetItemRef(o, i) PyList_GetItemRef(o, i) #define __Pyx_PyDict_GetItemRef(dict, key, result) PyDict_GetItemRef(dict, key, result) -#else +#elif !CYTHON_AVOID_BORROWED_REFS + +#if CYTHON_ASSUME_SAFE_MACROS #define __Pyx_PyList_GetItemRef(o, i) __Pyx_NewRef(PyList_GET_ITEM(o, i)) +#else +#define __Pyx_PyList_GetItemRef(o, i) __Pyx_NewRef(PyList_GetItem(o, i)) +#endif + static CYTHON_INLINE int __Pyx_PyDict_GetItemRef(PyObject *dict, PyObject *key, PyObject **result) { *result = PyDict_GetItem(dict, key); if (*result == NULL) { @@ -1064,6 +1070,17 @@ static CYTHON_INLINE int __Pyx_PyDict_GetItemRef(PyObject *dict, PyObject *key, Py_INCREF(*result); return 1; } +#else +#define __Pyx_PyList_GetItemRef(o, i) PySequence_GetItem(o, i) +static CYTHON_INLINE int __Pyx_PyDict_GetItemRef(PyObject *dict, PyObject *key, PyObject **result) { + *result = PyObject_GetItem(dict, key); + if (PyErr_Occurred()) { + return -1; + } else if (*result == NULL) { + return 0; + } + return 1; +} #endif // No-op macro for calling Py_VISIT() on known constants that can never participate in reference cycles. @@ -2225,42 +2242,20 @@ static PyObject* __Pyx_PyCode_New( } #if CYTHON_COMPILING_IN_LIMITED_API - - #if CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && __PYX_LIMITED_VERSION_HEX >= 0x030d00b1 - if (unlikely(PyDict_GetItemRef(tuple_dedup_map, varnames_tuple, &varnames_tuple_dedup) < 0)) goto done; - #else // !(CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && __PYX_LIMITED_VERSION_HEX >= 0x030d00b1) varnames_tuple_dedup = PyDict_GetItem(tuple_dedup_map, varnames_tuple); - #if CYTHON_AVOID_BORROWED_REFS - Py_XINCREF(varnames_tuple_dedup); - #endif // CYTHON_AVOID_BORROWED_REFS - #endif // CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && __PYX_LIMITED_VERSION_HEX >= 0x030d00b1 if (!varnames_tuple_dedup) { if (unlikely(PyDict_SetItem(tuple_dedup_map, varnames_tuple, varnames_tuple) < 0)) goto done; varnames_tuple_dedup = varnames_tuple; } - - #if CYTHON_AVOID_BORROWED_REFS || (CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && __PYX_LIMITED_VERSION_HEX >= 0x030d00b1) - Py_XDECREF(varnames_tuple_dedup); - #endif - - #else // !CYTHON_COMPILING_IN_LIMITED_API - - #if CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && PY_VERSION_HEX >= 0x030d00b1 - if (unlikely(PyDict_SetDefaultRef(tuple_dedup_map, varnames_tuple, varnames_tuple, &varnames_tuple_dedup) < 0)) goto done; - #else // !(CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && PY_VERSION_HEX >= 0x030d00b1) + #else varnames_tuple_dedup = PyDict_SetDefault(tuple_dedup_map, varnames_tuple, varnames_tuple); if (unlikely(!varnames_tuple_dedup)) goto done; + #endif + #if CYTHON_AVOID_BORROWED_REFS Py_INCREF(varnames_tuple_dedup); - #endif // CYTHON_AVOID_BORROWED_REFS - #endif // CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && PY_VERSION_HEX >= 0x030d00b1 - - #if CYTHON_AVOID_BORROWED_REFS || (CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS && PY_VERSION_HEX >= 0x030d00b1) - Py_XDECREF(varnames_tuple_dedup); #endif - #endif // CYTHON_COMPILING_IN_LIMITED_API - code_obj = (PyObject*) __Pyx__PyCode_New( (int) descr.argcount, (int) descr.num_posonly_args, @@ -2281,6 +2276,9 @@ static PyObject* __Pyx_PyCode_New( ); done: + #if CYTHON_AVOID_BORROWED_REFS + Py_XDECREF(varnames_tuple_dedup); + #endif Py_DECREF(varnames_tuple); return code_obj; } diff --git a/Cython/Utility/ObjectHandling.c b/Cython/Utility/ObjectHandling.c index ff1051916b7..6499c3219aa 100644 --- a/Cython/Utility/ObjectHandling.c +++ b/Cython/Utility/ObjectHandling.c @@ -438,7 +438,7 @@ static CYTHON_INLINE PyObject *__Pyx_GetItemInt_{{type}}_Fast(PyObject *o, Py_ss Py_INCREF(r); return r; } - return __Pyx_GetItemInt_Generic(o, PyInt_FromSsize_t(i)); + return __Pyx_GetItemInt_Generic(o, PyLong_FromSsize_t(i)); #else return PySequence_GetItem(o, i); #endif @@ -527,12 +527,12 @@ static CYTHON_INLINE int __Pyx_SetItemInt_Fast(PyObject *o, Py_ssize_t i, PyObje Py_ssize_t n = (!wraparound) ? i : ((likely(i >= 0)) ? i : i + PyList_GET_SIZE(o)); if ((!boundscheck) || likely(__Pyx_is_valid_index(n, PyList_GET_SIZE(o)))) { Py_INCREF(v); -#if !CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS +#if CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS + PyList_SetItem(o, n, v); +#else PyObject* old = PyList_GET_ITEM(o, n); PyList_SET_ITEM(o, n, v); Py_DECREF(old); -#else - PyList_SetItem(o, n, v); #endif return 1; } @@ -1427,9 +1427,15 @@ static CYTHON_INLINE PyObject *__Pyx__GetModuleGlobalName(PyObject *name) #endif { PyObject *result; -// FIXME: clean up the macro guard order here: limited API first, then borrowed refs, then cpython -#if !CYTHON_AVOID_BORROWED_REFS -#if CYTHON_COMPILING_IN_CPYTHON && !CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS +#if CYTHON_COMPILING_IN_LIMITED_API + if (unlikely(!$module_cname)) { + return NULL; + } + result = PyObject_GetAttr($module_cname, name); + if (likely(result)) { + return result; + } +#elif !CYTHON_AVOID_BORROWED_REFS && !CYTHON_AVOID_THREAD_UNSAFE_BORROWED_REFS // Identifier names are always interned and have a pre-calculated hash value. result = _PyDict_GetItem_KnownHash($moddict_cname, name, ((PyASCIIObject *) name)->hash); __PYX_UPDATE_DICT_CACHE($moddict_cname, result, *dict_cached_value, *dict_version) @@ -1438,21 +1444,12 @@ static CYTHON_INLINE PyObject *__Pyx__GetModuleGlobalName(PyObject *name) } else if (unlikely(PyErr_Occurred())) { return NULL; } -#elif CYTHON_COMPILING_IN_LIMITED_API - if (unlikely(!$module_cname)) { - return NULL; - } - result = PyObject_GetAttr($module_cname, name); - if (likely(result)) { - return result; - } -#else +#elif !CYTHON_AVOID_BORROWED_REFS if (unlikely(__Pyx_PyDict_GetItemRef($moddict_cname, name, &result) == -1)) PyErr_Clear(); __PYX_UPDATE_DICT_CACHE($moddict_cname, result, *dict_cached_value, *dict_version) if (likely(result)) { return result; } -#endif #else result = PyObject_GetItem($moddict_cname, name); __PYX_UPDATE_DICT_CACHE($moddict_cname, result, *dict_cached_value, *dict_version)