From ad7ddef79028b373bb67e36456f7c7bf9e103bd0 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 15 Nov 2022 00:37:00 +0100 Subject: [PATCH 1/2] Add Py_SETREF and Py_CLEAR operations * Remove FORCE_NEWREF. Instead, exclude NewRef operations from "all". * Enhance Py_INCREF_assign. * Replace also "PyObject *var = x; Py_INCREF(x);" --- docs/changelog.rst | 2 + docs/upgrade.rst | 28 ++++ tests/test_upgrade_pythoncapi.py | 274 +++++++++++++++++++++++++++++-- upgrade_pythoncapi.py | 188 +++++++++++++++++---- 4 files changed, 444 insertions(+), 48 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index f54d609..e124768 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,8 @@ Changelog ========= +* 2022-11-15: Add experimental operations to the upgrade_pythoncapi script: + ``Py_INCREF_return``, ``Py_INCREF_assign``, ``Py_CLEAR`` and ``Py_SETREF``. * 2022-11-09: Fix ``Py_SETREF()`` and ``Py_XSETREF()`` macros for `gh-98724 `_. * 2022-11-04: Add ``PyFrame_GetVar()`` and ``PyFrame_GetVarString()`` diff --git a/docs/upgrade.rst b/docs/upgrade.rst index 551ce3e..8d3b32f 100644 --- a/docs/upgrade.rst +++ b/docs/upgrade.rst @@ -141,3 +141,31 @@ PyThreadState_GetFrame ---------------------- * Replace ``tstate->frame`` with ``_PyThreadState_GetFrameBorrow(tstate)``. + +Experimental operations +----------------------- + +The following operations are experimental (ex: can introduce compiler warnings) +and so not included in the ``all`` group, they have to be selected explicitly. +Example: ``-o all,Py_SETREF``. + +Experimental operations: + +* ``Py_INCREF_return``: + + * Replace ``Py_INCREF(res); return res;`` with ``return Py_NewRef(res);`` + +* ``Py_INCREF_assign``: + + * Replace ``x = y; Py_INCREF(x);`` with ``x = Py_NewRef(y);`` + * Replace ``x = y; Py_INCREF(y);`` with ``x = Py_NewRef(y);`` + * Replace ``Py_INCREF(y); x = y;`` with ``x = Py_NewRef(y);`` + +* ``Py_CLEAR``: + + * Replace ``Py_XDECREF(var); var = NULL;`` with ``Py_CLEAR(var);`` + +* ``Py_SETREF``: + + * Replace ``Py_DECREF(x); x = y;`` with ``Py_SETREF(x, y);`` + * Replace ``Py_XDECREF(x); x = y;`` with ``Py_XSETREF(x, y);`` diff --git a/tests/test_upgrade_pythoncapi.py b/tests/test_upgrade_pythoncapi.py index ab9fe4b..df01e31 100644 --- a/tests/test_upgrade_pythoncapi.py +++ b/tests/test_upgrade_pythoncapi.py @@ -11,10 +11,17 @@ import upgrade_pythoncapi # noqa +OPERATIONS = ["all"] +for op in upgrade_pythoncapi.EXCLUDE_FROM_ALL: + OPERATIONS.append(op.NAME) +OPERATIONS = ','.join(OPERATIONS) + + def patch(source, no_compat=False): - args = ['script', 'mod.c'] + args = ['script', 'mod.c', '-o', OPERATIONS] if no_compat: args.append('--no-compat') + patcher = upgrade_pythoncapi.Patcher(args) return patcher.patch(source) @@ -391,7 +398,6 @@ def test_pythreadstate_getframe(self): { return _PyThreadState_GetFrameBorrow(tstate); } """) - @unittest.skipUnless(upgrade_pythoncapi.FORCE_NEWREF, 'FORCE_NEWREF=False') def test_py_incref_return(self): self.check_replace(""" PyObject* new_ref(PyObject *obj) { @@ -432,8 +438,8 @@ def test_py_incref_return(self): } """) - @unittest.skipUnless(upgrade_pythoncapi.FORCE_NEWREF, 'FORCE_NEWREF=False') def test_py_incref_assign(self): + # INCREF, assign self.check_replace(""" void set_attr(MyStruct *obj, PyObject *value, int test) { @@ -446,20 +452,6 @@ def test_py_incref_assign(self): // 3 obj->attr = value; Py_INCREF(obj->attr); - - // same line 1 - obj->attr = value; Py_INCREF(value); - // same line 2 - if (test) { obj->attr = value; Py_INCREF(obj->attr); } - // same line 3 - if (test) { Py_INCREF(value); obj->attr = value; } - - // cast 1 - Py_INCREF(value); - obj->attr = (PyObject*)value; - // cast 2 - obj->attr = (PyObject*)value; - Py_INCREF(value); } """, """ #include "pythoncapi_compat.h" @@ -472,21 +464,87 @@ def test_py_incref_assign(self): obj->attr = Py_NewRef(value); // 3 obj->attr = Py_NewRef(value); + } + """) + # Same line + self.check_replace(""" + void set_attr(MyStruct *obj, PyObject *value, int test) + { + // same line 1 + obj->attr = value; Py_INCREF(value); + // same line 2 + if (test) { obj->attr = value; Py_INCREF(obj->attr); } + // same line 3 + if (test) { Py_INCREF(value); obj->attr = value; } + } + """, """ + #include "pythoncapi_compat.h" + + void set_attr(MyStruct *obj, PyObject *value, int test) + { // same line 1 obj->attr = Py_NewRef(value); // same line 2 if (test) { obj->attr = Py_NewRef(value); } // same line 3 if (test) { obj->attr = Py_NewRef(value); } + } + """) + # Cast + self.check_replace(""" + void set_attr(MyStruct *obj, PyObject *value, int test) + { + // cast 1 + Py_INCREF(value); + obj->attr = (PyObject*)value; + // cast 2 + obj->attr = (PyObject*)value; + Py_INCREF(value); + + // assign var, incref + PyCodeObject *code_obj = (PyCodeObject *)code; + Py_INCREF(code_obj); + // assign var, incref + PyCodeObject* code_obj = (PyCodeObject *)code; + Py_INCREF(code); + // assign var, xincref + PyCodeObject * code_obj = (PyCodeObject *)code; + Py_XINCREF(code_obj); + + // incref, assign var + Py_INCREF(code); + PyCodeObject* code_obj = (PyCodeObject *)code; + // xincref, assign var + Py_XINCREF(code); + PyCodeObject *code_obj = (PyCodeObject *)code; + } + """, """ + #include "pythoncapi_compat.h" + + void set_attr(MyStruct *obj, PyObject *value, int test) + { // cast 1 obj->attr = Py_NewRef(value); // cast 2 obj->attr = Py_NewRef(value); + + // assign var, incref + PyCodeObject *code_obj = (PyCodeObject *)Py_NewRef(code); + // assign var, incref + PyCodeObject* code_obj = (PyCodeObject *)Py_NewRef(code); + // assign var, xincref + PyCodeObject * code_obj = (PyCodeObject *)Py_XNewRef(code); + + // incref, assign var + PyCodeObject* code_obj = (PyCodeObject *)Py_NewRef(code); + // xincref, assign var + PyCodeObject *code_obj = (PyCodeObject *)Py_XNewRef(code); } """) + # Py_XINCREF self.check_replace(""" void set_xattr(MyStruct *obj, PyObject *value) { @@ -566,6 +624,188 @@ def test_py_incref_assign(self): } """) + def test_py_clear(self): + self.check_replace(""" + void clear(PyObject **obj) + { + // 1 + Py_DECREF(*obj); + *obj = NULL; + // 2 + Py_XDECREF(*obj); + *obj = NULL; + } + """, """ + void clear(PyObject **obj) + { + // 1 + Py_CLEAR(*obj); + // 2 + Py_CLEAR(*obj); + } + """) + + def test_py_setref(self): + self.check_replace(""" + void set(PyObject **obj, PyObject *t) + { + // DECREF + Py_DECREF(*obj); + *obj = t; + + // XDECREF + Py_XDECREF(*obj); + *obj = t; + + // DECREF, INCREF + Py_DECREF(*obj); + Py_INCREF(t); + *obj = t; + } + """, """ + #include "pythoncapi_compat.h" + + void set(PyObject **obj, PyObject *t) + { + // DECREF + Py_SETREF(*obj, t); + + // XDECREF + Py_XSETREF(*obj, t); + + // DECREF, INCREF + Py_SETREF(*obj, Py_NewRef(t)); + } + """) + + self.check_replace(""" + void set(PyObject **obj, PyObject *value) + { + // 1 + PyObject *old = *obj; + *obj = value; + Py_DECREF(old); + // 2 + PyObject *old = *obj; + *obj = Py_XNewRef(value); + Py_DECREF(old); + // 3 + PyObject *old = *obj; + *obj = value; + Py_XDECREF(old); + // 4 + PyObject *old = *obj; + *obj = Py_NewRef(value); + Py_XDECREF(old); + } + """, """ + #include "pythoncapi_compat.h" + + void set(PyObject **obj, PyObject *value) + { + // 1 + Py_SETREF(*obj, value); + // 2 + Py_SETREF(*obj, Py_XNewRef(value)); + // 3 + Py_XSETREF(*obj, value); + // 4 + Py_XSETREF(*obj, Py_NewRef(value)); + } + """) + + # INCREF, DECREF, assign + self.check_replace(""" + void set(void) + { + // 1 + Py_INCREF(value); + Py_DECREF(obj); + obj = value; + // 2 + Py_INCREF(value); + Py_XDECREF(obj); + obj = value; + // 3 + Py_XINCREF(value); + Py_DECREF(obj); + obj = value; + // 4 + Py_XINCREF(value); + Py_XDECREF(obj); + obj = value; + } + """, """ + #include "pythoncapi_compat.h" + + void set(void) + { + // 1 + Py_SETREF(obj, Py_NewRef(value)); + // 2 + Py_XSETREF(obj, Py_NewRef(value)); + // 3 + Py_SETREF(obj, Py_XNewRef(value)); + // 4 + Py_XSETREF(obj, Py_XNewRef(value)); + } + """) + + # old variable + self.check_replace(""" + void set(PyObject **obj, PyObject *value) + { + // 1 + PyObject *old_next = (PyObject*)self->tb_next; + self->tb_next = (PyTracebackObject *)Py_XNewRef(new_next); + Py_XDECREF(old_next); + // 2 + old_next = (PyObject*)self->tb_next; + self->tb_next = (PyTracebackObject *)Py_XNewRef(new_next); + Py_XDECREF(old_next); + } + """, """ + #include "pythoncapi_compat.h" + + void set(PyObject **obj, PyObject *value) + { + // 1 + Py_XSETREF(self->tb_next, (PyTracebackObject *)Py_XNewRef(new_next)); + // 2 + Py_XSETREF(self->tb_next, (PyTracebackObject *)Py_XNewRef(new_next)); + } + """) + + # Py_CLEAR + self.check_replace(""" + void set(PyObject **obj, PyObject *value) + { + // 1 + Py_CLEAR(self->tb_next); + self->tb_next = value; + // 2 + Py_INCREF(value); + Py_CLEAR(self->tb_next); + self->tb_next = value; + // 3 + Py_XINCREF(value); + Py_CLEAR(self->tb_next); + self->tb_next = value; + } + """, """ + #include "pythoncapi_compat.h" + + void set(PyObject **obj, PyObject *value) + { + // 1 + Py_XSETREF(self->tb_next, value); + // 2 + Py_XSETREF(self->tb_next, Py_NewRef(value)); + // 3 + Py_XSETREF(self->tb_next, Py_XNewRef(value)); + } + """) + def test_py_is(self): self.check_replace(""" void test_py_is(PyObject *x) diff --git a/upgrade_pythoncapi.py b/upgrade_pythoncapi.py index e4edc0d..97b9c57 100755 --- a/upgrade_pythoncapi.py +++ b/upgrade_pythoncapi.py @@ -6,7 +6,7 @@ import sys -FORCE_NEWREF = False +MIN_PYTHON = (2, 7) PYTHONCAPI_COMPAT_URL = ('https://raw.githubusercontent.com/python/' @@ -44,6 +44,12 @@ fr"{SUBEXPR_REGEX}" # "var" fr"(?:(?:->|\.){SUBEXPR_REGEX})*") # "->attr" or ".attr" +# # Match 'PyObject *var' and 'struct MyStruct* var' +TYPE_PTR_REGEX = fr'{ID_REGEX} *\*' + +# Match '(PyObject*)' and nothing +OPT_CAST_REGEX = fr'(?:\({TYPE_PTR_REGEX} *\){SPACE_REGEX}*)?' + def same_indentation(group): # the regex must have re.MULTILINE flag @@ -68,7 +74,7 @@ def get_member_regex(member): def assign_regex_str(var, expr): # Match "var = expr;". - return fr'{var}\s*=\s*{expr}\s*;' + return fr'{var} *= *{expr}\s*;' def set_member_regex(member): @@ -85,10 +91,6 @@ def call_assign_regex(name): return re.compile(regex) -def optional_ptr_cast(to_type): - return fr'(?:\({to_type}\s*\*\){SPACE_REGEX}*)?' - - def is_c_filename(filename): return filename.endswith(C_FILE_EXT) @@ -141,7 +143,7 @@ class Py_SET_TYPE(Operation): (set_member_regex('ob_type'), r'Py_SET_TYPE(\1, \2);'), ) # Need Py_SET_TYPE(): new in Python 3.9. - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class Py_SET_SIZE(Operation): @@ -151,7 +153,7 @@ class Py_SET_SIZE(Operation): (set_member_regex('ob_size'), r'Py_SET_SIZE(\1, \2);'), ) # Need Py_SET_SIZE(): new in Python 3.9. - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class Py_SET_REFCNT(Operation): @@ -161,7 +163,7 @@ class Py_SET_REFCNT(Operation): (set_member_regex('ob_refcnt'), r'Py_SET_REFCNT(\1, \2);'), ) # Need Py_SET_REFCNT(): new in Python 3.9. - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class PyObject_NEW(Operation): @@ -211,7 +213,7 @@ class PyFrame_GetBack(Operation): (get_member_regex('f_back'), r'_PyFrame_GetBackBorrow(\1)'), ) # Need _PyFrame_GetBackBorrow() (PyFrame_GetBack() is new in Python 3.9) - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class PyFrame_GetCode(Operation): @@ -221,7 +223,7 @@ class PyFrame_GetCode(Operation): (get_member_regex('f_code'), r'_PyFrame_GetCodeBorrow(\1)'), ) # Need _PyFrame_GetCodeBorrow() (PyFrame_GetCode() is new in Python 3.9) - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class PyThreadState_GetInterpreter(Operation): @@ -230,7 +232,7 @@ class PyThreadState_GetInterpreter(Operation): (get_member_regex('interp'), r'PyThreadState_GetInterpreter(\1)'), ) # Need PyThreadState_GetInterpreter() (new in Python 3.9) - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class PyThreadState_GetFrame(Operation): @@ -240,7 +242,7 @@ class PyThreadState_GetFrame(Operation): ) # Need _PyThreadState_GetFrameBorrow() # (PyThreadState_GetFrame() is new in Python 3.9) - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 9)) class Py_INCREF_return(Operation): @@ -253,7 +255,7 @@ class Py_INCREF_return(Operation): (re.compile(fr'({INDENTATION_REGEX})' + fr'Py_(X?)INCREF\(({EXPR_REGEX})\)\s*;' + same_indentation(r'\1') - + r'return ' + optional_ptr_cast('PyObject') + r'\3;', + + fr'return {OPT_CAST_REGEX}\3;', re.MULTILINE), r'\1return Py_\2NewRef(\3);'), @@ -261,12 +263,12 @@ class Py_INCREF_return(Operation): # but the two statements are on the same line. (re.compile(fr'Py_(X?)INCREF\(({EXPR_REGEX})\)\s*;' + fr'{SPACE_REGEX}*' - + r'return ' + optional_ptr_cast('PyObject') + r'\2;', + + fr'return {OPT_CAST_REGEX}\2;', re.MULTILINE), r'return Py_\1NewRef(\2);'), ) # Need Py_NewRef(): new in Python 3.10 - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 10)) class Py_INCREF_assign(Operation): @@ -283,7 +285,7 @@ class Py_INCREF_assign(Operation): + fr'Py_(X?)INCREF\(({EXPR_REGEX})\);' + same_indentation(r'\1') + assign_regex_str(fr'({EXPR_REGEX})', - optional_ptr_cast('PyObject') + r'\3'), + fr'{OPT_CAST_REGEX}\3'), re.MULTILINE), r'\1\4 = Py_\2NewRef(\3);'), @@ -292,7 +294,7 @@ class Py_INCREF_assign(Operation): (re.compile(fr'Py_(X?)INCREF\(({EXPR_REGEX})\);' + fr'{SPACE_REGEX}*' + assign_regex_str(fr'({EXPR_REGEX})', - optional_ptr_cast('PyObject') + r'\2')), + fr'{OPT_CAST_REGEX}\2')), r'\3 = Py_\1NewRef(\2);'), # "y = x; Py_INCREF(x);" => "y = Py_NewRef(x);" @@ -304,7 +306,7 @@ class Py_INCREF_assign(Operation): # regex does not match. (re.compile(fr'({INDENTATION_REGEX})' + assign_regex_str(fr'({EXPR_REGEX})', - optional_ptr_cast('PyObject') + fr'({EXPR_REGEX})') + fr'{OPT_CAST_REGEX}({EXPR_REGEX})') + same_indentation(r'\1') + r'Py_(X?)INCREF\((?:\2|\3)\);', re.MULTILINE), @@ -313,13 +315,125 @@ class Py_INCREF_assign(Operation): # Same regex than the previous one, # but the two statements are on the same line. (re.compile(assign_regex_str(fr'({EXPR_REGEX})', - optional_ptr_cast('PyObject') + fr'({EXPR_REGEX})') + fr'{OPT_CAST_REGEX}({EXPR_REGEX})') + fr'{SPACE_REGEX}*' + r'Py_(X?)INCREF\((?:\1|\2)\);'), r'\1 = Py_\3NewRef(\2);'), + + # "PyObject *var = x; Py_INCREF(x);" => "PyObject *var = Py_NewRef(x);" + # The two statements must have the same indentation, otherwise the + # regex does not match. + (re.compile(fr'({INDENTATION_REGEX})' + # "type* var = expr;" + + assign_regex_str(fr'({TYPE_PTR_REGEX} *)({EXPR_REGEX})', + fr'({OPT_CAST_REGEX})({EXPR_REGEX})') + + same_indentation(r'\1') + # "Py_INCREF(var);" + + r'Py_(X?)INCREF\((?:\3|\5)\);', + re.MULTILINE), + r'\1\2\3 = \4Py_\6NewRef(\5);'), + + # "Py_INCREF(x); PyObject *var = x;" => "PyObject *var = Py_NewRef(x);" + # The two statements must have the same indentation, otherwise the + # regex does not match. + (re.compile(fr'({INDENTATION_REGEX})' + # "Py_INCREF(var);" + + fr'Py_(X?)INCREF\(({EXPR_REGEX})\);' + + same_indentation(r'\1') + # "type* var = expr;" + + assign_regex_str(fr'({TYPE_PTR_REGEX} *{EXPR_REGEX})', + fr'({OPT_CAST_REGEX})\3'), + re.MULTILINE), + r'\1\4 = \5Py_\2NewRef(\3);'), ) # Need Py_NewRef(): new in Python 3.10 - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 10)) + + +class Py_CLEAR(Operation): + NAME = "Py_CLEAR" + REPLACE = ( + # "Py_DECREF(x); x = NULL;" => "Py_CLEAR(x)"; + # The two statements must have the same indentation, otherwise the + # regex does not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'Py_X?DECREF\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\2', r'NULL'), + re.MULTILINE), + r'\1Py_CLEAR(\2);'), + ) + + +SETREF_VALUE = fr'{OPT_CAST_REGEX}(?:{EXPR_REGEX}|Py_X?NewRef\({EXPR_REGEX}\))' + + +class Py_SETREF(Operation): + NAME = "Py_SETREF" + REPLACE = ( + # "Py_INCREF(y); Py_CLEAR(x); x = y;" => "Py_XSETREF(x, y)"; + # Statements must have the same indentation, otherwise the regex does + # not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'Py_(X?)INCREF\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + fr'Py_CLEAR\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\4', r'\3'), + re.MULTILINE), + r'\1Py_XSETREF(\4, Py_\2NewRef(\3));'), + + # "Py_CLEAR(x); x = y;" => "Py_XSETREF(x, y)"; + # Statements must have the same indentation, otherwise the regex does + # not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'Py_CLEAR\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\2', + fr'({SETREF_VALUE})'), + re.MULTILINE), + r'\1Py_XSETREF(\2, \3);'), + + # "Py_INCREF(y); Py_DECREF(x); x = y;" => "Py_SETREF(x, y)"; + # Statements must have the same indentation, otherwise the regex does + # not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'Py_(X?)INCREF\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + fr'Py_(X?)DECREF\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\5', r'\3'), + re.MULTILINE), + r'\1Py_\4SETREF(\5, Py_\2NewRef(\3));'), + + # "Py_DECREF(x); x = y;" => "Py_SETREF(x, y)"; + # "Py_DECREF(x); x = Py_NewRef(y);" => "Py_SETREF(x, Py_NewRef(y))"; + # Statements must have the same indentation, otherwise the regex does + # not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'Py_(X?)DECREF\(({EXPR_REGEX})\) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\3', + fr'({SETREF_VALUE})'), + re.MULTILINE), + r'\1Py_\2SETREF(\3, \4);'), + + # "old = var; var = new; Py_DECREF(old);" => "Py_SETREF(var, new);" + # "PyObject *old = var; var = new; Py_DECREF(old);" => "Py_SETREF(var, new);" + # Statements must have the same indentation, otherwise the regex does + # not match. + (re.compile(fr'({INDENTATION_REGEX})' + + fr'(?:{ID_REGEX} *\* *)?({ID_REGEX}) *= *{OPT_CAST_REGEX}({EXPR_REGEX}) *;' + + same_indentation(r'\1') + + assign_regex_str(r'\3', + fr'({SETREF_VALUE})') + + same_indentation(r'\1') + + fr'Py_(X?)DECREF\(\2\) *;', + re.MULTILINE), + r'\1Py_\5SETREF(\3, \4);'), + ) + # Need Py_NewRef(): new in Python 3.5 + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 5)) class Py_Is(Operation): @@ -343,10 +457,10 @@ def replace2(regs): )) # Need Py_IsNone(), Py_IsTrue(), Py_IsFalse(): new in Python 3.10 - NEED_PYTHONCAPI_COMPAT = True + NEED_PYTHONCAPI_COMPAT = (MIN_PYTHON < (3, 10)) -OPERATIONS = [ +OPERATIONS = ( Py_SET_TYPE, Py_SET_SIZE, Py_SET_REFCNT, @@ -366,12 +480,25 @@ def replace2(regs): PyThreadState_GetInterpreter, PyThreadState_GetFrame, -] -if FORCE_NEWREF: - OPERATIONS.extend(( - Py_INCREF_return, - Py_INCREF_assign, - )) + + # Code style: excluded from "all" + Py_INCREF_return, + Py_INCREF_assign, + Py_CLEAR, + Py_SETREF, +) + +EXCLUDE_FROM_ALL = ( + Py_INCREF_return, + Py_INCREF_assign, + Py_CLEAR, + Py_SETREF, +) + + +def all_operations(): + return set(operation_class.NAME for operation_class in OPERATIONS + if operation_class not in EXCLUDE_FROM_ALL) class Patcher: @@ -404,8 +531,7 @@ def _get_operations(self, parser): continue if name == "all": - wanted |= set(operation_class.NAME - for operation_class in OPERATIONS) + wanted |= all_operations() elif name.startswith("-"): name = name[1:] wanted.discard(name) From 902ee12e1085f9cf6d35864096a38d1e341c32c2 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 22 Nov 2022 14:32:06 +0100 Subject: [PATCH 2/2] Py_CLEAR doesn't replace Py_DECREF() --- tests/test_upgrade_pythoncapi.py | 69 ++++++++++++++++++++++---------- upgrade_pythoncapi.py | 10 ++++- 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/tests/test_upgrade_pythoncapi.py b/tests/test_upgrade_pythoncapi.py index df01e31..b66f957 100644 --- a/tests/test_upgrade_pythoncapi.py +++ b/tests/test_upgrade_pythoncapi.py @@ -11,14 +11,24 @@ import upgrade_pythoncapi # noqa -OPERATIONS = ["all"] -for op in upgrade_pythoncapi.EXCLUDE_FROM_ALL: - OPERATIONS.append(op.NAME) -OPERATIONS = ','.join(OPERATIONS) - - -def patch(source, no_compat=False): - args = ['script', 'mod.c', '-o', OPERATIONS] +def operations(disable=None): + if isinstance(disable, str): + disable = (disable,) + elif not disable: + disable = () + operations = ["all"] + for op in upgrade_pythoncapi.EXCLUDE_FROM_ALL: + if op.NAME in disable: + continue + operations.append(op.NAME) + for name in disable: + operations.append(f'-{name}') + operations = ','.join(operations) + return operations + + +def patch(source, no_compat=False, disable=None): + args = ['script', 'mod.c', '-o', operations(disable=disable)] if no_compat: args.append('--no-compat') @@ -113,9 +123,9 @@ def check_replace(self, source, expected, **kwargs): expected = reformat(expected) self.assertEqual(patch(source, **kwargs), expected) - def check_dont_replace(self, source): + def check_dont_replace(self, source, disable=None): source = reformat(source) - self.assertEqual(patch(source), source) + self.assertEqual(patch(source, disable=disable), source) def test_expr_regex(self): # Test EXPR_REGEX @@ -626,25 +636,40 @@ def test_py_incref_assign(self): def test_py_clear(self): self.check_replace(""" - void clear(PyObject **obj) + void clear(int test) { - // 1 - Py_DECREF(*obj); - *obj = NULL; - // 2 - Py_XDECREF(*obj); - *obj = NULL; + PyObject *obj; + + // two lines + Py_XDECREF(obj); + obj = NULL; + + // inside if + if (test) { Py_XDECREF(obj); obj = NULL; } } """, """ - void clear(PyObject **obj) + void clear(int test) { - // 1 - Py_CLEAR(*obj); - // 2 - Py_CLEAR(*obj); + PyObject *obj; + + // two lines + Py_CLEAR(obj); + + // inside if + if (test) { Py_CLEAR(obj); } } """) + # Don't replace Py_DECREF() + self.check_dont_replace(""" + void dont_clear(void) + { + PyObject *obj; + Py_DECREF(obj); + obj = NULL; + } + """, disable="Py_SETREF") + def test_py_setref(self): self.check_replace(""" void set(PyObject **obj, PyObject *t) diff --git a/upgrade_pythoncapi.py b/upgrade_pythoncapi.py index 97b9c57..5db2c98 100755 --- a/upgrade_pythoncapi.py +++ b/upgrade_pythoncapi.py @@ -353,15 +353,21 @@ class Py_INCREF_assign(Operation): class Py_CLEAR(Operation): NAME = "Py_CLEAR" REPLACE = ( - # "Py_DECREF(x); x = NULL;" => "Py_CLEAR(x)"; + # "Py_XDECREF(x); x = NULL;" => "Py_CLEAR(x)"; # The two statements must have the same indentation, otherwise the # regex does not match. (re.compile(fr'({INDENTATION_REGEX})' - + fr'Py_X?DECREF\(({EXPR_REGEX})\) *;' + + fr'Py_XDECREF\(({EXPR_REGEX})\) *;' + same_indentation(r'\1') + assign_regex_str(r'\2', r'NULL'), re.MULTILINE), r'\1Py_CLEAR(\2);'), + + # "Py_XDECREF(x); x = NULL;" => "Py_CLEAR(x)"; + (re.compile(fr'Py_XDECREF\(({EXPR_REGEX})\) *;' + + fr'{SPACE_REGEX}*' + + assign_regex_str(r'\1', r'NULL')), + r'Py_CLEAR(\1);'), )