-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix crash with function redefinition #14064
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems OK as quick fix but the error message is still pretty bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't want to block this, but I think I have a better solution, see comment below.
test-data/unit/check-functions.test
Outdated
# weirdly, needs to be an empty tuple. empty lists and various other special assignments | ||
# don't trigger the bug | ||
cond = () | ||
if cond: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A simpler test would be just if False:
The problem is that mypy considers if branch unreachable, and therefore doesn't type check it, and therefore the original definition doesn't get its type inferred (and stays None
).
I think a better solution is to simply not show any error in this case:
- It will be more consistent with not showing error in unreachable code
- Technically, there is no redefinition, only one definition is ever executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note however, this will be inconsistent with foo: int = 1
, where user will get an error, even if the first definition is unreachable, but IMO this is OK.
Btw it is a wider problem, some code may be semantically reachable (because types of expressions in branches are not known), but not type checked, because during type-checking expression type is inferred as something false-y. A possible option to explore in the future (not now) is to still actually type check unreachable code, only for the purpose of type inference, but don't show any errors there. |
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Fixes #14027 (issue was surfaced by #13509)