From 05ef4ca94c694bc50e6fd221fe648d05d227f3a4 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 21 Aug 2023 00:40:41 +0100 Subject: [PATCH] gh-104504: cases generator: Add `--warn-unreachable` to the mypy config (#108112) --- Tools/cases_generator/_typing_backports.py | 15 ++++++++++ Tools/cases_generator/analysis.py | 7 +++-- Tools/cases_generator/generate_cases.py | 33 +++++++++------------- Tools/cases_generator/mypy.ini | 9 +++--- 4 files changed, 36 insertions(+), 28 deletions(-) create mode 100644 Tools/cases_generator/_typing_backports.py diff --git a/Tools/cases_generator/_typing_backports.py b/Tools/cases_generator/_typing_backports.py new file mode 100644 index 00000000000000..c2aa50804cefe2 --- /dev/null +++ b/Tools/cases_generator/_typing_backports.py @@ -0,0 +1,15 @@ +"""Backports from newer versions of the typing module. + +We backport these features here so that Python can still build +while using an older Python version for PYTHON_FOR_REGEN. +""" + +from typing import NoReturn + + +def assert_never(obj: NoReturn) -> NoReturn: + """Statically assert that a line of code is unreachable. + + Backport of typing.assert_never (introduced in Python 3.11). + """ + raise AssertionError(f"Expected code to be unreachable, but got: {obj}") diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 48f2db981c95b6..72fb2d761dbfae 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -2,6 +2,7 @@ import sys import typing +from _typing_backports import assert_never from flags import InstructionFlags, variable_used from formatting import prettify_filename, UNUSED from instructions import ( @@ -172,7 +173,7 @@ def parse_file(self, filename: str, instrs_idx: dict[str, int]) -> None: self.pseudos[name] = thing self.everything.append(thing) case _: - typing.assert_never(thing) + assert_never(thing) if not psr.eof(): raise psr.make_syntax_error(f"Extra stuff at the end of {filename}") @@ -368,7 +369,7 @@ def analyze_macro(self, macro: parsing.Macro) -> MacroInstruction: # SAVE_IP in a macro is a no-op in Tier 1 flags.add(instr.instr_flags) case _: - typing.assert_never(component) + assert_never(component) format = "IB" if flags.HAS_ARG_FLAG else "IX" if offset: format += "C" + "0" * (offset - 1) @@ -409,5 +410,5 @@ def check_macro_components( case parsing.CacheEffect(): components.append(uop) case _: - typing.assert_never(uop) + assert_never(uop) return components diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index de31129ac0548d..7b1880b98a8feb 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -13,6 +13,7 @@ from collections.abc import Iterator import stacking # Early import to avoid circular import +from _typing_backports import assert_never from analysis import Analyzer from formatting import Formatter, list_effect_size from flags import InstructionFlags, variable_used @@ -146,7 +147,7 @@ class Generator(Analyzer): def get_stack_effect_info( self, thing: parsing.InstDef | parsing.Macro | parsing.Pseudo - ) -> tuple[AnyInstruction | None, str | None, str | None]: + ) -> tuple[AnyInstruction | None, str, str]: def effect_str(effects: list[StackEffect]) -> str: n_effect, sym_effect = list_effect_size(effects) if sym_effect: @@ -154,8 +155,6 @@ def effect_str(effects: list[StackEffect]) -> str: return str(n_effect) instr: AnyInstruction | None - popped: str | None - pushed: str | None match thing: case parsing.InstDef(): if thing.kind != "op" or self.instrs[thing.name].is_viable_uop(): @@ -171,10 +170,9 @@ def effect_str(effects: list[StackEffect]) -> str: popped, pushed = stacking.get_stack_effect_info_for_macro(instr) case parsing.Pseudo(): instr = self.pseudo_instrs[thing.name] - popped = pushed = None # Calculate stack effect, and check that it's the the same # for all targets. - for target in self.pseudos[thing.name].targets: + for idx, target in enumerate(self.pseudos[thing.name].targets): target_instr = self.instrs.get(target) # Currently target is always an instr. This could change # in the future, e.g., if we have a pseudo targetting a @@ -182,14 +180,13 @@ 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 + if idx == 0: popped, pushed = target_popped, target_pushed else: assert popped == target_popped assert pushed == target_pushed case _: - typing.assert_never(thing) + assert_never(thing) return instr, popped, pushed @contextlib.contextmanager @@ -209,7 +206,6 @@ def write_stack_effect_functions(self) -> None: continue instr, popped, pushed = self.get_stack_effect_info(thing) if instr is not None: - assert popped is not None and pushed is not None popped_data.append((instr, popped)) pushed_data.append((instr, pushed)) @@ -379,7 +375,6 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No # Compute the set of all instruction formats. all_formats: set[str] = set() for thing in self.everything: - format: str | None match thing: case OverriddenInstructionPlaceHolder(): continue @@ -388,17 +383,15 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No case parsing.Macro(): format = self.macro_instrs[thing.name].instr_fmt case parsing.Pseudo(): - format = None - for target in self.pseudos[thing.name].targets: + for idx, target in enumerate(self.pseudos[thing.name].targets): target_instr = self.instrs.get(target) assert target_instr - if format is None: + if idx == 0: format = target_instr.instr_fmt else: assert format == target_instr.instr_fmt - assert format is not None case _: - typing.assert_never(thing) + assert_never(thing) all_formats.add(format) # Turn it into a sorted list of enum values. @@ -488,7 +481,7 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No self.pseudo_instrs[thing.name] ) case _: - typing.assert_never(thing) + assert_never(thing) with self.metadata_item( "const struct opcode_macro_expansion " @@ -525,7 +518,7 @@ def write_metadata(self, metadata_filename: str, pymetadata_filename: str) -> No case parsing.Pseudo(): pass case _: - typing.assert_never(thing) + assert_never(thing) with self.metadata_item( "const char * const _PyOpcode_uop_name[OPCODE_UOP_NAME_SIZE]", "=", ";" @@ -774,7 +767,7 @@ def write_instructions( case parsing.Pseudo(): pass case _: - typing.assert_never(thing) + assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_macros} macros " @@ -818,7 +811,7 @@ def write_executor_instructions( case parsing.Pseudo(): pass case _: - typing.assert_never(thing) + assert_never(thing) print( f"Wrote {n_instrs} instructions and {n_uops} ops to {executor_filename}", file=sys.stderr, @@ -850,7 +843,7 @@ def write_abstract_interpreter_instructions( case parsing.Pseudo(): pass case _: - typing.assert_never(thing) + assert_never(thing) print( f"Wrote some stuff to {abstract_interpreter_filename}", file=sys.stderr, diff --git a/Tools/cases_generator/mypy.ini b/Tools/cases_generator/mypy.ini index 7480841bf07edc..54ccb8c5c1a3a2 100644 --- a/Tools/cases_generator/mypy.ini +++ b/Tools/cases_generator/mypy.ini @@ -2,13 +2,12 @@ files = Tools/cases_generator/ pretty = True +# Make sure Python can still be built +# using Python 3.10 for `PYTHON_FOR_REGEN`... python_version = 3.10 -# Be strict: +# ...And 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 +warn_unreachable = True