Skip to content

Fix #996 #997

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

Closed
wants to merge 1 commit into from
Closed

Fix #996 #997

wants to merge 1 commit into from

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Nov 24, 2015

@bcdarwin @gvanrossum Please try and let me know if it works!

@gvanrossum
Copy link
Member

Sadly doesn't look like it fixes #991 for me. So maybe the two issues are not the same.

@bcdarwin
Copy link

This doesn't seem to crash any more, although it's still possible to omit the type arguments entirely (i.e., annotate a variable with type C rather than C[int]) and silently pass the checker -- not sure that's intended.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 29, 2015

Ah, a test case seems broken. Anyway, this will fix the crash but there could be other similar issues that this won't catch :-(

I believe that the root cause of the crash is that recently I changed mypy to type check code even if there are semantic errors. However, the type checker assumes certain invariants to hold after semantic analysis, such as the number of type arguments being correct, and this no longer is necessarily the case.

I'm thinking about a more general fix where we replace bad types with Any or something during semantic analysis, so that the type checker can still assume that things are correct. This would make the type checker simpler (actually no more complex than it is right now).

JukkaL added a commit that referenced this pull request Dec 14, 2015
Fixes #996. This is a more general alternative to PR #997.
@JukkaL
Copy link
Collaborator

JukkaL commented Dec 14, 2015

I pushed a hopefully more general fix to this issue, so I'm closing this PR. @kirbyfan64 thanks for the original PR!

@JukkaL JukkaL closed this Dec 14, 2015
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.

4 participants