-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Serialization] Drop enums if a case payload can't be deserialized. #9720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Serialization] Drop enums if a case payload can't be deserialized. #9720
Conversation
Layout for an enum depends very intimately on its cases---both their existence and what their payload types are. That means there's no way to "partly" recover from failure to deserialize an individual case's payload type, the way we can partly recover from failing to deserialize an initializer in a class. Add deserialization recovery to enums by validating all of their payload types up front, and dropping the enum if we can't import all of the cases. This is the first time where we're trying to do deserialization recovery for a /type/, and that could have many more ripple effects than for a var/func/subscript/init. A better answer here might be to still import the enum but mark it as unavailable, but in that case we'd have to make sure to propagate that unavailability to anything that /used/ the enum as well. (In Swift, availability is checked based on use of the name, so if someone manages to refer to an enum using inferred types we'd be in trouble.) There is one case here that's not covered: if an enum case has a payload that references a type declaration nested within the enum, but then that nested type /itself/ can't be loaded for some reason, we have no way to check that up front, because we can't even try to load the nested type without loading its parent DeclContext (the enum). I can't think of an easy solution for this right now. (In the future, we'll be able to support dropping a single case for resilient enums. But we're not there right now.) rdar://problem/31920901
Slava, Joe, how risky do you think this is? @swift-ci Please test |
We were already doing this for top-level declarations; just use the same recovery code for operators. More rdar://problem/31920901
0e17fdd
to
f30800c
Compare
@swift-ci Please test |
Build failed |
Build failed |
Build failed |
Caveat: I don't think I understand the big picture of this change, so I might be talking nonsense here. But would it make sense (perhaps in the future) to treat an invalid payload type as though it were |
Unfortunately not—as long as we try to do smart layout based on the "spare bits" in the payload types, we can't change the type without changing the layout of the enum. I think you could do this for an indirect case (though not an indirect enum), or for a missing type with known layout, or (as stated) for a resilient enum where you have to go through functions to create or access cases, but for now I want to get the simple thing working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. You could perhaps partially recover when an indirect case's type is unavailable, but we could do that later.
…covery-for-enums [Serialization] Drop enums if a case payload can't be deserialized. rdar://problem/31920901
Filed SR-4926 describing what would need to be done to let indirect enum cases through. |
Layout for an enum depends very intimately on its cases—both their existence and what their payload types are. That means there's no way to "partly" recover from failure to deserialize an individual case's payload type, the way we can partly recover from failing to deserialize an initializer in a class. Add deserialization recovery to enums by validating all of their payload types up front, and dropping the enum if we can't import all of the cases.
This is the first time where we're trying to do deserialization recovery for a type, and that could have many more ripple effects than for a var/func/subscript/init. A better answer here might be to still import the enum but mark it as unavailable, but in that case we'd have to make sure to propagate that unavailability to anything that used the enum as well. (In Swift, availability is checked based on use of the name, so if someone manages to refer to an enum using inferred types we'd be in trouble.)
There is one case here that's not covered: if an enum case has a payload that references a type declaration nested within the enum, but then that nested type itself can't be loaded for some reason, we have no way to check that up front, because we can't even try to load the nested type without loading its parent DeclContext (the enum). I can't think of an easy solution for this right now.
(In the future, we'll be able to support dropping a single case for resilient enums. But we're not there right now.)
rdar://problem/31920901
Oh, and handle operators that can't be deserialized too.