diff --git a/tests/test_upgrade_pythoncapi.py b/tests/test_upgrade_pythoncapi.py index 33b6708..ab9a202 100644 --- a/tests/test_upgrade_pythoncapi.py +++ b/tests/test_upgrade_pythoncapi.py @@ -399,6 +399,10 @@ def test_py_incref_return(self): return obj; } + PyObject* same_line(PyObject *obj) { + Py_INCREF(obj); return obj; + } + PyObject* new_xref(PyObject *obj) { Py_XINCREF(obj); return obj; @@ -410,6 +414,10 @@ def test_py_incref_return(self): return Py_NewRef(obj); } + PyObject* same_line(PyObject *obj) { + return Py_NewRef(obj); + } + PyObject* new_xref(PyObject *obj) { return Py_XNewRef(obj); } @@ -418,7 +426,7 @@ def test_py_incref_return(self): @unittest.skipUnless(upgrade_pythoncapi.FORCE_NEWREF, 'FORCE_NEWREF=False') def test_py_incref_assign(self): self.check_replace(""" - void set_attr(MyStruct *obj, PyObject *value) + void set_attr(MyStruct *obj, PyObject *value, int test) { // 1 Py_INCREF(value); @@ -429,13 +437,25 @@ def test_py_incref_assign(self): // 3 obj->attr = value; Py_INCREF(obj->attr); - // 4 - 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" - void set_attr(MyStruct *obj, PyObject *value) + void set_attr(MyStruct *obj, PyObject *value, int test) { // 1 obj->attr = Py_NewRef(value); @@ -443,7 +463,17 @@ def test_py_incref_assign(self): obj->attr = Py_NewRef(value); // 3 obj->attr = Py_NewRef(value); - // 4 + + // 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 1 + obj->attr = Py_NewRef(value); + // cast 2 obj->attr = Py_NewRef(value); } """) @@ -476,7 +506,7 @@ def test_py_incref_assign(self): """) self.check_dont_replace(""" - void true_false(int test) + void test1(int test) { PyObject *res; if (test) @@ -487,6 +517,17 @@ def test_py_incref_assign(self): Py_DECREF(res); } + + int test2(struct datetime* result, PyObject *tzinfo) + { + int res = 0; + if (test) + res = 1; + else + Py_INCREF(tzinfo); + result->tzinfo = tzinfo; + return res; + } """) def test_py_is(self): diff --git a/upgrade_pythoncapi.py b/upgrade_pythoncapi.py index 8436422..e8eb1c0 100755 --- a/upgrade_pythoncapi.py +++ b/upgrade_pythoncapi.py @@ -46,12 +46,13 @@ def same_indentation(group): + # the regex must have re.MULTILINE flag return fr'{SPACE_REGEX}*(?:{NEWLINE_REGEX}{group})?' def get_member_regex_str(member): # Match "var->member". - return fr'\b({EXPR_REGEX}) *-> *%s\b' % member + return fr'\b({EXPR_REGEX}) *-> *{member}\b' def get_member_regex(member): @@ -67,7 +68,7 @@ def get_member_regex(member): def assign_regex_str(var, expr): # Match "var = expr;". - return (r'%s\s*=\s*%s\s*;' % (var, expr)) + return fr'{var}\s*=\s*{expr}\s*;' def set_member_regex(member): @@ -80,7 +81,7 @@ def call_assign_regex(name): # Match "Py_TYPE(expr) = expr;". # Don't match "assert(Py_TYPE(expr) == expr);". # Tolerate spaces - regex = (r'%s *\( *(.+) *\) *= *([^=].*) *;' % name) + regex = fr'{name} *\( *(.+) *\) *= *([^=].*) *;' return re.compile(regex) @@ -251,11 +252,22 @@ class Py_INCREF_return(Operation): + r'return \3;', re.MULTILINE), r'\1return Py_\2NewRef(\3);'), + + # Same regex than the previous one, + # but the two statements are on the same line. + (re.compile(fr'Py_(X?)INCREF\(({EXPR_REGEX})\)\s*;' + + fr'{SPACE_REGEX}*' + + r'return \2;', + re.MULTILINE), + r'return Py_\1NewRef(\2);'), ) # Need Py_NewRef(): new in Python 3.10 NEED_PYTHONCAPI_COMPAT = True +def optional_ptr_cast(to_type): + return fr'(?:\({to_type}\s*\*\))?' + class Py_INCREF_assign(Operation): NAME = "Py_INCREF_assign" REPLACE = ( @@ -263,18 +275,43 @@ class Py_INCREF_assign(Operation): # "y = x; Py_INCREF(y);" => "y = Py_NewRef(x);" # "y = x; Py_XINCREF(x);" => "y = Py_XNewRef(x);" # "y = x; Py_XINCREF(y);" => "y = Py_XNewRef(x);" + # "y = (PyObject*)x; Py_XINCREF(y);" => "y = Py_XNewRef(x);" # The two statements must have the same indentation, otherwise the # regex does not match. (re.compile(fr'({INDENTATION_REGEX})' - + assign_regex_str(r'(%s)' % EXPR_REGEX, r'(%s)' % EXPR_REGEX) + + assign_regex_str(fr'({EXPR_REGEX})', + optional_ptr_cast('PyObject') + fr'({EXPR_REGEX})') + same_indentation(r'\1') + r'Py_(X?)INCREF\((?:\2|\3)\);', re.MULTILINE), r'\1\2 = Py_\4NewRef(\3);'), + + # 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'{SPACE_REGEX}*' + + r'Py_(X?)INCREF\((?:\1|\2)\);'), + r'\1 = Py_\3NewRef(\2);'), + # "Py_INCREF(x); y = x;" => "y = Py_NewRef(x)" # "Py_XINCREF(x); y = x;" => "y = Py_XNewRef(x)" - (re.compile(r'Py_(X?)INCREF\((%s)\);\s*' % EXPR_REGEX - + assign_regex_str(r'(%s)' % EXPR_REGEX, r'\2')), + # The two 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') + + assign_regex_str(fr'({EXPR_REGEX})', + optional_ptr_cast('PyObject') + r'\3'), + re.MULTILINE), + r'\1\4 = Py_\2NewRef(\3);'), + + # Same regex than the previous one, + # but the two statements are on the same line. + (re.compile(fr'Py_(X?)INCREF\(({EXPR_REGEX})\);' + + fr'{SPACE_REGEX}*' + + assign_regex_str(fr'({EXPR_REGEX})', + optional_ptr_cast('PyObject') + r'\2')), r'\3 = Py_\1NewRef(\2);'), ) # Need Py_NewRef(): new in Python 3.10 @@ -292,7 +329,7 @@ def replace2(regs): return f'{x} = _Py_StealRef({y});' REPLACE = [] - expr = r'(%s)' % EXPR_REGEX + expr = fr'({EXPR_REGEX})' for name in ('None', 'True', 'False'): REPLACE.extend(( (re.compile(fr'{expr} == Py_{name}\b'), @@ -351,7 +388,7 @@ def log(self, msg=''): print(msg, file=sys.stderr, flush=True) def warning(self, msg): - self.log("WARNING: %s" % msg) + self.log(f"WARNING: {msg}") def _get_operations(self, parser): args_names = self.args.operations.split(',') @@ -381,7 +418,7 @@ def _get_operations(self, parser): operations.append(operation) if wanted: - print("invalid operations: %s" % ','.join(wanted)) + print(f"invalid operations: {','.join(wanted)}") print() self.usage(parser) sys.exit(1) @@ -479,8 +516,7 @@ def _walk_dir(self, path): empty = False if empty: - self.warning("Directory %s doesn't contain any " - "C file" % path) + self.warning(f"Directory {path} doesn't contain any C file") self.exitcode = 1 def walk(self, paths): @@ -491,7 +527,7 @@ def walk(self, paths): elif os.path.exists(path): yield path else: - self.warning("Path %s does not exist" % path) + self.warning(f"Path {path} does not exist") self.exitcode = 1 def get_latest_header(self, base_dir):