Conversation
This comment has been minimized.
This comment has been minimized.
|
Non-trivial results in |
This comment has been minimized.
This comment has been minimized.
|
Although remaining errors are kind of correct, they are sill weird. It looks like they appear because previously class C
@no_type_check
def __new__(cls): ...resulted in |
|
OK, so it turns out there was an existing bug (inconsistency) with While looking at this I found second place where we don't handle not-ready types properly (decorated overloads), again I simply don't cache in this case, since a proper fix may be non-trivial (I also update the relevant TODO item). I update the PR description to mention this. |
This comment has been minimized.
This comment has been minimized.
sterliakov
left a comment
There was a problem hiding this comment.
LG! The idea makes sense, and the bench on GHA shows 2.6-2.9% win for selfcheck. Nice! I'm a bit worried that it uncovered at least three semi-unrelated issues - can there be others, not revealed by primer runs? It may be too much fun to debug caching-related bugs later...
mypy/typeops.py
Outdated
| else: | ||
| builtins_type = named_type("builtins.type") | ||
|
|
||
| fallback = info.metaclass_type or builtins_type |
There was a problem hiding this comment.
Can we keep it lazy (only call named_type if info.metaclass_type is None)?
|
@sterliakov Of course there can be more. But those are all real bugs, we would need to fix them sooner or later anyway. In an ideal bug-free world, we should be able to always cache. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This gives almost 4% performance boost (Python 3.12, compiled). Note there is an old bug in
type_object_type(), we treat not ready types asAnywithout deferring, I disable caching in this case.Unfortunately, using this in fine-grained mode is tricky, essentially I have three options:
__init__/__new__I decided to choose the last option. I think it has the best balance complexity/benefits.