Skip to content

Enhance NewRef operations #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 1 commit into from
Nov 10, 2022
Merged

Enhance NewRef operations #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 47 additions & 6 deletions tests/test_upgrade_pythoncapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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);
Expand All @@ -429,21 +437,43 @@ 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);
// 2
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);
}
""")
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down
60 changes: 48 additions & 12 deletions upgrade_pythoncapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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)


Expand Down Expand Up @@ -251,30 +252,66 @@ 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 = (
# "y = x; Py_INCREF(x);" => "y = Py_NewRef(x);"
# "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
Expand All @@ -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'),
Expand Down Expand Up @@ -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(',')
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down