Fix reachability inference with 'isinstance(Any, Any)' #7048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While experimenting with some reachability inference logic, I discovered
the following program does not seem to behave as expected:
```python
from typing import Any
from foo import A # type: ignore
x: Any
if isinstance(x, A):
reveal_type(x)
else:
reveal_type(x)
```
Both branches really ought to be reachable: since both `x` and `A` are
of type `Any`, we really can't say whether or not any particular branch
will run.
However, mypy currently instead assumes that only the first branch is
reachable and does not type-check the second branch. So in this example,
only the first `reveal_type(...)` outputs a note.
This pull request fixes this bug.
Michael0x2a
added a commit
to Michael0x2a/mypy
that referenced
this pull request
Jun 24, 2019
This diff adds a `--disallow-inferred-unreachable` flag that reports an error if: 1. Any branch is inferred to be unreachable 2. Any subexpression in boolean expressions, inline if statements, and list/set/generator/dict comprehensions are always unreachable or redundant. This only takes into account the results of *type* analysis. We do not report errors for branches that are inferred to be unreachable in the semantic analysis phase (e.g. due to `sys.platform` checks and the like). Those checks are all intentional/deliberately flag certain branches as unreachable, so error messages would be annoying in those cases. A few additional notes: 1. I didn't spend a huge amount of time trying to come up with error messages. They could probably do with some polishing/rewording. 2. I thought about enabling this flag for default with mypy, but unfortunately that ended up producing ~40 to 50 (apparently legitimate) error messages. About a fourth of them were due to runtime checks checking to see if some type is None (despite not being declared to be Optional), about a fourth seemed to be due to mypy not quite understanding how to handle things like traits and Bogus[...], and about a fourth seemed to be legitimately unnecessary checks we could safely remove. The final checks were a mixture of typeshed bugs and misc errors with the reachability checks. (e.g. see python#7048) 3. For these reasons, I decided against adding this flag to `--strict` and against documenting it yet. We'll probably need to flush out a few bugs in mypy first/get mypy to the state where we can at least honestly dogfood this. Resolves python#2395; partially addresses python#7008.
msullivan
approved these changes
Jun 26, 2019
JukkaL
pushed a commit
that referenced
this pull request
Jun 28, 2019
This diff adds a `--warn-unreachable` flag that reports an error if: 1. Any branch is inferred to be unreachable 2. Any subexpression in boolean expressions, inline if statements, and list/set/generator/dict comprehensions are always unreachable or redundant. This only takes into account the results of *type* analysis. We do not report errors for branches that are inferred to be unreachable in the semantic analysis phase (e.g. due to `sys.platform` checks and the like). Those checks are all intentional/deliberately flag certain branches as unreachable, so error messages would be annoying in those cases. A few additional notes: 1. I thought about enabling this flag for default with mypy, but unfortunately that ended up producing ~40 to 50 (apparently legitimate) error messages. About a fourth of them were due to runtime checks checking to see if some type is None (despite not being declared to be Optional), about a fourth seemed to be due to mypy not quite understanding how to handle things like traits and Bogus[...], and about a fourth seemed to be legitimately unnecessary checks we could safely remove. The final checks were a mixture of typeshed bugs and misc errors with the reachability checks. (e.g. see #7048) 2. For these reasons, I decided against adding this flag to `--strict` and against documenting it yet. We'll probably need to flush out a few bugs in mypy first/get mypy to the state where we can at least honestly dogfood this. Resolves #2395; partially addresses #7008.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While experimenting with some reachability inference logic, I discovered the following program does not seem to behave as expected:
Both branches really ought to be reachable: since both
xandAare of typeAny, we really can't say whether or not any particular branch will run: both branches ought to be reachable.However, mypy currently instead assumes that only the first branch is reachable and does not type-check the second branch. So in this example, only the first
reveal_type(...)outputs a note.This pull request fixes this bug.