Bypass class attribute lookup for PEP 695 typevars #20292
Conversation
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
sterliakov
left a comment
sterliakov
left a comment
There was a problem hiding this comment.
Nice one! This part definitely should not use lookup, we are only interested in locals, and (except for one bad corner case) only in the most recent locals entry. Traversing further is only needed when we have
class Foo[T]:
class Bar[T]: # Error here, but typevar is still needed
...
|
|
||
| def is_defined_type_param(self, name: str) -> bool: | ||
| def get_defined_type_param(self, name: str) -> TypeVarLikeExpr | None: | ||
| for names in self.locals: |
There was a problem hiding this comment.
Ough, I spelled that out but didn't realize that this goes in the wrong direction (?). We are typically interested in the most recent locals frame, so shouldn't this be
| for names in self.locals: | |
| for names in reversed(self.locals): |
? It didn't matter when we only requested a boolean "is defined" result, but now could potentially resolve to a wrong symbol. I don't have a testcase for that, but anyway the variable will normally be found in the most recent local frame, so checking it first makes more sense? Upd: no, it cannot resolve to the wrong symbol, we only accept type variables, and type param redefinitions are never added to symtable, but maybe it's still better to avoid the unnecessary work? reversed is fast
| if names is None: | ||
| continue | ||
| if name in names: |
There was a problem hiding this comment.
And just to save a bit on branching,
| if names is None: | |
| continue | |
| if name in names: | |
| if names is not None and name in names: |
2 participants
Fixes #20283
This is a bit gross, but I'm not sure how to better do this. The problem is that we look up class attributes before looking at locals, but then put the type vars in a new locals scope. I guess one idea would be to define the typevars at usage time? But this is less of a change.