Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Literal + Final don't work well on new semantic analyzer #6952

Closed
ilevkivskyi opened this issue Jun 7, 2019 · 3 comments · Fixed by #6961
Closed

Literal + Final don't work well on new semantic analyzer #6952

ilevkivskyi opened this issue Jun 7, 2019 · 3 comments · Fixed by #6961
Assignees
@Michael0x2a
Labels
bug mypy got something wrong priority-0-high semantic-analyzer Problems that happen during semantic analysis topic-literal-types

Comments

@ilevkivskyi
Copy link
Member

This test case fails

[case testLiteralFinalInferredAsLiteral]
from typing_extensions import Final, Literal

defer: Yes

var: Final = 42
def force(x: Literal[42]) -> None: pass
force(var)

class Yes: ...

with

Argument 1 to "force" has incompatible type "int"; expected "Literal[42]"

on new semantic analyzer

cc @Michael0x2a
@ilevkivskyi ilevkivskyi added bug mypy got something wrong priority-0-high topic-literal-types semantic-analyzer Problems that happen during semantic analysis labels Jun 7, 2019
@Michael0x2a Michael0x2a self-assigned this Jun 8, 2019
@Michael0x2a
Copy link
Collaborator

Michael0x2a commented Jun 8, 2019 •
edited
Loading

This seems to be related to how we handle the type context in some way.

Mypy seems to correctly remember that the type of var has a last_known_value of 42 in each iteration of the new semantic analyzer, but the type context seems to contain just Nones in the final iteration. So, var is treated as just an int at that stage, which is why we get the type error.

(It's a bit puzzling though -- I don't see why the behavior of the new semantic analyzer would influence the type checking phase in this way?)

I'll try digging a bit deeper.

Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Jun 8, 2019
@Michael0x2a
Make narrow_declared_type correctly handle Literal types
beeee2c 
This diff fixes the `narrowed_declared_type` function to correctly
handle cases where the declared type is actually a `Literal[...]`.

This fixes python#6952. In short, it
seems like using the new semantic analyzer somehow made mypy call
[the `narrow_type_from_binder(...)` function][0] where it previously
did not.

Specifically, mypy passed in the `var` expression and a `known_type` of
`Literal[42]`, we got a restriction of `builtins.int` (with a
`last_known_value` of 42), and attempted to narrow the `Literal[42]` on
line 3512.

But due to the missing case in `narrow_declared_type`, we ended up
returning just the int.

  [0]: https://github.com/python/mypy/blob/master/mypy/checkexpr.py#L3504
@Michael0x2a Michael0x2a mentioned this issue Jun 8, 2019
Merged
@ilevkivskyi ilevkivskyi mentioned this issue Jun 13, 2019
Merged
ilevkivskyi added a commit that referenced this issue Jun 13, 2019
@ilevkivskyi
New semantic analyzer: Keep explicit type and inferred status in sync (
d90eb18 
#6988)

This is (kind of) an actual fix for #6952

The problem is that previously `unwrap_final()` unconditionally set `s.type` to `None` for bare `Final`, even if the type was inferred from a simple literal r.h.s. on a previous iteration. At the same time `lvalue.is_inferred_def` remained `False`, so that the type was not inferred on the second iteration.

This way `s.type` and `lvalue.is_inferred_def` may get out of sync depending on the number of iterations, thus causing weird behavior in type checker.

I generally can say the current way of storing the inferred status is error-prone, but we already have #6458 to track this.
@Michael0x2a
Copy link
Collaborator

@ilevkivskyi -- actually, should we keep this one open? Both of our PRs are only kind of fixes this.

Or maybe it's sufficient to just keep this closed and track just #6458 instead.

@ilevkivskyi
Copy link
Member Author

#6458 is sufficient.

PattenR pushed a commit to PattenR/mypy that referenced this issue Jun 23, 2019
@ilevkivskyi @PattenR
New semantic analyzer: Keep explicit type and inferred status in sync (
ff5f5fa 
python#6988)

This is (kind of) an actual fix for python#6952

The problem is that previously `unwrap_final()` unconditionally set `s.type` to `None` for bare `Final`, even if the type was inferred from a simple literal r.h.s. on a previous iteration. At the same time `lvalue.is_inferred_def` remained `False`, so that the type was not inferred on the second iteration.

This way `s.type` and `lvalue.is_inferred_def` may get out of sync depending on the number of iterations, thus causing weird behavior in type checker.

I generally can say the current way of storing the inferred status is error-prone, but we already have python#6458 to track this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Michael0x2a Michael0x2a

Labels
bug mypy got something wrong priority-0-high semantic-analyzer Problems that happen during semantic analysis topic-literal-types
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make narrow_declared_type correctly handle Literal types Michael0x2a/mypy
2 participants
@Michael0x2a @ilevkivskyi