gh-104504: cases generator: Add --warn-unreachable to the mypy config
#108112
Conversation
AlexWaygood
requested review from
gvanrossum,
Fidget-Spinner and
JelleZijlstra
as code owners
AlexWaygood
changed the title
--warn-unreachable to the mypy config--warn-unreachable to the mypy config
| @@ -388,7 +388,6 @@ 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 | |||
There was a problem hiding this comment.
Similarly, here mypy was eagerly narrowing the type of format to None, meaning that it was inferring the else branch on line 396 to be unreachable.
There was a problem hiding this comment.
Still feels like a mypy bug to me, but your fix is fine.
|
@gvanrossum, are you happier with the new refactor? Now |
|
(The test_concurrent_futures failure on Azure Pipelines is unrelated) |
| # 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): |
There was a problem hiding this comment.
Clever idea. I wrote a whole paragraph arguing against using enumerate() here, then realized it wasn't worth arguing over. :-)
There was a problem hiding this comment.
FYI, the saga continues. Currently we get errors from pyright on this: "popped is possibly unbound" (on all occurrences of popped and pushed in this function below here).
There was a problem hiding this comment.
FYI, the saga continues. Currently we get errors from pyright on this: "popped is possibly unbound" (on all occurrences of popped and pushed in this function below here).
Yeah, and mypy will also give you that error if you add --enable-error-code=possibly-undefined to the mypy config :/
There is a third way of fixing this. I'll make a PR and see how you like it.
| @@ -388,7 +388,6 @@ 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 | |||
There was a problem hiding this comment.
Still feels like a mypy bug to me, but your fix is fine.
|
Thanks for the review! |
| 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) |
Backport
typing.assert_neverto thecases_generatordirectory, so that mypy doesn't complain about the fact that we're using a feature new in Python 3.11 when we have--python-version 3.10in our config file.mypyoncases_generator#104504