Fix some daemon crashes involving classes becoming generic #8157
Conversation
Fixes #3279. Also fixes another related crash.
|
|
||
| [out] | ||
| == | ||
| b.py:7: note: Revealed type is 'def () -> b.C[Any]' |
There was a problem hiding this comment.
What about a similar test case where a class goes from generic to non-generic?
| for i in range(len(instance.args)): | ||
| # N.B: We use zip instead of indexing because the lengths might have | ||
| # mismatches during daemon reprocessing. | ||
| for tvar, mapped_arg, instance_arg in zip(tvars, mapped.args, instance.args): |
There was a problem hiding this comment.
It's kind of unfortunate that we have to do this. Another option might be to make args a property and have it adjust the number of args dynamically if the TypeInfo has changed. This could have performance impliciations, however.
There was a problem hiding this comment.
This is unfortunate. What about returning something obviously wrong (like we do in fixup.py) for the temporary results? Otherwise this may mask some other bugs.
Also why not fix a similar issue in mypy.join.join_instances()? What about a test where number or type variables changes from 2 to 1 and/or vice versa?
|
I didn't do join_instances because I couldn't manage to make it crash and because I had convinced myself from a glance that it couldn't break because of some other side condition, but I don't think that is actually true. I don't love the idea of adjusting things as an |
mypy/meet.py
Outdated
| @@ -486,13 +486,15 @@ def visit_type_var(self, t: TypeVarType) -> ProperType: | |||
| def visit_instance(self, t: Instance) -> ProperType: | |||
| if isinstance(self.s, Instance): | |||
| si = self.s | |||
| if t.type == si.type: | |||
| if t.type == si.type and len(t.args) == len(si.args): | |||
There was a problem hiding this comment.
Btw why do we need the and ... here if we anyway use zip() below?
There was a problem hiding this comment.
Oh, no, we definitely do not. The and was my first approach for fixing this, before I decided I liked the zip version more. (This check does work, though, so that could be an approach?)
Fixes #3279. Also fixes another related crash.