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

Make `refresh_top_level()` more like `visit_file()` in the third pass #6098

Merged
merged 3 commits into from Jan 2, 2019

Conversation

Projects
None yet
2 participants
@ilevkivskyi
Copy link
Collaborator

ilevkivskyi commented Dec 22, 2018

Fixes #6032 (first part)

This is a very straightforward fix. I am actually surprised no one found this before (maybe because mypy daemon is not so widely used yet).

In future, it would be great to have a mechanism to avoid such mistakes, by somehow making refresh_top_level() to do all the special things visit_file() does. I don't have any ideas now, but it probably makes sense to consider this during semantic analyzer refactoring (another possible solution is to avoid any such special things, but this should be clearly documented).

@ilevkivskyi ilevkivskyi requested review from msullivan , JukkaL and gvanrossum Dec 22, 2018

@ilevkivskyi

This comment has been minimized.

Copy link
Collaborator Author

ilevkivskyi commented Dec 22, 2018

Argh, forgot to run tests. Will fix them now.

Ivan Levkivskyi
@ilevkivskyi

This comment has been minimized.

Copy link
Collaborator Author

ilevkivskyi commented Dec 22, 2018

OK, now everything should be fine. I also updated PR description.

@JukkaL

JukkaL approved these changes Jan 2, 2019

Copy link
Collaborator

JukkaL left a comment

Looks good! Hopefully this code will be cleaned up as part of the semantic analyzer redesign. My only suggestion is to split this into two separate PRs. Feel free to merge the second PR once builds are green without further review.

# are not real independent targets, and should be processed when
# the enclosing synthetic type is processed.
old_recurse = self.recurse_into_functions
self.recurse_into_functions = True

This comment has been minimized.

@JukkaL

JukkaL Jan 2, 2019

Collaborator

If this is not needed to fix the test cases, could you move this to a separate PR?

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Jan 2, 2019

Author Collaborator

Hm, I think I can split it by moving one of the tests added to the second PR.

Ivan Levkivskyi

@ilevkivskyi ilevkivskyi merged commit fa41d19 into python:master Jan 2, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:fix-daemon-forward-crash branch Jan 2, 2019

ilevkivskyi pushed a commit to ilevkivskyi/mypy that referenced this pull request Jan 2, 2019

ilevkivskyi added a commit that referenced this pull request Jan 2, 2019

Second part of #6098 (#6126)
This PR fixes insufficient traversal of synthetic `TypeInfo`s in fine grained mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment