From bec3fb7037b60c560f9a7d264d0917442e2e717e Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 24 Aug 2023 21:09:03 +0100 Subject: [PATCH 1/3] Cases generator: enable mypy's `possibly-undefined` error code --- Tools/cases_generator/generate_cases.py | 25 ++++++++++++++++--------- Tools/cases_generator/mypy.ini | 2 +- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 8f9a6502e529c7..bdc897ff6b4a64 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -173,7 +173,9 @@ def effect_str(effects: list[StackEffect]) -> str: instr = self.pseudo_instrs[thing.name] # Calculate stack effect, and check that it's the the same # for all targets. - for idx, target in enumerate(self.pseudos[thing.name].targets): + maybe_popped: str | None = None + maybe_pushed: str | None = None + for target in 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 @@ -181,11 +183,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 idx == 0: - popped, pushed = target_popped, target_pushed + if maybe_popped is None: + maybe_popped, maybe_pushed = target_popped, target_pushed else: - assert popped == target_popped - assert pushed == target_pushed + assert maybe_popped == target_popped + assert maybe_pushed == target_pushed + assert maybe_popped is not None and maybe_pushed is not None + popped, pushed = maybe_popped, maybe_pushed case _: assert_never(thing) return instr, popped, pushed @@ -384,13 +388,16 @@ 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(): - for idx, target in enumerate(self.pseudos[thing.name].targets): + maybe_format: str | None = None + for target in self.pseudos[thing.name].targets: target_instr = self.instrs.get(target) assert target_instr - if idx == 0: - format = target_instr.instr_fmt + if maybe_format is None: + maybe_format = target_instr.instr_fmt else: - assert format == target_instr.instr_fmt + assert maybe_format == target_instr.instr_fmt + assert maybe_format is not None + format = maybe_format case _: assert_never(thing) all_formats.add(format) diff --git a/Tools/cases_generator/mypy.ini b/Tools/cases_generator/mypy.ini index 54ccb8c5c1a3a2..e7175e263350b2 100644 --- a/Tools/cases_generator/mypy.ini +++ b/Tools/cases_generator/mypy.ini @@ -9,5 +9,5 @@ python_version = 3.10 # ...And be strict: strict = True strict_concatenate = True -enable_error_code = ignore-without-code,redundant-expr,truthy-bool +enable_error_code = ignore-without-code,redundant-expr,truthy-bool,possibly-undefined warn_unreachable = True From cda549975b2bf6a552d78be70ed43b463bfa551a Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 24 Aug 2023 21:43:04 +0100 Subject: [PATCH 2/3] Better variable name --- 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 bdc897ff6b4a64..41eb5219d7c2ae 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -388,16 +388,16 @@ 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(): - maybe_format: str | None = None + fmt: str | None = None for target in self.pseudos[thing.name].targets: target_instr = self.instrs.get(target) assert target_instr - if maybe_format is None: - maybe_format = target_instr.instr_fmt + if fmt is None: + fmt = target_instr.instr_fmt else: - assert maybe_format == target_instr.instr_fmt - assert maybe_format is not None - format = maybe_format + assert fmt == target_instr.instr_fmt + assert fmt is not None + format = fmt case _: assert_never(thing) all_formats.add(format) From 163167b8ca68f4715450da986e86e263bc15e13e Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 25 Aug 2023 11:43:59 +0100 Subject: [PATCH 3/3] Work around the mypy bugs --- Tools/cases_generator/generate_cases.py | 26 ++++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 41eb5219d7c2ae..63a3db5c74f353 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -156,6 +156,8 @@ def effect_str(effects: list[StackEffect]) -> str: return str(n_effect) instr: AnyInstruction | None + popped: str | None = None + pushed: str | None = None match thing: case parsing.InstDef(): if thing.kind != "op" or self.instrs[thing.name].is_viable_uop(): @@ -173,8 +175,6 @@ def effect_str(effects: list[StackEffect]) -> str: instr = self.pseudo_instrs[thing.name] # Calculate stack effect, and check that it's the the same # for all targets. - maybe_popped: str | None = None - maybe_pushed: str | None = None for target in self.pseudos[thing.name].targets: target_instr = self.instrs.get(target) # Currently target is always an instr. This could change @@ -183,15 +183,14 @@ 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 maybe_popped is None: - maybe_popped, maybe_pushed = target_popped, target_pushed + if popped is None: + popped, pushed = target_popped, target_pushed else: - assert maybe_popped == target_popped - assert maybe_pushed == target_pushed - assert maybe_popped is not None and maybe_pushed is not None - popped, pushed = maybe_popped, maybe_pushed + assert popped == target_popped + assert pushed == target_pushed case _: assert_never(thing) + assert popped is not None and pushed is not None return instr, popped, pushed @contextlib.contextmanager @@ -380,6 +379,7 @@ 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 = None match thing: case OverriddenInstructionPlaceHolder(): continue @@ -388,18 +388,16 @@ 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(): - fmt: str | None = None for target in self.pseudos[thing.name].targets: target_instr = self.instrs.get(target) assert target_instr - if fmt is None: - fmt = target_instr.instr_fmt + if format is None: + format = target_instr.instr_fmt else: - assert fmt == target_instr.instr_fmt - assert fmt is not None - format = fmt + assert format == target_instr.instr_fmt case _: assert_never(thing) + assert format is not None all_formats.add(format) # Turn it into a sorted list of enum values.