Fix crash due to checking type variable values too early #4384
Conversation
Move type variable checks which use subtype and type sameness checks to happen at the end of semantic analysis. The implementation also adds the concept of priorities to semantic analysis patch callbacks. Callback calls are sorted by the priority. We resolve forward references and calculate fallbacks before checking type variable values, as otherwise the latter could see incomplete types and crash. Fixes #4200.
| # (after pass 3): | ||
|
|
||
| # Fix forward references (needs to happen first) | ||
| PRIORITY_FORWARD_REF = 0 |
There was a problem hiding this comment.
Just a minor suggestion: I would use plural PRIORITY_FORWARD_REFS to be more consistent with other constants.
There was a problem hiding this comment.
Why not just make this an enum? That should be possible now that mypy no longer supports being run on 3.3.
There was a problem hiding this comment.
I decided to continue using the old idiom, since I like consistency and I don't want to change all the existing constants into enums :-)
| or tvar.upper_bound.type.fullname() != 'builtins.object'): | ||
| # Some restrictions on type variable. These can only be checked later | ||
| # after we have final MROs and forward references have been resolved. | ||
| self.indicator['typevar'] = True |
There was a problem hiding this comment.
Unrelated idea: indicator is a good candidate to be a TypedDict (but it is not in typing yet).
There was a problem hiding this comment.
Yes, or maybe just a regular object.
There was a problem hiding this comment.
Yes, or maybe just a regular object.
:-) Good point!
mypy/typeanal.py
Outdated
| @@ -1036,3 +996,58 @@ def make_optional_type(t: Type) -> Type: | |||
| return UnionType(items + [NoneTyp()], t.line, t.column) | |||
| else: | |||
| return UnionType([t, NoneTyp()], t.line, t.column) | |||
|
|
|||
|
|
|||
| class TypeVariableChecker(TypeTranslator): | |||
There was a problem hiding this comment.
I am wondering if this class should be in semanal_pass3.py.
|
I was testing this branch and noticed an issue with where is recursive types error claimed to come from. I can observe the same in reproduction I made for #4385. I am not sure if this is because of this pull request, but this pull request make it even possible to observe the issue, so I am reporting it here. So if you ran my reproduction, If you look carefully, you can see that the line numbers are OK, but the file reported is wrong. It should be This is problematic because it is hard to silence these errors. The only way I found was to use |
Move type variable checks which use subtype and type sameness
checks to happen at the end of semantic analysis.
The implementation also adds the concept of priorities to
semantic analysis patch callbacks. Callback calls are
sorted by the priority. We resolve forward references and
calculate fallbacks before checking type variable values,
as otherwise the latter could see incomplete types and crash.
Fixes #4200
Fixes #3977