From 6f4ab6e0826d83236e754a0feb8004f1e4995b44 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 01:52:59 +0900 Subject: [PATCH 01/17] gh-104504: Run mypy on cases_generator --- .github/workflows/mypy.yml | 17 +++++++- Tools/cases_generator/analysis.py | 4 -- Tools/cases_generator/flags.py | 10 ++--- Tools/cases_generator/formatting.py | 10 ++--- Tools/cases_generator/generate_cases.py | 51 +++++++--------------- Tools/cases_generator/lexer.py | 28 +++++++----- Tools/cases_generator/parsing.py | 4 +- Tools/cases_generator/requirements-dev.txt | 2 + Tools/cases_generator/stacking.py | 4 +- 9 files changed, 65 insertions(+), 65 deletions(-) create mode 100644 Tools/cases_generator/requirements-dev.txt diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 1315bb5a966f01..54766e11285f86 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -8,6 +8,7 @@ on: pull_request: paths: - "Tools/clinic/**" + - "Tools/cases_generator/**" - ".github/workflows/mypy.yml" workflow_dispatch: @@ -24,7 +25,7 @@ concurrency: cancel-in-progress: true jobs: - mypy: + mypy-clinic: name: Run mypy on Tools/clinic/ runs-on: ubuntu-latest timeout-minutes: 10 @@ -37,3 +38,17 @@ jobs: cache-dependency-path: Tools/clinic/requirements-dev.txt - run: pip install -r Tools/clinic/requirements-dev.txt - run: mypy --config-file Tools/clinic/mypy.ini + + mypy-cases-generator: + name: Run mypy on Tools/cases_generator/ + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: "3.x" + cache: pip + cache-dependency-path: Tools/cases_generator/requirements-dev.txt + - run: pip install -r Tools/cases_generator/requirements-dev.txt + - run: mypy --config-file Tools/cases_generator/mypy.ini diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 48f2db981c95b6..84a25e25c215e4 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -171,8 +171,6 @@ def parse_file(self, filename: str, instrs_idx: dict[str, int]) -> None: case parsing.Pseudo(name): self.pseudos[name] = thing self.everything.append(thing) - case _: - typing.assert_never(thing) if not psr.eof(): raise psr.make_syntax_error(f"Extra stuff at the end of {filename}") @@ -408,6 +406,4 @@ def check_macro_components( components.append(self.instrs[name]) case parsing.CacheEffect(): components.append(uop) - case _: - typing.assert_never(uop) return components diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 962f003b194dbd..79b88cc6dd8bdb 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -16,11 +16,11 @@ class InstructionFlags: HAS_FREE_FLAG: bool HAS_LOCAL_FLAG: bool - def __post_init__(self): + def __post_init__(self) -> None: self.bitmask = {name: (1 << i) for i, name in enumerate(self.names())} @staticmethod - def fromInstruction(instr: parsing.Node): + def fromInstruction(instr: parsing.Node) -> 'InstructionFlags': has_free = ( variable_used(instr, "PyCell_New") @@ -41,7 +41,7 @@ def fromInstruction(instr: parsing.Node): ) @staticmethod - def newEmpty(): + def newEmpty() -> 'InstructionFlags': return InstructionFlags(False, False, False, False, False, False) def add(self, other: "InstructionFlags") -> None: @@ -49,7 +49,7 @@ def add(self, other: "InstructionFlags") -> None: if value: setattr(self, name, value) - def names(self, value=None) -> list[str]: + def names(self, value: bool|None = None) -> list[str]: if value is None: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] @@ -62,7 +62,7 @@ def bitmap(self) -> int: return flags @classmethod - def emit_macros(cls, out: Formatter): + def emit_macros(cls, out: Formatter) -> None: flags = cls.newEmpty() for name, value in flags.bitmask.items(): out.emit(f"#define {name} ({value})") diff --git a/Tools/cases_generator/formatting.py b/Tools/cases_generator/formatting.py index 5894751bd9635c..d3a6da92def531 100644 --- a/Tools/cases_generator/formatting.py +++ b/Tools/cases_generator/formatting.py @@ -58,13 +58,13 @@ def reset_lineno(self) -> None: self.set_lineno(self.lineno + 1, self.filename) @contextlib.contextmanager - def indent(self): + def indent(self) -> typing.Iterator[None]: self.prefix += " " yield self.prefix = self.prefix[:-4] @contextlib.contextmanager - def block(self, head: str, tail: str = ""): + def block(self, head: str, tail: str = "") -> typing.Iterator[None]: if head: self.emit(head + " {") else: @@ -77,7 +77,7 @@ def stack_adjust( self, input_effects: list[StackEffect], output_effects: list[StackEffect], - ): + ) -> None: shrink, isym = list_effect_size(input_effects) grow, osym = list_effect_size(output_effects) diff = grow - shrink @@ -90,7 +90,7 @@ def stack_adjust( if osym and osym != isym: self.emit(f"STACK_GROW({osym});") - def declare(self, dst: StackEffect, src: StackEffect | None): + def declare(self, dst: StackEffect, src: StackEffect | None) -> None: if dst.name == UNUSED or dst.cond == "0": return typ = f"{dst.type}" if dst.type else "PyObject *" @@ -107,7 +107,7 @@ def declare(self, dst: StackEffect, src: StackEffect | None): sepa = "" if typ.endswith("*") else " " self.emit(f"{typ}{sepa}{dst.name}{init};") - def assign(self, dst: StackEffect, src: StackEffect): + def assign(self, dst: StackEffect, src: StackEffect) -> None: if src.name == UNUSED or dst.name == UNUSED: return cast = self.cast(dst, src) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index f31b6658d6599e..11f6cb3a5c9b64 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -23,7 +23,6 @@ MacroInstruction, MacroParts, PseudoInstruction, - StackEffect, OverriddenInstructionPlaceHolder, TIER_ONE, TIER_TWO, @@ -183,17 +182,11 @@ def effect_str(effects: list[StackEffect]) -> str: assert target_instr target_popped = effect_str(target_instr.input_effects) target_pushed = effect_str(target_instr.output_effects) - if popped is None and pushed is None: - popped, pushed = target_popped, target_pushed - else: - assert popped == target_popped - assert pushed == target_pushed - case _: - typing.assert_never(thing) + popped, pushed = target_popped, target_pushed return instr, popped, pushed @contextlib.contextmanager - def metadata_item(self, signature, open, close): + def metadata_item(self, signature: str, open: str, close: str) -> typing.Iterator[None]: self.out.emit("") self.out.emit(f"extern {signature};") self.out.emit("#ifdef NEED_OPCODE_METADATA") @@ -243,21 +236,21 @@ def from_source_files(self) -> str: paths = f"\n{self.out.comment} ".join(filenames) return f"{self.out.comment} from:\n{self.out.comment} {paths}\n" - def write_provenance_header(self): + def write_provenance_header(self) -> None: self.out.write_raw(f"{self.out.comment} This file is generated by {THIS}\n") self.out.write_raw(self.from_source_files()) self.out.write_raw(f"{self.out.comment} Do not edit!\n") - def assign_opcode_ids(self): + def assign_opcode_ids(self) -> None: """Assign IDs to opcodes""" - ops: list[(bool, str)] = [] # (has_arg, name) for each opcode + ops: list[tuple[bool, str]] = [] # (has_arg, name) for each opcode instrumented_ops: list[str] = [] for instr in itertools.chain( [instr for instr in self.instrs.values() if instr.kind != "op"], self.macro_instrs.values()): - + assert isinstance(instr, (Instruction, MacroInstruction, PseudoInstruction)) name = instr.name if name.startswith('INSTRUMENTED_'): instrumented_ops.append(name) @@ -274,11 +267,11 @@ def assign_opcode_ids(self): assert len(set(ops)) == len(ops) assert len(set(instrumented_ops)) == len(instrumented_ops) - opname: list[str or None] = [None] * 512 - opmap: dict = {} - markers: dict = {} + opname: list[str|None] = [None] * 512 + opmap: dict[str, int] = {} + markers: dict[str, int] = {} - def map_op(op, name): + def map_op(op: int, name: str) -> None: assert op < len(opname) assert opname[op] is None assert name not in opmap @@ -320,11 +313,11 @@ def map_op(op, name): for i, op in enumerate(sorted(self.pseudos)): map_op(256 + i, op) - assert 255 not in opmap # 255 is reserved + assert 255 not in opmap.values() # 255 is reserved self.opmap = opmap self.markers = markers - def write_opcode_ids(self, opcode_ids_h_filename, opcode_targets_filename): + def write_opcode_ids(self, opcode_ids_h_filename: str, opcode_targets_filename: str) -> None: """Write header file that defined the opcode IDs""" with open(opcode_ids_h_filename, "w") as f: @@ -342,10 +335,10 @@ def write_opcode_ids(self, opcode_ids_h_filename, opcode_targets_filename): self.out.emit("") self.out.emit("/* Instruction opcodes for compiled code */") - def define(name, opcode): + def define(name: str, opcode: int) -> None: self.out.emit(f"#define {name:<38} {opcode:>3}") - all_pairs = [] + all_pairs: list[tuple[int, int, str]] = [] # the second item in the tuple sorts the markers before the ops all_pairs.extend((i, 1, name) for (name, i) in self.markers.items()) all_pairs.extend((i, 2, name) for (name, i) in self.opmap.items()) @@ -392,11 +385,7 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No assert target_instr if format is None: format = target_instr.instr_fmt - else: - assert format == target_instr.instr_fmt assert format is not None - case _: - typing.assert_never(thing) all_formats.add(format) # Turn it into a sorted list of enum values. @@ -483,8 +472,6 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No self.write_metadata_for_macro(self.macro_instrs[thing.name]) case parsing.Pseudo(): self.write_metadata_for_pseudo(self.pseudo_instrs[thing.name]) - case _: - typing.assert_never(thing) with self.metadata_item( "const struct opcode_macro_expansion " @@ -520,8 +507,6 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No ) case parsing.Pseudo(): pass - case _: - typing.assert_never(thing) with self.metadata_item( "const char * const _PyOpcode_uop_name[OPCODE_UOP_NAME_SIZE]", "=", ";" @@ -770,8 +755,6 @@ def write_instructions( # self.write_macro(self.macro_instrs[thing.name]) case parsing.Pseudo(): pass - case _: - typing.assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_macros} macros " @@ -814,8 +797,6 @@ def write_executor_instructions( pass case parsing.Pseudo(): pass - case _: - typing.assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_uops} ops to {executor_filename}", file=sys.stderr, @@ -843,8 +824,6 @@ def write_abstract_interpreter_instructions( pass case parsing.Pseudo(): pass - case _: - typing.assert_never(thing) print( f"Wrote some stuff to {abstract_interpreter_filename}", file=sys.stderr, @@ -878,7 +857,7 @@ def write_instr(self, instr: Instruction) -> None: self.out.emit(f"DISPATCH();") -def main(): +def main() -> None: """Parse command line, parse input, analyze, write output.""" args = arg_parser.parse_args() # Prints message and sys.exit(2) on error if len(args.input) == 0: diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index fe9c05ede5aa47..d0e3be9051caf1 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -4,8 +4,10 @@ import re from dataclasses import dataclass +from collections.abc import Iterator -def choice(*opts): + +def choice(*opts: str) -> str: return "|".join("(%s)" % opt for opt in opts) # Regexes @@ -123,13 +125,19 @@ def choice(*opts): 'SWITCH', 'TYPEDEF', 'UNION', 'UNSIGNED', 'VOID', 'VOLATILE', 'WHILE' ) + +# For mypy +REGISTER = 'REGISTER' +OVERRIDE = 'OVERRIDE' +IF = 'IF' + for name in kwds: globals()[name] = name keywords = { name.lower() : name for name in kwds } def make_syntax_error( - message: str, filename: str, line: int, column: int, line_text: str, + message: str, filename: str | None, line: int, column: int, line_text: str, ) -> SyntaxError: return SyntaxError(message, (filename, line, column, line_text)) @@ -142,30 +150,30 @@ class Token: end: tuple[int, int] @property - def line(self): + def line(self) -> int: return self.begin[0] @property - def column(self): + def column(self) -> int: return self.begin[1] @property - def end_line(self): + def end_line(self) -> int: return self.end[0] @property - def end_column(self): + def end_column(self) -> int: return self.end[1] @property - def width(self): + def width(self) -> int: return self.end[1] - self.begin[1] - def replaceText(self, txt): + def replaceText(self, txt: str) -> 'Token': assert isinstance(txt, str) return Token(self.kind, txt, self.begin, self.end) - def __repr__(self): + def __repr__(self) -> str: b0, b1 = self.begin e0, e1 = self.end if b0 == e0: @@ -174,7 +182,7 @@ def __repr__(self): return f"{self.kind}({self.text!r}, {b0}:{b1}, {e0}:{e1})" -def tokenize(src, line=1, filename=None): +def tokenize(src: str, line: int = 1, filename: str | None = None) -> Iterator[Token]: linestart = -1 for m in matcher.finditer(src): start, end = m.span() diff --git a/Tools/cases_generator/parsing.py b/Tools/cases_generator/parsing.py index cdd20d7a0b3f59..25de3a5adc2cf9 100644 --- a/Tools/cases_generator/parsing.py +++ b/Tools/cases_generator/parsing.py @@ -32,7 +32,7 @@ class Context(NamedTuple): end: int owner: PLexer - def __repr__(self): + def __repr__(self) -> str: return f"<{self.owner.filename}: {self.begin}-{self.end}>" @@ -75,7 +75,7 @@ class StackEffect(Node): size: str = "" # Optional `[size]` # Note: size cannot be combined with type or cond - def __repr__(self): + def __repr__(self) -> str: items = [self.name, self.type, self.cond, self.size] while items and items[-1] == "": del items[-1] diff --git a/Tools/cases_generator/requirements-dev.txt b/Tools/cases_generator/requirements-dev.txt new file mode 100644 index 00000000000000..e9529f3527e95e --- /dev/null +++ b/Tools/cases_generator/requirements-dev.txt @@ -0,0 +1,2 @@ +# Requirements file for external linters and checks we run on Tools/clinic/ in CI +mypy==1.4.1 diff --git a/Tools/cases_generator/stacking.py b/Tools/cases_generator/stacking.py index 632298a567dd40..fcf1ec044ccb21 100644 --- a/Tools/cases_generator/stacking.py +++ b/Tools/cases_generator/stacking.py @@ -415,7 +415,7 @@ def write_components( def write_single_instr_for_abstract_interp( instr: Instruction, out: Formatter -): +) -> None: try: _write_components_for_abstract_interp( [Component(instr, instr.active_caches)], @@ -428,7 +428,7 @@ def write_single_instr_for_abstract_interp( def _write_components_for_abstract_interp( parts: list[Component], out: Formatter, -): +) -> None: managers = get_managers(parts) for mgr in managers: if mgr is managers[-1]: From c0fb645fb231929fff0ed1ee83dc075f5b774c0c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 01:54:09 +0900 Subject: [PATCH 02/17] Add mypy.ini --- Tools/cases_generator/mypy.ini | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 Tools/cases_generator/mypy.ini diff --git a/Tools/cases_generator/mypy.ini b/Tools/cases_generator/mypy.ini new file mode 100644 index 00000000000000..7f56e66fe90232 --- /dev/null +++ b/Tools/cases_generator/mypy.ini @@ -0,0 +1,11 @@ +[mypy] +files = Tools/cases_generator/ +pretty = True + +python_version = 3.10 + +# and be strict! +strict = True +strict_concatenate = True +enable_error_code = ignore-without-code,redundant-expr,truthy-bool +warn_unreachable = True From 840dab9f6dffbd9661379b7fba24229a2e536087 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:11:25 +0900 Subject: [PATCH 03/17] Address code review --- .github/workflows/mypy.yml | 10 +- Tools/cases_generator/analysis.py | 4 + Tools/cases_generator/flags.py | 12 +- Tools/cases_generator/formatting.py | 5 +- Tools/cases_generator/generate_cases.py | 73 +++-- Tools/cases_generator/lexer.py | 315 +++++++++++++-------- Tools/cases_generator/mypy.ini | 2 +- Tools/cases_generator/requirements-dev.txt | 2 - Tools/cases_generator/stacking.py | 14 +- Tools/clinic/requirements-dev.txt | 2 - Tools/requirements-dev.txt | 4 + 11 files changed, 273 insertions(+), 170 deletions(-) delete mode 100644 Tools/cases_generator/requirements-dev.txt delete mode 100644 Tools/clinic/requirements-dev.txt create mode 100644 Tools/requirements-dev.txt diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 54766e11285f86..742fd146c43ec1 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -35,8 +35,8 @@ jobs: with: python-version: "3.x" cache: pip - cache-dependency-path: Tools/clinic/requirements-dev.txt - - run: pip install -r Tools/clinic/requirements-dev.txt + cache-dependency-path: Tools/requirements-dev.txt + - run: pip install -r Tools/requirements-dev.txt - run: mypy --config-file Tools/clinic/mypy.ini mypy-cases-generator: @@ -49,6 +49,8 @@ jobs: with: python-version: "3.x" cache: pip - cache-dependency-path: Tools/cases_generator/requirements-dev.txt - - run: pip install -r Tools/cases_generator/requirements-dev.txt + cache-dependency-path: Tools/requirements-dev.txt + - run: pip install -r Tools/requirements-dev.txt + - name: Lint with black + run: black Tools/cases_generator - run: mypy --config-file Tools/cases_generator/mypy.ini diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 84a25e25c215e4..48f2db981c95b6 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -171,6 +171,8 @@ def parse_file(self, filename: str, instrs_idx: dict[str, int]) -> None: case parsing.Pseudo(name): self.pseudos[name] = thing self.everything.append(thing) + case _: + typing.assert_never(thing) if not psr.eof(): raise psr.make_syntax_error(f"Extra stuff at the end of {filename}") @@ -406,4 +408,6 @@ def check_macro_components( components.append(self.instrs[name]) case parsing.CacheEffect(): components.append(uop) + case _: + typing.assert_never(uop) return components diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 79b88cc6dd8bdb..c82f94a5271e4c 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -20,7 +20,7 @@ def __post_init__(self) -> None: self.bitmask = {name: (1 << i) for i, name in enumerate(self.names())} @staticmethod - def fromInstruction(instr: parsing.Node) -> 'InstructionFlags': + def fromInstruction(instr: parsing.Node) -> "InstructionFlags": has_free = ( variable_used(instr, "PyCell_New") @@ -41,7 +41,7 @@ def fromInstruction(instr: parsing.Node) -> 'InstructionFlags': ) @staticmethod - def newEmpty() -> 'InstructionFlags': + def newEmpty() -> "InstructionFlags": return InstructionFlags(False, False, False, False, False, False) def add(self, other: "InstructionFlags") -> None: @@ -49,7 +49,7 @@ def add(self, other: "InstructionFlags") -> None: if value: setattr(self, name, value) - def names(self, value: bool|None = None) -> list[str]: + def names(self, value: bool | None = None) -> list[str]: if value is None: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] @@ -90,9 +90,9 @@ def variable_used_unspecialized(node: parsing.Node, name: str) -> bool: text = "".join(token.text.split()) # TODO: Handle nested #if if text == "#if": - if ( - i + 1 < len(node.tokens) - and node.tokens[i + 1].text in ("ENABLE_SPECIALIZATION", "TIER_ONE") + if i + 1 < len(node.tokens) and node.tokens[i + 1].text in ( + "ENABLE_SPECIALIZATION", + "TIER_ONE", ): skipping = True elif text in ("#else", "#endif"): diff --git a/Tools/cases_generator/formatting.py b/Tools/cases_generator/formatting.py index d3a6da92def531..65ed5d7d042ef9 100644 --- a/Tools/cases_generator/formatting.py +++ b/Tools/cases_generator/formatting.py @@ -1,3 +1,4 @@ +import collections import contextlib import re import typing @@ -58,13 +59,13 @@ def reset_lineno(self) -> None: self.set_lineno(self.lineno + 1, self.filename) @contextlib.contextmanager - def indent(self) -> typing.Iterator[None]: + def indent(self) -> collections.abc.Iterator[None]: self.prefix += " " yield self.prefix = self.prefix[:-4] @contextlib.contextmanager - def block(self, head: str, tail: str = "") -> typing.Iterator[None]: + def block(self, head: str, tail: str = "") -> collections.abc.Iterator[None]: if head: self.emit(head + " {") else: diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 11f6cb3a5c9b64..5b4c61ceed3181 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -79,12 +79,10 @@ "STORE_FAST", "STORE_FAST_MAYBE_NULL", "COPY", - # Arithmetic "_BINARY_OP_MULTIPLY_INT", "_BINARY_OP_ADD_INT", "_BINARY_OP_SUBTRACT_INT", - } arg_parser = argparse.ArgumentParser( @@ -143,6 +141,7 @@ default=DEFAULT_ABSTRACT_INTERPRETER_OUTPUT, ) + class Generator(Analyzer): def get_stack_effect_info( self, thing: parsing.InstDef | parsing.Macro | parsing.Pseudo @@ -183,10 +182,14 @@ def effect_str(effects: list[StackEffect]) -> str: target_popped = effect_str(target_instr.input_effects) target_pushed = effect_str(target_instr.output_effects) popped, pushed = target_popped, target_pushed + case _: + typing.assert_never(thing) return instr, popped, pushed @contextlib.contextmanager - def metadata_item(self, signature: str, open: str, close: str) -> typing.Iterator[None]: + def metadata_item( + self, signature: str, open: str, close: str + ) -> typing.Iterator[None]: self.out.emit("") self.out.emit(f"extern {signature};") self.out.emit("#ifdef NEED_OPCODE_METADATA") @@ -211,7 +214,9 @@ def write_function( ) -> None: with self.metadata_item( - f"int _PyOpcode_num_{direction}(int opcode, int oparg, bool jump)", "", "" + f"int _PyOpcode_num_{direction}(int opcode, int oparg, bool jump)", + "", + "", ): with self.out.block("switch(opcode)"): for instr, effect in data: @@ -249,10 +254,11 @@ def assign_opcode_ids(self) -> None: for instr in itertools.chain( [instr for instr in self.instrs.values() if instr.kind != "op"], - self.macro_instrs.values()): + self.macro_instrs.values(), + ): assert isinstance(instr, (Instruction, MacroInstruction, PseudoInstruction)) name = instr.name - if name.startswith('INSTRUMENTED_'): + if name.startswith("INSTRUMENTED_"): instrumented_ops.append(name) else: ops.append((instr.instr_flags.HAS_ARG_FLAG, name)) @@ -261,13 +267,13 @@ def assign_opcode_ids(self) -> None: # rather than bytecodes.c, so we need to add it explicitly # here (at least until we add something to bytecodes.c to # declare external instructions). - instrumented_ops.append('INSTRUMENTED_LINE') + instrumented_ops.append("INSTRUMENTED_LINE") # assert lists are unique assert len(set(ops)) == len(ops) assert len(set(instrumented_ops)) == len(instrumented_ops) - opname: list[str|None] = [None] * 512 + opname: list[str | None] = [None] * 512 opmap: dict[str, int] = {} markers: dict[str, int] = {} @@ -278,16 +284,15 @@ def map_op(op: int, name: str) -> None: opname[op] = name opmap[name] = op - # 0 is reserved for cache entries. This helps debugging. - map_op(0, 'CACHE') + map_op(0, "CACHE") # 17 is reserved as it is the initial value for the specializing counter. # This helps catch cases where we attempt to execute a cache. - map_op(17, 'RESERVED') + map_op(17, "RESERVED") # 166 is RESUME - it is hard coded as such in Tools/build/deepfreeze.py - map_op(166, 'RESUME') + map_op(166, "RESUME") next_opcode = 1 @@ -299,13 +304,13 @@ def map_op(op: int, name: str) -> None: assert next_opcode < 255 map_op(next_opcode, name) - if has_arg and 'HAVE_ARGUMENT' not in markers: - markers['HAVE_ARGUMENT'] = next_opcode + if has_arg and "HAVE_ARGUMENT" not in markers: + markers["HAVE_ARGUMENT"] = next_opcode # Instrumented opcodes are at the end of the valid range min_instrumented = 254 - (len(instrumented_ops) - 1) assert next_opcode <= min_instrumented - markers['MIN_INSTRUMENTED_OPCODE'] = min_instrumented + markers["MIN_INSTRUMENTED_OPCODE"] = min_instrumented for i, op in enumerate(instrumented_ops): map_op(min_instrumented + i, op) @@ -317,7 +322,9 @@ def map_op(op: int, name: str) -> None: self.opmap = opmap self.markers = markers - def write_opcode_ids(self, opcode_ids_h_filename: str, opcode_targets_filename: str) -> None: + def write_opcode_ids( + self, opcode_ids_h_filename: str, opcode_targets_filename: str + ) -> None: """Write header file that defined the opcode IDs""" with open(opcode_ids_h_filename, "w") as f: @@ -330,7 +337,7 @@ def write_opcode_ids(self, opcode_ids_h_filename: str, opcode_targets_filename: self.out.emit("#ifndef Py_OPCODE_IDS_H") self.out.emit("#define Py_OPCODE_IDS_H") self.out.emit("#ifdef __cplusplus") - self.out.emit("extern \"C\" {") + self.out.emit('extern "C" {') self.out.emit("#endif") self.out.emit("") self.out.emit("/* Instruction opcodes for compiled code */") @@ -363,7 +370,6 @@ def define(name: str, opcode: int) -> None: targets[op] = f"TARGET_{name}" f.write(",\n".join([f" &&{s}" for s in targets])) - def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> None: """Write instruction metadata to output file.""" @@ -458,7 +464,7 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No "const struct opcode_metadata " "_PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE]", "=", - ";" + ";", ): # Write metadata for each instruction for thing in self.everything: @@ -471,13 +477,15 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No case parsing.Macro(): self.write_metadata_for_macro(self.macro_instrs[thing.name]) case parsing.Pseudo(): - self.write_metadata_for_pseudo(self.pseudo_instrs[thing.name]) + self.write_metadata_for_pseudo( + self.pseudo_instrs[thing.name] + ) with self.metadata_item( "const struct opcode_macro_expansion " "_PyOpcode_macro_expansion[OPCODE_MACRO_EXPANSION_SIZE]", "=", - ";" + ";", ): # Write macro expansion for each non-pseudo instruction for thing in self.everything: @@ -514,7 +522,9 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No self.write_uop_items(lambda name, counter: f'[{name}] = "{name}",') with self.metadata_item( - f"const char *const _PyOpcode_OpName[{1 + max(self.opmap.values())}]", "=", ";" + f"const char *const _PyOpcode_OpName[{1 + max(self.opmap.values())}]", + "=", + ";", ): for name in self.opmap: self.out.emit(f'[{name}] = "{name}",') @@ -527,11 +537,9 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No for m in family.members: deoptcodes[m] = name # special case: - deoptcodes['BINARY_OP_INPLACE_ADD_UNICODE'] = 'BINARY_OP' + deoptcodes["BINARY_OP_INPLACE_ADD_UNICODE"] = "BINARY_OP" - with self.metadata_item( - f"const uint8_t _PyOpcode_Deopt[256]", "=", ";" - ): + with self.metadata_item(f"const uint8_t _PyOpcode_Deopt[256]", "=", ";"): for opt, deopt in sorted(deoptcodes.items()): self.out.emit(f"[{opt}] = {deopt},") @@ -589,10 +597,9 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No if name not in specialized_ops: self.out.emit(f"'{name}': {op},") - for name in ['MIN_INSTRUMENTED_OPCODE', 'HAVE_ARGUMENT']: + for name in ["MIN_INSTRUMENTED_OPCODE", "HAVE_ARGUMENT"]: self.out.emit(f"{name} = {self.markers[name]}") - def write_pseudo_instrs(self) -> None: """Write the IS_PSEUDO_INSTR macro""" self.out.emit("\n\n#define IS_PSEUDO_INSTR(OP) ( \\") @@ -815,7 +822,10 @@ def write_abstract_interpreter_instructions( pass case parsing.InstDef(): instr = AbstractInstruction(self.instrs[thing.name].inst) - if instr.is_viable_uop() and instr.name not in SPECIALLY_HANDLED_ABSTRACT_INSTR: + if ( + instr.is_viable_uop() + and instr.name not in SPECIALLY_HANDLED_ABSTRACT_INSTR + ): self.out.emit("") with self.out.block(f"case {thing.name}:"): instr.write(self.out, tier=TIER_TWO) @@ -878,8 +888,9 @@ def main() -> None: a.write_opcode_ids(args.opcode_ids_h, args.opcode_targets_h) a.write_metadata(args.metadata, args.pymetadata) a.write_executor_instructions(args.executor_cases, args.emit_line_directives) - a.write_abstract_interpreter_instructions(args.abstract_interpreter_cases, - args.emit_line_directives) + a.write_abstract_interpreter_instructions( + args.abstract_interpreter_cases, args.emit_line_directives + ) if __name__ == "__main__": diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index d0e3be9051caf1..a60f6c11a4c460 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -10,134 +10,215 @@ def choice(*opts: str) -> str: return "|".join("(%s)" % opt for opt in opts) + # Regexes # Longer operators must go before shorter ones. -PLUSPLUS = r'\+\+' -MINUSMINUS = r'--' +PLUSPLUS = r"\+\+" +MINUSMINUS = r"--" # -> -ARROW = r'->' -ELLIPSIS = r'\.\.\.' +ARROW = r"->" +ELLIPSIS = r"\.\.\." # Assignment operators -TIMESEQUAL = r'\*=' -DIVEQUAL = r'/=' -MODEQUAL = r'%=' -PLUSEQUAL = r'\+=' -MINUSEQUAL = r'-=' -LSHIFTEQUAL = r'<<=' -RSHIFTEQUAL = r'>>=' -ANDEQUAL = r'&=' -OREQUAL = r'\|=' -XOREQUAL = r'\^=' +TIMESEQUAL = r"\*=" +DIVEQUAL = r"/=" +MODEQUAL = r"%=" +PLUSEQUAL = r"\+=" +MINUSEQUAL = r"-=" +LSHIFTEQUAL = r"<<=" +RSHIFTEQUAL = r">>=" +ANDEQUAL = r"&=" +OREQUAL = r"\|=" +XOREQUAL = r"\^=" # Operators -PLUS = r'\+' -MINUS = r'-' -TIMES = r'\*' -DIVIDE = r'/' -MOD = r'%' -NOT = r'~' -XOR = r'\^' -LOR = r'\|\|' -LAND = r'&&' -LSHIFT = r'<<' -RSHIFT = r'>>' -LE = r'<=' -GE = r'>=' -EQ = r'==' -NE = r'!=' -LT = r'<' -GT = r'>' -LNOT = r'!' -OR = r'\|' -AND = r'&' -EQUALS = r'=' +PLUS = r"\+" +MINUS = r"-" +TIMES = r"\*" +DIVIDE = r"/" +MOD = r"%" +NOT = r"~" +XOR = r"\^" +LOR = r"\|\|" +LAND = r"&&" +LSHIFT = r"<<" +RSHIFT = r">>" +LE = r"<=" +GE = r">=" +EQ = r"==" +NE = r"!=" +LT = r"<" +GT = r">" +LNOT = r"!" +OR = r"\|" +AND = r"&" +EQUALS = r"=" # ? -CONDOP = r'\?' +CONDOP = r"\?" # Delimiters -LPAREN = r'\(' -RPAREN = r'\)' -LBRACKET = r'\[' -RBRACKET = r'\]' -LBRACE = r'\{' -RBRACE = r'\}' -COMMA = r',' -PERIOD = r'\.' -SEMI = r';' -COLON = r':' -BACKSLASH = r'\\' - -operators = { op: pattern for op, pattern in globals().items() if op == op.upper() } +LPAREN = r"\(" +RPAREN = r"\)" +LBRACKET = r"\[" +RBRACKET = r"\]" +LBRACE = r"\{" +RBRACE = r"\}" +COMMA = r"," +PERIOD = r"\." +SEMI = r";" +COLON = r":" +BACKSLASH = r"\\" + +operators = {op: pattern for op, pattern in globals().items() if op == op.upper()} for op in operators: globals()[op] = op -opmap = { pattern.replace("\\", "") or '\\' : op for op, pattern in operators.items() } +opmap = {pattern.replace("\\", "") or "\\": op for op, pattern in operators.items()} # Macros -macro = r'# *(ifdef|ifndef|undef|define|error|endif|if|else|include|#)' -MACRO = 'MACRO' +macro = r"# *(ifdef|ifndef|undef|define|error|endif|if|else|include|#)" +MACRO = "MACRO" -id_re = r'[a-zA-Z_][0-9a-zA-Z_]*' -IDENTIFIER = 'IDENTIFIER' +id_re = r"[a-zA-Z_][0-9a-zA-Z_]*" +IDENTIFIER = "IDENTIFIER" -suffix = r'([uU]?[lL]?[lL]?)' -octal = r'0[0-7]+' + suffix -hex = r'0[xX][0-9a-fA-F]+' -decimal_digits = r'(0|[1-9][0-9]*)' +suffix = r"([uU]?[lL]?[lL]?)" +octal = r"0[0-7]+" + suffix +hex = r"0[xX][0-9a-fA-F]+" +decimal_digits = r"(0|[1-9][0-9]*)" decimal = decimal_digits + suffix exponent = r"""([eE][-+]?[0-9]+)""" fraction = r"""([0-9]*\.[0-9]+)|([0-9]+\.)""" -float = '(((('+fraction+')'+exponent+'?)|([0-9]+'+exponent+'))[FfLl]?)' +float = "((((" + fraction + ")" + exponent + "?)|([0-9]+" + exponent + "))[FfLl]?)" number_re = choice(octal, hex, float, decimal) -NUMBER = 'NUMBER' +NUMBER = "NUMBER" simple_escape = r"""([a-zA-Z._~!=&\^\-\\?'"])""" decimal_escape = r"""(\d+)""" hex_escape = r"""(x[0-9a-fA-F]+)""" -escape_sequence = r"""(\\("""+simple_escape+'|'+decimal_escape+'|'+hex_escape+'))' -string_char = r"""([^"\\\n]|"""+escape_sequence+')' -str_re = '"'+string_char+'*"' -STRING = 'STRING' -char = r'\'.\'' # TODO: escape sequence -CHARACTER = 'CHARACTER' +escape_sequence = ( + r"""(\\(""" + simple_escape + "|" + decimal_escape + "|" + hex_escape + "))" +) +string_char = r"""([^"\\\n]|""" + escape_sequence + ")" +str_re = '"' + string_char + '*"' +STRING = "STRING" +char = r"\'.\'" # TODO: escape sequence +CHARACTER = "CHARACTER" -comment_re = r'//.*|/\*([^*]|\*[^/])*\*/' -COMMENT = 'COMMENT' +comment_re = r"//.*|/\*([^*]|\*[^/])*\*/" +COMMENT = "COMMENT" newline = r"\n" -invalid = r"\S" # A single non-space character that's not caught by any of the other patterns -matcher = re.compile(choice(id_re, number_re, str_re, char, newline, macro, comment_re, *operators.values(), invalid)) -letter = re.compile(r'[a-zA-Z_]') - -kwds = ( - 'AUTO', 'BREAK', 'CASE', 'CHAR', 'CONST', - 'CONTINUE', 'DEFAULT', 'DO', 'DOUBLE', 'ELSE', 'ENUM', 'EXTERN', - 'FLOAT', 'FOR', 'GOTO', 'IF', 'INLINE', 'INT', 'LONG', 'OVERRIDE', - 'REGISTER', 'OFFSETOF', - 'RESTRICT', 'RETURN', 'SHORT', 'SIGNED', 'SIZEOF', 'STATIC', 'STRUCT', - 'SWITCH', 'TYPEDEF', 'UNION', 'UNSIGNED', 'VOID', - 'VOLATILE', 'WHILE' +invalid = ( + r"\S" # A single non-space character that's not caught by any of the other patterns ) +matcher = re.compile( + choice( + id_re, + number_re, + str_re, + char, + newline, + macro, + comment_re, + *operators.values(), + invalid, + ) +) +letter = re.compile(r"[a-zA-Z_]") + + +kwds = [] +AUTO = "AUTO" +kwds.append(AUTO) +BREAK = "BREAK" +kwds.append(BREAK) +CASE = "CASE" +kwds.append(CASE) +CHAR = "CHAR" +kwds.append(CHAR) +CONST = "CONST" +kwds.append(CONST) +CONTINUE = "CONTINUE" +kwds.append(CONTINUE) +DEFAULT = "DEFAULT" +kwds.append(DEFAULT) +DO = "DO" +kwds.append(DO) +DOUBLE = "DOUBLE" +kwds.append(DOUBLE) +ELSE = "ELSE" +kwds.append(ELSE) +ENUM = "ENUM" +kwds.append(ENUM) +EXTERN = "EXTERN" +kwds.append(EXTERN) +FLOAT = "FLOAT" +kwds.append(FLOAT) +FOR = "FOR" +kwds.append(FOR) +GOTO = "GOTO" +kwds.append(GOTO) +IF = "IF" +kwds.append(IF) +INLINE = "INLINE" +kwds.append(INLINE) +INT = "INT" +kwds.append(INT) +LONG = "LONG" +kwds.append(LONG) +OVERRIDE = "OVERRIDE" +kwds.append(OVERRIDE) +REGISTER = "REGISTER" +kwds.append(REGISTER) +OFFSETOF = "OFFSETOF" +kwds.append(OFFSETOF) +RESTRICT = "RESTRICT" +kwds.append(RESTRICT) +RETURN = "RETURN" +kwds.append(RETURN) +SHORT = "SHORT" +kwds.append(SHORT) +SIGNED = "SIGNED" +kwds.append(SIGNED) +SIZEOF = "SIZEOF" +kwds.append(SIZEOF) +STATIC = "STATIC" +kwds.append(STATIC) +STRUCT = "STRUCT" +kwds.append(STRUCT) +SWITCH = "SWITCH" +kwds.append(SWITCH) +TYPEDEF = "TYPEDEF" +kwds.append(TYPEDEF) +UNION = "UNION" +kwds.append(UNION) +UNSIGNED = "UNSIGNED" +kwds.append(UNSIGNED) +VOID = "VOID" +kwds.append(VOID) +VOLATILE = "VOLATILE" +kwds.append(VOLATILE) +WHILE = "WHILE" +kwds.append(WHILE) +keywords = {name.lower(): name for name in kwds} -# For mypy -REGISTER = 'REGISTER' -OVERRIDE = 'OVERRIDE' -IF = 'IF' - -for name in kwds: - globals()[name] = name -keywords = { name.lower() : name for name in kwds } +__all__ = [] +__all__.extend(kwds) def make_syntax_error( - message: str, filename: str | None, line: int, column: int, line_text: str, + message: str, + filename: str | None, + line: int, + column: int, + line_text: str, ) -> SyntaxError: return SyntaxError(message, (filename, line, column, line_text)) @@ -169,7 +250,7 @@ def end_column(self) -> int: def width(self) -> int: return self.end[1] - self.begin[1] - def replaceText(self, txt: str) -> 'Token': + def replaceText(self, txt: str) -> "Token": assert isinstance(txt, str) return Token(self.kind, txt, self.begin, self.end) @@ -191,73 +272,75 @@ def tokenize(src: str, line: int = 1, filename: str | None = None) -> Iterator[T kind = keywords[text] elif letter.match(text): kind = IDENTIFIER - elif text == '...': + elif text == "...": kind = ELLIPSIS - elif text == '.': + elif text == ".": kind = PERIOD - elif text[0] in '0123456789.': + elif text[0] in "0123456789.": kind = NUMBER elif text[0] == '"': kind = STRING elif text in opmap: kind = opmap[text] - elif text == '\n': + elif text == "\n": linestart = start line += 1 - kind = '\n' + kind = "\n" elif text[0] == "'": kind = CHARACTER - elif text[0] == '#': + elif text[0] == "#": kind = MACRO - elif text[0] == '/' and text[1] in '/*': + elif text[0] == "/" and text[1] in "/*": kind = COMMENT else: lineend = src.find("\n", start) if lineend == -1: lineend = len(src) - raise make_syntax_error(f"Bad token: {text}", - filename, line, start-linestart+1, src[linestart:lineend]) + raise make_syntax_error( + f"Bad token: {text}", + filename, + line, + start - linestart + 1, + src[linestart:lineend], + ) if kind == COMMENT: - begin = line, start-linestart - newlines = text.count('\n') + begin = line, start - linestart + newlines = text.count("\n") if newlines: - linestart = start + text.rfind('\n') + linestart = start + text.rfind("\n") line += newlines else: - begin = line, start-linestart + begin = line, start - linestart if kind != "\n": - yield Token(kind, text, begin, (line, start-linestart+len(text))) - - -__all__ = [] -__all__.extend([kind for kind in globals() if kind.upper() == kind]) + yield Token(kind, text, begin, (line, start - linestart + len(text))) def to_text(tkns: list[Token], dedent: int = 0) -> str: res: list[str] = [] - line, col = -1, 1+dedent + line, col = -1, 1 + dedent for tkn in tkns: if line == -1: line, _ = tkn.begin l, c = tkn.begin - #assert(l >= line), (line, txt, start, end) + # assert(l >= line), (line, txt, start, end) while l > line: line += 1 - res.append('\n') - col = 1+dedent - res.append(' '*(c-col)) + res.append("\n") + col = 1 + dedent + res.append(" " * (c - col)) text = tkn.text - if dedent != 0 and tkn.kind == 'COMMENT' and '\n' in text: + if dedent != 0 and tkn.kind == "COMMENT" and "\n" in text: if dedent < 0: - text = text.replace('\n', '\n' + ' '*-dedent) + text = text.replace("\n", "\n" + " " * -dedent) # TODO: dedent > 0 res.append(text) line, col = tkn.end - return ''.join(res) + return "".join(res) if __name__ == "__main__": import sys + filename = sys.argv[1] if filename == "-c": src = sys.argv[2] diff --git a/Tools/cases_generator/mypy.ini b/Tools/cases_generator/mypy.ini index 7f56e66fe90232..12409b8ad953ea 100644 --- a/Tools/cases_generator/mypy.ini +++ b/Tools/cases_generator/mypy.ini @@ -8,4 +8,4 @@ python_version = 3.10 strict = True strict_concatenate = True enable_error_code = ignore-without-code,redundant-expr,truthy-bool -warn_unreachable = True +warn_unreachable = False diff --git a/Tools/cases_generator/requirements-dev.txt b/Tools/cases_generator/requirements-dev.txt deleted file mode 100644 index e9529f3527e95e..00000000000000 --- a/Tools/cases_generator/requirements-dev.txt +++ /dev/null @@ -1,2 +0,0 @@ -# Requirements file for external linters and checks we run on Tools/clinic/ in CI -mypy==1.4.1 diff --git a/Tools/cases_generator/stacking.py b/Tools/cases_generator/stacking.py index fcf1ec044ccb21..1e117f11825938 100644 --- a/Tools/cases_generator/stacking.py +++ b/Tools/cases_generator/stacking.py @@ -413,16 +413,16 @@ def write_components( return next_instr_is_set -def write_single_instr_for_abstract_interp( - instr: Instruction, out: Formatter -) -> None: +def write_single_instr_for_abstract_interp(instr: Instruction, out: Formatter) -> None: try: _write_components_for_abstract_interp( [Component(instr, instr.active_caches)], out, ) except AssertionError as err: - raise AssertionError(f"Error writing abstract instruction {instr.name}") from err + raise AssertionError( + f"Error writing abstract instruction {instr.name}" + ) from err def _write_components_for_abstract_interp( @@ -438,5 +438,7 @@ def _write_components_for_abstract_interp( # NULL out the output stack effects for poke in mgr.pokes: if not poke.effect.size and poke.effect.name not in mgr.instr.unmoved_names: - out.emit(f"PARTITIONNODE_OVERWRITE((_Py_PARTITIONNODE_t *)" - f"PARTITIONNODE_NULLROOT, PEEK(-({poke.offset.as_index()})), true);") + out.emit( + f"PARTITIONNODE_OVERWRITE((_Py_PARTITIONNODE_t *)" + f"PARTITIONNODE_NULLROOT, PEEK(-({poke.offset.as_index()})), true);" + ) diff --git a/Tools/clinic/requirements-dev.txt b/Tools/clinic/requirements-dev.txt deleted file mode 100644 index e9529f3527e95e..00000000000000 --- a/Tools/clinic/requirements-dev.txt +++ /dev/null @@ -1,2 +0,0 @@ -# Requirements file for external linters and checks we run on Tools/clinic/ in CI -mypy==1.4.1 diff --git a/Tools/requirements-dev.txt b/Tools/requirements-dev.txt new file mode 100644 index 00000000000000..279d8c2dcdab42 --- /dev/null +++ b/Tools/requirements-dev.txt @@ -0,0 +1,4 @@ +# Requirements file for external linters and checks we run on +# Tools/clinic and Tools/cases_generator/ in CI +mypy==1.5.1 +black From 6c89a682bd42b811905633ae7fdbc8aa3f32c9fe Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:17:34 +0900 Subject: [PATCH 04/17] Update --- .github/dependabot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index f026b0f5f9454a..c8a3165d690364 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -13,7 +13,7 @@ updates: - "version-update:semver-minor" - "version-update:semver-patch" - package-ecosystem: "pip" - directory: "/Tools/clinic/" + directory: "/Tools/" schedule: interval: "monthly" labels: From 42ed5023fb937501039efb34c3af31654e57ca08 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:23:44 +0900 Subject: [PATCH 05/17] Move lint --- .github/workflows/lint.yml | 2 ++ .github/workflows/mypy.yml | 2 -- Tools/requirements-dev.txt | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 4481ea80bfd936..c2ca2111b94e99 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -20,3 +20,5 @@ jobs: with: python-version: "3.x" - uses: pre-commit/action@v3.0.0 + - run: pip install black + - run: black Tools/cases_generator diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 742fd146c43ec1..f4632e80332c33 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -51,6 +51,4 @@ jobs: cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt - - name: Lint with black - run: black Tools/cases_generator - run: mypy --config-file Tools/cases_generator/mypy.ini diff --git a/Tools/requirements-dev.txt b/Tools/requirements-dev.txt index 279d8c2dcdab42..111773f4f47378 100644 --- a/Tools/requirements-dev.txt +++ b/Tools/requirements-dev.txt @@ -1,4 +1,3 @@ # Requirements file for external linters and checks we run on # Tools/clinic and Tools/cases_generator/ in CI mypy==1.5.1 -black From 6bc0d262a887c70a4f943809e0aa569d331b70ac Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:25:47 +0900 Subject: [PATCH 06/17] Use alphabetical order --- .github/workflows/mypy.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index f4632e80332c33..79f222354e81a1 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -25,8 +25,8 @@ concurrency: cancel-in-progress: true jobs: - mypy-clinic: - name: Run mypy on Tools/clinic/ + mypy-cases-generator: + name: Run mypy on Tools/cases_generator/ runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -37,10 +37,10 @@ jobs: cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt - - run: mypy --config-file Tools/clinic/mypy.ini + - run: mypy --config-file Tools/cases_generator/mypy.ini - mypy-cases-generator: - name: Run mypy on Tools/cases_generator/ + mypy-clinic: + name: Run mypy on Tools/clinic/ runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -51,4 +51,4 @@ jobs: cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt - - run: mypy --config-file Tools/cases_generator/mypy.ini + - run: mypy --config-file Tools/clinic/mypy.ini From 999ba109a234b19791dd1bc8ce58877567530e96 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:37:13 +0900 Subject: [PATCH 07/17] Update --- .github/workflows/lint.yml | 8 +++++--- Tools/requirements-dev.txt | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c2ca2111b94e99..e216b67f8a428b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,7 +18,9 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.x" + python-version: "3.11" + cache: pip + cache-dependency-path: Tools/requirements-dev.txt - uses: pre-commit/action@v3.0.0 - - run: pip install black - - run: black Tools/cases_generator + - run: pip install -r Tools/requirements-dev.txt + - run: black --check Tools/cases_generator diff --git a/Tools/requirements-dev.txt b/Tools/requirements-dev.txt index 111773f4f47378..5cb87afc0a2107 100644 --- a/Tools/requirements-dev.txt +++ b/Tools/requirements-dev.txt @@ -1,3 +1,4 @@ # Requirements file for external linters and checks we run on # Tools/clinic and Tools/cases_generator/ in CI mypy==1.5.1 +black==23.7.0 From 5078f7a0806c4149da9ccf319817ef8ce20b8392 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 09:39:54 +0900 Subject: [PATCH 08/17] revert --- .github/workflows/lint.yml | 6 +----- Tools/requirements-dev.txt | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e216b67f8a428b..4481ea80bfd936 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,9 +18,5 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.11" - cache: pip - cache-dependency-path: Tools/requirements-dev.txt + python-version: "3.x" - uses: pre-commit/action@v3.0.0 - - run: pip install -r Tools/requirements-dev.txt - - run: black --check Tools/cases_generator diff --git a/Tools/requirements-dev.txt b/Tools/requirements-dev.txt index 5cb87afc0a2107..111773f4f47378 100644 --- a/Tools/requirements-dev.txt +++ b/Tools/requirements-dev.txt @@ -1,4 +1,3 @@ # Requirements file for external linters and checks we run on # Tools/clinic and Tools/cases_generator/ in CI mypy==1.5.1 -black==23.7.0 From e695d2f320f696174717bf3a98e988ae046a02b6 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 10:27:18 +0900 Subject: [PATCH 09/17] Address code review --- .github/workflows/mypy.yml | 4 ++-- Tools/cases_generator/generate_cases.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 79f222354e81a1..715547bda9e395 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -33,7 +33,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.x" + python-version: "3.11" cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt @@ -47,7 +47,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.x" + python-version: "3.11" cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 5b4c61ceed3181..4c214e25113295 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -181,7 +181,11 @@ def effect_str(effects: list[StackEffect]) -> str: assert target_instr target_popped = effect_str(target_instr.input_effects) target_pushed = effect_str(target_instr.output_effects) + if popped is None and pushed is None: popped, pushed = target_popped, target_pushed + else: + assert popped == target_popped + assert pushed == target_pushed case _: typing.assert_never(thing) return instr, popped, pushed @@ -391,7 +395,11 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No assert target_instr if format is None: format = target_instr.instr_fmt + else: + assert format == target_instr.instr_fmt assert format is not None + case _: + typing.assert_never(thing) all_formats.add(format) # Turn it into a sorted list of enum values. @@ -480,6 +488,8 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No self.write_metadata_for_pseudo( self.pseudo_instrs[thing.name] ) + case _: + typing.assert_never(thing) with self.metadata_item( "const struct opcode_macro_expansion " @@ -515,6 +525,8 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No ) case parsing.Pseudo(): pass + case _: + typing.assert_never(thing) with self.metadata_item( "const char * const _PyOpcode_uop_name[OPCODE_UOP_NAME_SIZE]", "=", ";" @@ -762,6 +774,8 @@ def write_instructions( # self.write_macro(self.macro_instrs[thing.name]) case parsing.Pseudo(): pass + case _: + typing.assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_macros} macros " @@ -804,6 +818,8 @@ def write_executor_instructions( pass case parsing.Pseudo(): pass + case _: + typing.assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_uops} ops to {executor_filename}", file=sys.stderr, From a136c24c2f63df68fabc8c12c95d0714b8edf769 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 10:29:57 +0900 Subject: [PATCH 10/17] Resolve redundant-expr --- Tools/cases_generator/generate_cases.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 4c214e25113295..134ad7e0600d43 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -181,7 +181,8 @@ def effect_str(effects: list[StackEffect]) -> str: assert target_instr target_popped = effect_str(target_instr.input_effects) target_pushed = effect_str(target_instr.output_effects) - if popped is None and pushed is None: + if pushed is None: + assert popped is None popped, pushed = target_popped, target_pushed else: assert popped == target_popped From d26b39eff75ed6101c0b661af41a4f7551c1799d Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 11:32:11 +0900 Subject: [PATCH 11/17] Use matrix --- .github/workflows/mypy.yml | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 715547bda9e395..a83a90c69529f2 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -25,8 +25,11 @@ concurrency: cancel-in-progress: true jobs: - mypy-cases-generator: - name: Run mypy on Tools/cases_generator/ + mypy: + strategy: + matrix: + target: ["Tools/cases_generator", "Tools/clinic"] + name: Run mypy on ${{ matrix.target }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -37,18 +40,4 @@ jobs: cache: pip cache-dependency-path: Tools/requirements-dev.txt - run: pip install -r Tools/requirements-dev.txt - - run: mypy --config-file Tools/cases_generator/mypy.ini - - mypy-clinic: - name: Run mypy on Tools/clinic/ - runs-on: ubuntu-latest - timeout-minutes: 10 - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 - with: - python-version: "3.11" - cache: pip - cache-dependency-path: Tools/requirements-dev.txt - - run: pip install -r Tools/requirements-dev.txt - - run: mypy --config-file Tools/clinic/mypy.ini + - run: mypy --config-file ${{ matrix.target }}/mypy.ini From c0ae7c24093c3a041530911fa8c88ccda51a4463 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 11:34:19 +0900 Subject: [PATCH 12/17] update --- Tools/cases_generator/generate_cases.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 134ad7e0600d43..23a933f90b1c95 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -851,6 +851,8 @@ def write_abstract_interpreter_instructions( pass case parsing.Pseudo(): pass + case _: + typing.assert_never(thing) print( f"Wrote some stuff to {abstract_interpreter_filename}", file=sys.stderr, From dcb3ada5fb66d5f6fd891bf53e1604a3c916a4fa Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 11:36:41 +0900 Subject: [PATCH 13/17] Address code review --- Tools/cases_generator/generate_cases.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 23a933f90b1c95..53a4501d997e78 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -4,6 +4,7 @@ """ import argparse +import collections import contextlib import itertools import os @@ -194,7 +195,7 @@ def effect_str(effects: list[StackEffect]) -> str: @contextlib.contextmanager def metadata_item( self, signature: str, open: str, close: str - ) -> typing.Iterator[None]: + ) -> collections.abc.Iterator[None]: self.out.emit("") self.out.emit(f"extern {signature};") self.out.emit("#ifdef NEED_OPCODE_METADATA") From 7f9e68b7012f8a3e0efdaa3340c333f66b965ff9 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 11:42:31 +0900 Subject: [PATCH 14/17] Fix indent --- Tools/cases_generator/generate_cases.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 53a4501d997e78..a21a19995709d8 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -182,12 +182,12 @@ def effect_str(effects: list[StackEffect]) -> str: assert target_instr target_popped = effect_str(target_instr.input_effects) target_pushed = effect_str(target_instr.output_effects) - if pushed is None: - assert popped is None - popped, pushed = target_popped, target_pushed - else: - assert popped == target_popped - assert pushed == target_pushed + if pushed is None: + assert popped is None + popped, pushed = target_popped, target_pushed + else: + assert popped == target_popped + assert pushed == target_pushed case _: typing.assert_never(thing) return instr, popped, pushed From 9b13d6b0fd807199b703f96c228f7bacf9fa77e3 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 13:29:23 +0900 Subject: [PATCH 15/17] run black --- Tools/cases_generator/flags.py | 1 - Tools/cases_generator/generate_cases.py | 1 - 2 files changed, 2 deletions(-) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index c82f94a5271e4c..536f093c7d6ede 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -21,7 +21,6 @@ def __post_init__(self) -> None: @staticmethod def fromInstruction(instr: parsing.Node) -> "InstructionFlags": - has_free = ( variable_used(instr, "PyCell_New") or variable_used(instr, "PyCell_GET") diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index a21a19995709d8..aa255cf772a205 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -218,7 +218,6 @@ def write_stack_effect_functions(self) -> None: def write_function( direction: str, data: list[tuple[AnyInstruction, str]] ) -> None: - with self.metadata_item( f"int _PyOpcode_num_{direction}(int opcode, int oparg, bool jump)", "", From 5b3732dbad60d1b64efeb417c4538bde3ae86481 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 16:21:54 +0900 Subject: [PATCH 16/17] Use shorter name of Iterator --- Tools/cases_generator/formatting.py | 6 +++--- Tools/cases_generator/generate_cases.py | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Tools/cases_generator/formatting.py b/Tools/cases_generator/formatting.py index 65ed5d7d042ef9..4fd9172d20c274 100644 --- a/Tools/cases_generator/formatting.py +++ b/Tools/cases_generator/formatting.py @@ -1,7 +1,7 @@ -import collections import contextlib import re import typing +from collections.abc import Iterator from parsing import StackEffect, Family @@ -59,13 +59,13 @@ def reset_lineno(self) -> None: self.set_lineno(self.lineno + 1, self.filename) @contextlib.contextmanager - def indent(self) -> collections.abc.Iterator[None]: + def indent(self) -> Iterator[None]: self.prefix += " " yield self.prefix = self.prefix[:-4] @contextlib.contextmanager - def block(self, head: str, tail: str = "") -> collections.abc.Iterator[None]: + def block(self, head: str, tail: str = "") -> Iterator[None]: if head: self.emit(head + " {") else: diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index aa255cf772a205..de31129ac0548d 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -4,13 +4,13 @@ """ import argparse -import collections import contextlib import itertools import os import posixpath import sys import typing +from collections.abc import Iterator import stacking # Early import to avoid circular import from analysis import Analyzer @@ -193,9 +193,7 @@ def effect_str(effects: list[StackEffect]) -> str: return instr, popped, pushed @contextlib.contextmanager - def metadata_item( - self, signature: str, open: str, close: str - ) -> collections.abc.Iterator[None]: + def metadata_item(self, signature: str, open: str, close: str) -> Iterator[None]: self.out.emit("") self.out.emit(f"extern {signature};") self.out.emit("#ifdef NEED_OPCODE_METADATA") From aebfff3141381223996ca2970b26ff0907cadd8b Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 18 Aug 2023 16:22:41 +0900 Subject: [PATCH 17/17] Apply suggestions from code review Co-authored-by: Alex Waygood --- Tools/cases_generator/mypy.ini | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/mypy.ini b/Tools/cases_generator/mypy.ini index 12409b8ad953ea..7480841bf07edc 100644 --- a/Tools/cases_generator/mypy.ini +++ b/Tools/cases_generator/mypy.ini @@ -4,8 +4,11 @@ pretty = True python_version = 3.10 -# and be strict! +# Be strict: strict = True strict_concatenate = True enable_error_code = ignore-without-code,redundant-expr,truthy-bool + +# Don't enable this one yet; +# it has a lot of false positives on `cases_generator` warn_unreachable = False