From 99e2e5a1ef0dec09ada1da37c74c3ab41e7d8b76 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Sat, 20 May 2023 11:29:23 +0100 Subject: [PATCH 1/6] Run pyupgrade --py310-plus --- Tools/clinic/clinic.py | 54 +++++++++++++++++++++--------------------- Tools/clinic/cpp.py | 2 +- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 3d4961e6e7d7dd..69a18b538769f1 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -208,7 +208,7 @@ def is_legal_py_identifier(s: str) -> bool: def ensure_legal_c_identifier(s: str) -> str: # for now, just complain if what we're given isn't legal if not is_legal_c_identifier(s): - fail("Illegal C identifier: {}".format(s)) + fail(f"Illegal C identifier: {s}") # but if we picked a C keyword, pick something else if s in c_keywords: return s + "_value" @@ -577,7 +577,7 @@ def strip_leading_and_trailing_blank_lines(s): del lines[0] return '\n'.join(lines) -@functools.lru_cache() +@functools.lru_cache def normalize_snippet(s, *, indent=0): """ Reformats s: @@ -993,7 +993,7 @@ def parser_body(prototype, *fields, declarations=''): argname_fmt = 'PyTuple_GET_ITEM(args, %d)' - left_args = "{} - {}".format(nargs, max_pos) + left_args = f"{nargs} - {max_pos}" max_args = NO_VARARG if (vararg != NO_VARARG) else max_pos parser_code = [normalize_snippet(""" if (!_PyArg_CheckPositional("{name}", %s, %d, %s)) {{ @@ -1070,14 +1070,14 @@ def parser_body(prototype, *fields, declarations=''): else: has_optional_kw = (max(pos_only, min_pos) + min_kw_only < len(converters) - int(vararg != NO_VARARG)) if vararg == NO_VARARG: - args_declaration = "_PyArg_UnpackKeywords", "%s, %s, %s" % ( + args_declaration = "_PyArg_UnpackKeywords", "{}, {}, {}".format( min_pos, max_pos, min_kw_only ) nargs = "nargs" else: - args_declaration = "_PyArg_UnpackKeywordsWithVararg", "%s, %s, %s, %s" % ( + args_declaration = "_PyArg_UnpackKeywordsWithVararg", "{}, {}, {}, {}".format( min_pos, max_pos, min_kw_only, @@ -1440,7 +1440,7 @@ def render_function(self, clinic, f): first_optional = min(first_optional, i) if p.is_vararg(): - data.cleanup.append("Py_XDECREF({});".format(c.parser_name)) + data.cleanup.append(f"Py_XDECREF({c.parser_name});") # insert group variable group = p.group @@ -1492,7 +1492,7 @@ def render_function(self, clinic, f): template_dict['c_basename'] = c_basename - methoddef_name = "{}_METHODDEF".format(c_basename.upper()) + methoddef_name = f"{c_basename.upper()}_METHODDEF" template_dict['methoddef_name'] = methoddef_name template_dict['docstring'] = self.docstring_for_c_string(f) @@ -1797,7 +1797,7 @@ def is_stop_line(line): for field in shlex.split(arguments): name, equals, value = field.partition('=') if not equals: - fail("Mangled Argument Clinic marker line: {!r}".format(line)) + fail(f"Mangled Argument Clinic marker line: {line!r}") d[name.strip()] = value.strip() if self.verify: @@ -1875,7 +1875,7 @@ def print_block(self, block, *, core_includes=False): output += '\n' write(output) - arguments="output={} input={}".format(compute_checksum(output, 16), compute_checksum(input, 16)) + arguments=f"output={compute_checksum(output, 16)} input={compute_checksum(input, 16)}" write(self.language.checksum_line.format(dsl_name=dsl_name, arguments=arguments)) write("\n") @@ -1984,7 +1984,7 @@ def dump(self): def file_changed(filename: str, new_contents: str) -> bool: """Return true if file contents changed (meaning we must update it)""" try: - with open(filename, 'r', encoding="utf-8") as fp: + with open(filename, encoding="utf-8") as fp: old_contents = fp.read() return old_contents != new_contents except FileNotFoundError: @@ -2140,7 +2140,7 @@ def parse(self, input): dsl_name = block.dsl_name if dsl_name: if dsl_name not in self.parsers: - assert dsl_name in parsers, "No parser to handle {!r} block.".format(dsl_name) + assert dsl_name in parsers, f"No parser to handle {dsl_name!r} block." self.parsers[dsl_name] = parsers[dsl_name](self) parser = self.parsers[dsl_name] try: @@ -2180,7 +2180,7 @@ def parse(self, input): "can't make directory {}!".format( destination.filename, dirname)) if self.verify: - with open(destination.filename, "rt") as f: + with open(destination.filename) as f: parser_2 = BlockParser(f.read(), language=self.language) blocks = list(parser_2) if (len(blocks) != 1) or (blocks[0].input != 'preserve\n'): @@ -2247,7 +2247,7 @@ def parse_file( except KeyError: fail("Can't identify file type for file " + repr(filename)) - with open(filename, 'r', encoding="utf-8") as f: + with open(filename, encoding="utf-8") as f: raw = f.read() # exit quickly if there are no clinic markers in the file @@ -2547,9 +2547,9 @@ def get_displayname(self, i): if i == 0: return '"argument"' if not self.is_positional_only(): - return '''"argument '{}'"'''.format(self.name) + return f'''"argument '{self.name}'"''' else: - return '"argument {}"'.format(i) + return f'"argument {i}"' class LandMine: @@ -2733,7 +2733,7 @@ def __init__(self, if isinstance(self.default_type, type): types_str = self.default_type.__name__ else: - types_str = ', '.join((cls.__name__ for cls in self.default_type)) + types_str = ', '.join(cls.__name__ for cls in self.default_type) fail("{}: default value {!r} for field {} is not of type {}".format( self.__class__.__name__, default, name, types_str)) self.default = default @@ -3965,7 +3965,7 @@ def set_template_dict(self, template_dict): ' Py_TYPE({0})->tp_new == base_tp->tp_new)' ).format(self.name) - line = '{} &&\n '.format(type_check) + line = f'{type_check} &&\n ' template_dict['self_type_check'] = line type_object = self.function.cls.type_object @@ -4021,10 +4021,10 @@ def declare(self, data): data.return_value = data.converter_retval def err_occurred_if(self, expr, data): - data.return_conversion.append('if (({}) && PyErr_Occurred()) {{\n goto exit;\n}}\n'.format(expr)) + data.return_conversion.append(f'if (({expr}) && PyErr_Occurred()) {{\n goto exit;\n}}\n') def err_occurred_if_null_pointer(self, variable, data): - data.return_conversion.append('if ({} == NULL) {{\n goto exit;\n}}\n'.format(variable)) + data.return_conversion.append(f'if ({variable} == NULL) {{\n goto exit;\n}}\n') def render(self, function, data): """ @@ -4488,13 +4488,13 @@ def state_modulename_name(self, line): c_basename = c_basename.strip() or None if not is_legal_py_identifier(full_name): - fail("Illegal function name: {}".format(full_name)) + fail(f"Illegal function name: {full_name}") if c_basename and not is_legal_c_identifier(c_basename): - fail("Illegal C basename: {}".format(c_basename)) + fail(f"Illegal C basename: {c_basename}") return_converter = None if returns: - ast_input = "def x() -> {}: pass".format(returns) + ast_input = f"def x() -> {returns}: pass" module = None try: module = ast.parse(ast_input) @@ -4707,7 +4707,7 @@ def state_parameter(self, line): module = None try: - ast_input = "def x({}): pass".format(base) + ast_input = f"def x({base}): pass" module = ast.parse(ast_input) except SyntaxError: try: @@ -4715,7 +4715,7 @@ def state_parameter(self, line): # c: int(accept={str}) # so assume there was no actual default value. default = None - ast_input = "def x({}): pass".format(line) + ast_input = f"def x({line}): pass" module = ast.parse(ast_input) except SyntaxError: pass @@ -4759,7 +4759,7 @@ def state_parameter(self, line): self.parameter_state = self.ps_optional default = default.strip() bad = False - ast_input = "x = {}".format(default) + ast_input = f"x = {default}" bad = False try: module = ast.parse(ast_input) @@ -4867,7 +4867,7 @@ def bad_node(self, node): dict = legacy_converters if legacy else converters legacy_str = "legacy " if legacy else "" if name not in dict: - fail('{} is not a valid {}converter'.format(name, legacy_str)) + fail(f'{name} is not a valid {legacy_str}converter') # if you use a c_name for the parameter, we just give that name to the converter # but the parameter object gets the python name converter = dict[name](c_name or parameter_name, parameter_name, self.function, value, **kwargs) @@ -5399,7 +5399,7 @@ def main(argv): for parameter_name, parameter in signature.parameters.items(): if parameter.kind == inspect.Parameter.KEYWORD_ONLY: if parameter.default != inspect.Parameter.empty: - s = '{}={!r}'.format(parameter_name, parameter.default) + s = f'{parameter_name}={parameter.default!r}' else: s = parameter_name parameters.append(s) diff --git a/Tools/clinic/cpp.py b/Tools/clinic/cpp.py index a3546f570c5aca..c1a2eeef22deca 100644 --- a/Tools/clinic/cpp.py +++ b/Tools/clinic/cpp.py @@ -185,7 +185,7 @@ def pop_stack() -> TokenAndCondition: if __name__ == '__main__': for filename in sys.argv[1:]: - with open(filename, "rt") as f: + with open(filename) as f: cpp = Monitor(filename, verbose=True) print() print(filename) From bb5bcf22bb1c82ef1c86440e9534ef7f876e6a59 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Sat, 20 May 2023 11:30:41 +0100 Subject: [PATCH 2/6] Revert a change that made it less readable --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 69a18b538769f1..fdfdb99f773100 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1070,14 +1070,14 @@ def parser_body(prototype, *fields, declarations=''): else: has_optional_kw = (max(pos_only, min_pos) + min_kw_only < len(converters) - int(vararg != NO_VARARG)) if vararg == NO_VARARG: - args_declaration = "_PyArg_UnpackKeywords", "{}, {}, {}".format( + args_declaration = "_PyArg_UnpackKeywords", "%s, %s, %s" % ( min_pos, max_pos, min_kw_only ) nargs = "nargs" else: - args_declaration = "_PyArg_UnpackKeywordsWithVararg", "{}, {}, {}, {}".format( + args_declaration = "_PyArg_UnpackKeywordsWithVararg", "%s, %s, %s, %s" % ( min_pos, max_pos, min_kw_only, From 841c7f7ef93069983b73fbf891e6e4db9dec93ab Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 20 May 2023 19:07:01 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Tools/clinic/clinic.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2d6efd0f7f8141..ab71daae258ecb 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -208,7 +208,7 @@ def is_legal_py_identifier(s: str) -> bool: def ensure_legal_c_identifier(s: str) -> str: # for now, just complain if what we're given isn't legal if not is_legal_c_identifier(s): - fail(f"Illegal C identifier: {s}") + fail("Illegal C identifier:", s) # but if we picked a C keyword, pick something else if s in c_keywords: return s + "_value" @@ -1487,8 +1487,7 @@ def render_function(self, clinic, f): template_dict['c_basename'] = c_basename - methoddef_name = f"{c_basename.upper()}_METHODDEF" - template_dict['methoddef_name'] = methoddef_name + template_dict['methoddef_name'] = c_basename.upper() + "_METHODDEF" template_dict['docstring'] = self.docstring_for_c_string(f) @@ -1867,7 +1866,10 @@ def print_block(self, block, *, core_includes=False): output += '\n' write(output) - arguments=f"output={compute_checksum(output, 16)} input={compute_checksum(input, 16)}" + arguments="output={output}, input={input}".format( + output=compute_checksum(output, 16), + input=compute_checksum(input, 16) + ) write(self.language.checksum_line.format(dsl_name=dsl_name, arguments=arguments)) write("\n") @@ -2537,7 +2539,7 @@ def get_displayname(self, i): if i == 0: return '"argument"' if not self.is_positional_only(): - return f'''"argument '{self.name}'"''' + return f'"argument {self.name!r}"' else: return f'"argument {i}"' @@ -2723,7 +2725,8 @@ def __init__(self, if isinstance(self.default_type, type): types_str = self.default_type.__name__ else: - types_str = ', '.join(cls.__name__ for cls in self.default_type) + names = (cls.__name__ for cls in self.default_type) + types_str = ', '.join(names) fail("{}: default value {!r} for field {} is not of type {}".format( self.__class__.__name__, default, name, types_str)) self.default = default @@ -4011,10 +4014,12 @@ def declare(self, data): data.return_value = data.converter_retval def err_occurred_if(self, expr, data): - data.return_conversion.append(f'if (({expr}) && PyErr_Occurred()) {{\n goto exit;\n}}\n') + line = f'if (({expr}) && PyErr_Occurred()) {{\n goto exit;\n}}\n' + data.return_conversion.append(line) def err_occurred_if_null_pointer(self, variable, data): - data.return_conversion.append(f'if ({variable} == NULL) {{\n goto exit;\n}}\n') + line = f'if ({variable} == NULL) {{\n goto exit;\n}}\n' + data.return_conversion.append(line) def render(self, function, data): """ @@ -4477,9 +4482,9 @@ def state_modulename_name(self, line): c_basename = c_basename.strip() or None if not is_legal_py_identifier(full_name): - fail(f"Illegal function name: {full_name}") + fail("Illegal function name:", full_name) if c_basename and not is_legal_c_identifier(c_basename): - fail(f"Illegal C basename: {c_basename}") + fail("Illegal C basename:", c_basename) return_converter = None if returns: @@ -4749,7 +4754,6 @@ def state_parameter(self, line): default = default.strip() bad = False ast_input = f"x = {default}" - bad = False try: module = ast.parse(ast_input) From a8c265f3fd53548eb6a8c6a7a3035684654ef6e1 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 20 May 2023 19:15:50 +0100 Subject: [PATCH 4/6] smol fixes --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index ab71daae258ecb..62424fd77932bf 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1866,7 +1866,7 @@ def print_block(self, block, *, core_includes=False): output += '\n' write(output) - arguments="output={output}, input={input}".format( + arguments="output={output} input={input}".format( output=compute_checksum(output, 16), input=compute_checksum(input, 16) ) @@ -2725,7 +2725,7 @@ def __init__(self, if isinstance(self.default_type, type): types_str = self.default_type.__name__ else: - names = (cls.__name__ for cls in self.default_type) + names = [cls.__name__ for cls in self.default_type] types_str = ', '.join(names) fail("{}: default value {!r} for field {} is not of type {}".format( self.__class__.__name__, default, name, types_str)) From ee83e294b7da0a141340a88890ee3b4c0dadf05e Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 20 May 2023 20:48:07 +0100 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 62424fd77932bf..49e564fee67ef1 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1790,7 +1790,7 @@ def is_stop_line(line): for field in shlex.split(arguments): name, equals, value = field.partition('=') if not equals: - fail(f"Mangled Argument Clinic marker line: {line!r}") + fail("Mangled Argument Clinic marker line:", repr(line)) d[name.strip()] = value.strip() if self.verify: @@ -1866,7 +1866,7 @@ def print_block(self, block, *, core_includes=False): output += '\n' write(output) - arguments="output={output} input={input}".format( + arguments = "output={output} input={input}".format( output=compute_checksum(output, 16), input=compute_checksum(input, 16) ) From 22b167354f781ae3eb4c7101e81553a45fe651d5 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 20 May 2023 20:56:41 +0100 Subject: [PATCH 6/6] Add back parens to lru_cache --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 49e564fee67ef1..41e08d15436b11 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -575,7 +575,7 @@ def strip_leading_and_trailing_blank_lines(s): del lines[0] return '\n'.join(lines) -@functools.lru_cache +@functools.lru_cache() def normalize_snippet(s, *, indent=0): """ Reformats s: