[WIP] Tighten Lvalues and remove all references to Node #2221
Conversation
This change prepares the ground for making it Dict[Expression, Type] instead of Dict[Node, Type]. TypeMap has been moved to mypy.types, following existing dependencies in code that uses it.
|
Sorry the name is bad, but I haven't been following what you've been doing here closely, but I think it doesn't make sense to merge |
|
Thanks @rwbarton. I have noticed (after posting this PR) that there are different maps with different purposes. I still think there should be a single name for each purpose, but I am not that certain. |
|
|
||
| class Frame(Dict[Any, Type]): | ||
| pass |
There was a problem hiding this comment.
I don't understand why is it needed. This can be fixed regardless of this PR.
| type = node.node.type | ||
| def get_declaration(self, expr: BLval) -> Type: | ||
| if isinstance(expr, (RefExpr, SymbolTableNode)) and isinstance(expr.node, Var): | ||
| type = expr.node.type |
There was a problem hiding this comment.
The test here is never true, and I think the original (changed in one of my previous PRs) was never true either. I wonder why.
| @@ -1812,7 +1820,7 @@ def visit_decorator(self, e: Decorator) -> Type: | |||
| if d.fullname == 'typing.no_type_check': | |||
| e.var.type = AnyType() | |||
| e.var.is_ready = True | |||
| return NoneTyp() | |||
| return None | |||
There was a problem hiding this comment.
This mistake can be fixed regardless of this PR.
|
Will you let us know when you're ready for a review? |
|
@gvanrossum this PR is ready now, as far as I'm concerned. However, since it is so big, and since introducing Lvalue as a separate type has implications on the old parser, I am splitting it to smaller chunks, without "real" Lvalue yet. I will close this one and will submit a new PR momentarily. I also have certain questions about the specific details of the code, and about design decisions, but I don't know what's the right place to ask them; they are probably too minor for dedicated issues. |
TypeMap is tighten to Dict[Expression, Type], which will help removing most of the remaining references to Node. It will also allow movingliteralandliteral_hashfrom Node to Expression (leaving only line and column as data in Node), possibly making it easier to perform the NB regarding TypeMap (see inside).One culprit is that TypeMap is Optional everywhere now. Admittedly I don't understand why is it Optional at all.[I intend to use this PR as a reference for changes in much smaller chunks]
All references to Node are removed from everywhere except mypy.node.
Make Lvalue a separate Union type. This has many implications; I think the PR can serve as a demonstration for now.
The need for dedicated LvalueTuple and LvalueStar is seen through the casts needed when accessing their elements. Perhaps making them generic can help.