Skip to content

Re-broaden type to Any if it started as Any but was narrowed in an enclosing frame #3361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 11, 2017

Conversation

lincolnq
Copy link
Contributor

Fixes #3338.

  • Note 1: I don't understand mypy very well yet so be warned that this may break things I don't understand.
  • Note 2: it looks like I was confused in the original bug report, and attributing the problem to strict-optional was actually a red herring. So I created a new test case for which strict-optional isn't needed.

My investigation notes:

The explanation is that the bug is caused by an interaction between most_recent_enclosing_type and its caller, assign_type. assign_type has a special case for when the most recent enclosing type is Any, commented with the following note: "If x is Any and y is int, after x = y we do not infer that x is int. This could be changed." The special case does nothing (the normal case assigns the inferred type to the variable).

In my test case, most_recent_enclosing_type misses the frame which reassigns v because int is not a subtype of str, and so returns Any and so the special case applies (this explains why declaring with type Union doesn't reproduce the problem). So it doesn't assign any new type for this assignment, causing the bug.

I took the approach of fixing that special case so if type is not bound to Any at the moment of assignment, it re-binds it to Any (or, really, whatever is the result of most_recent_enclosing_type).

Another approach might be to remove the subtype check from most_recent_enclosing_type. https://github.com/python/mypy/compare/master...lincolnq:lincoln/remove-subtype-check-from-binder?expand=1 is where I tried that. I thought that change was more aggressive / likely to break things, and I didn't understand why the subtype check was put there in the first place. But it has the desirable (to me) property of inferring int on the reassignment rather than Any.

Both approaches pass the automated tests.

@gvanrossum
Copy link
Member

Thanks for your contribution! We're currently getting ready for PyCon but hopefully we'll get to reviewing this PR soon.

Copy link
Collaborator

@ddfisher ddfisher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Thanks for the detailed explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strict optional: Any narrowing to None in a conditional gets "stuck" at None
3 participants