Skip to content

Commit

Permalink
Address feedback & remove FIXME
Browse files Browse the repository at this point in the history
  • Loading branch information
lysnikolaou committed Aug 2, 2024
1 parent a0a5a99 commit e94c168
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 63 deletions.
28 changes: 8 additions & 20 deletions Cython/Compiler/ExprNodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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" % (
Expand Down Expand Up @@ -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("}")

Expand Down
50 changes: 24 additions & 26 deletions Cython/Utility/ModuleSetupCode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
31 changes: 14 additions & 17 deletions Cython/Utility/ObjectHandling.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit e94c168

Please sign in to comment.