0.990 regression with implicit complex/int promotion

[JelleZijlstra]

Given this code:

from typing import reveal_type
x: complex = 1
reveal_type(x)
if isinstance(x, int):
    reveal_type(x)

mypy 0.990:

% mypy --strict --warn-unreachable ~/py/tmp/compl.py
/Users/jelle/py/tmp/compl.py:3: note: Revealed type is "builtins.complex"
/Users/jelle/py/tmp/compl.py:4: error: Subclass of "complex" and "int" cannot exist: would have incompatible method signatures  [unreachable]
/Users/jelle/py/tmp/compl.py:5: error: Statement is unreachable  [unreachable]
Found 2 errors in 1 file (checked 1 source file)

mypy 0.982:

% mypy --strict --warn-unreachable ~/py/tmp/compl.py
/Users/jelle/py/tmp/compl.py:3: note: Revealed type is "builtins.complex"
/Users/jelle/py/tmp/compl.py:5: note: Revealed type is "builtins.int"
Success: no issues found in 1 source file

The 0.982 behavior is correct, since int is a subclass of complex in the static type system.

[AlexWaygood]

This bug introduces 8 false positives when mypy checks the flake8-pyi codebase (we have --warn-unreachable enabled in our pyproject.toml file):

pyi.py:1126: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                    if type(literal.n) is int:
                            ^~~~~~~~~
pyi.py:1275: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                    if isinstance(slc.n, int) and slc.n == 0:
                                  ^~~~~
pyi.py:1275: error: Right operand of "and" is never evaluated
[unreachable]
                    if isinstance(slc.n, int) and slc.n == 0:
                                                  ^~~~~~~~~~
pyi.py:1276: error: Statement is unreachable  [unreachable]
                        must_be_single = True
                        ^~~~~~~~~~~~~~~~~~~~~
pyi.py:1287: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                        and isinstance(slc.upper.n, int)
                                       ^~~~~~~~~~~
pyi.py:1288: error: Right operand of "and" is never evaluated
[unreachable]
                        and slc.upper.n in (1, 2)
                            ^~~~~~~~~~~~~~~~~~~~~
pyi.py:1290: error: Statement is unreachable  [unreachable]
                        can_have_strict_equals = slc.upper.n
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pyi.py:1313: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
    ...nstance(comparator, ast.Num) or not isinstance(comparator.n, int):
                                                      ^~~~~~~~~~~~
Found 8 errors in 1 file (checked 1 source file)

[hauntsaninja]

cc @ilevkivskyi this is from #13781

[ilevkivskyi]

FWIW this is intentional, isinstance() checks have always been determined by runtime behavior, not static subtyping. Previously promotions sometimes leaked, and caused various inconsistencies.

There is really no middle ground, if we enable promotions, there will be other issues, see, e.g. #6060. I would say current behavior is better because:

• It is more consistent with how isinstance() check behaves in other situations
• It may cause "false positives" with a strictness flag, while opposite causes false positives with default options.

[ilevkivskyi]

Also fix should be trivial, use a more precise type, like Union[int, complex].

[JelleZijlstra]

Without strictness flags, the current behavior leads code to be silently skipped as unreachable, which is arguably worse. I agree that there are few good options here though.

[hauntsaninja]

Maybe there's some hack, so we keep the better narrowing behaviour, but it doesn't affect reachability

[ilevkivskyi]

@hauntsaninja Actually, if you like hacks, I think there may be a (relatively) easy one: ignore the impossible intersection check in case if one type can be promoted to another one.

[ilevkivskyi]

Well, we should probably still prohibit (consider unreachable) something like:

if isinstance(x, int) and isinstance(x, float):
    ...

so it may require some work to implement.

[A5rocks]

Just making sure you all are aware of the idea in #11516

Specifically: in the if statement, split types up into this union and rely on already-existing narrowing.

Y'all are probably already aware, but just making sure this "solution" is considered. (I don't see any reasoning on that issue itself that would disqualify it)

[ilevkivskyi]

@hauntsaninja Actually after thinking some more, it seems to me we shouldn't change this, otherwise we may not flag legitimately unreachable code like

x = 1j
if isinstance(x, int):
    ...

that may potentially also hide bugs.

In general, most of the original motivation for type promotions was that practically all functions originally intended for float will also (accidentally) work for int. So to avoid forcing people to annotate all their argument types as e.g. Union[int, float], this decision was made. On the opposite, if a function actually differentiates between int and float, it should be annotated as taking Union[int, float] to emphasize this:

def weighted(a: float, b: float) -> float:
    return (a + 2 * b) / 3
weighted(1, 2)  # This is fine

def num_type(x: float) -> str:  # Bad, should be Union[int, float]
    if isinstance(x, int):
        return "Integer"
    return "Floating"

Anyway, it is still gray area and likely there will still be some people who likes (or used to) the old behavior. So we should clearly document this.