isinstance checks cause type declarations to leak from the binder

[ddfisher]

The problem with this is most easily visible with list comprehensions:

lst = [1, 2, 3]  # type: List[Union[int, str]]
[x if isinstance(x, int) else 0 for x in lst]
reveal_type([x for x in [4, 5, 6]])

$ python3 -m mypy ~/test.py
test.py:3: error: Revealed type is 'builtins.list[Union[builtins.int, builtins.str]]'

This happens because of this line in the binder, which always adds the declared type of a variable to the topmost frame whenever a binding for it is pushed: https://github.com/python/mypy/blob/master/mypy/checker.py#L127

This is obviously a bug, but it's not yet clear what the best way to fix the issue is. The binder is small, but complicated -- perhaps the best fix is it rewrite it in a more understandable/robust way?

This issue is the reason mypy still has a self-typecheck error with #1562.

[gvanrossum]

There's a Python 2 vs. 3 issue here too: in Python 2, the original 'x' value leaks out into the outer scope; in Python 3, it doesn't.

[ddfisher]

Right. But I think I'd be pretty reasonable to tell people not to use that in Python 2.

[rwbarton]

Hi @ecprice :)

[rwbarton]

#1044 looks like it might be the same issue.

[ecprice]

I think the top commit of my typebinder branch fixes this. It probably cleanly applies to master, but I haven't checked.

[rwbarton]

I'm actually not going to fix this right now, in part because the correct behavior isn't entirely clear; should we treat comprehension loop variables as deleted after the comprehension (maybe only in Python 2 mode)?

[gvanrossum]

In CPython, comprehension variables are deleted only in Python 3. But I'm fine with mypy treating them as deleted everywhere -- even in Python 2 mode. If you reference a leaked comprehension variable your code is very smelly. Another aspect of comprehensions is that we should not insist on the same variable having the same type throughout a function. Also note that generator expressions are different -- the are always their own scope even in Python 2.

[rwbarton]

Oh right and there's also the tricky bit that while list comprehensions are evaluated immediately (of course), only the first sequence in a generator comprehension is evaluated immediately. So technically we should not use information from the binder for any sequence expression past the first. (Like we don't use information from the binder when checking the body of a lambda, since the variables it refers to may have changed value by the time the lambda is executed.)

[elazarg]

The output now is error: Revealed type is 'builtins.list[builtins.int*]' as I assume it should be.

[ilevkivskyi]

I am not sure how this got fixed, but the current behaviour looks right to me.