Fix some crashes in dataclasses #8271
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Store the type of an attribute in `DataclassAttribute` so we don't need to pull it out of `TypeInfo` in a fragile way (since the child might try to override it in a way that breaks things). This also allows us to get rid of some pretty dodgy code having to do with InitVars (that could cause an incremental mode crash.) Fixes #6809 and an unreported incremental bug
JukkaL
reviewed
Jan 13, 2020
| assert attr.is_init_var and not attr.is_in_init | ||
| continue | ||
| if node.type is None: | ||
| if attr.type is None: |
There was a problem hiding this comment.
What if the some attribute has no type? We seem to handle this case during deserialization, so maybe it can happen here as well. Could this result in always deferring for some attribute? If not, maybe we should require that the serialized type is not None?
There was a problem hiding this comment.
We only consider something as being an attribute if it has a type annotation, so the type missing is (should be) always due to resolution issues that can be fixed by deferring.
I think we can only serialize things with non-None types, yeah.
JukkaL
approved these changes
Jan 16, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Store the type of an attribute in
DataclassAttributeso we don'tneed to pull it out of
TypeInfoin a fragile way (since the childmight try to override it in a way that breaks things). This also
allows us to get rid of some pretty dodgy code having to do with
InitVars (that could cause an incremental mode crash.)
Fixes #6809 and an unreported incremental bug