Decoding choice elements that can hold empty structs #120
Merged
MaxDesiatov merged 3 commits intoCoreOffice:masterfrom Jul 31, 2019
Merged
Decoding choice elements that can hold empty structs #120MaxDesiatov merged 3 commits intoCoreOffice:masterfrom
MaxDesiatov merged 3 commits intoCoreOffice:masterfrom
Conversation
MaxDesiatov
approved these changes
Jul 31, 2019
Collaborator
MaxDesiatov
left a comment
There was a problem hiding this comment.
Fantastic, thank you @bwetherfield!
Collaborator
Author
|
So quick! Thank you, @MaxDesiatov |
arjungupta0107
pushed a commit
to salido/XMLCoder
that referenced
this pull request
Jun 26, 2020
## Introduction In merging in CoreOffice#119, we fixed most but not quite all of CoreOffice#91! Decoding of _null_ choice elements (represented as enums with empty struct associated values on the Swift side) still results in errors. This PR adds a test to demonstrate and fixes this issue by wrapping each `NullBox` inside of a `SingleKeyedBox` at the `merge` phase (called by `transformToBoxTree`). ## Motivation One of the main lessons from CoreOffice#119 was that we have to wrap choice elements in the decoding phase to hold onto their keys. The keys are needed for directing us to the correct branch of the do-catch pyramid used for decoding. ```swift private enum Entry: Equatable { case run(Run) case properties(Properties) case br(Break) } extension Entry: Decodable { private enum CodingKeys: String, XMLChoiceCodingKey { case run, properties, br } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) do { self = .run(try container.decode(Run.self, forKey: .run)) } catch { do { self = .properties(try container.decode(Properties.self, forKey: .properties)) } catch { self = .br(try container.decode(Break.self, forKey: .br)) } } } } ``` where one of the associated values could be an empty struct (represented by null): ```swift private struct Break: Decodable {} ``` Although we _can_ throw out keys for non-choice null elements, a mechanism is needed for holding onto the keys while transforming from the `XMLCoderElement` tree to the `boxTree`. Only later will we know if the key is needed (if this wrapped element is transformed to a `ChoiceBox`); if not, we will be able to throw out the key. ## Proposed solution The Public API is unchanged. On the implementation side, we catch `NullBox` values in `merge` and wrap them in `SingleKeyedBox` instances. ## Detailed Design In `merge`, we wrap each `NullBox` in a `SingleKeyedBox` with the appropriate key bundled in. An `XMLChoiceDecodingContainer` can be constructed from the `SingleKeyedBox` by converting it to a `ChoiceBox` (just transferring over the contents) - as normal. In `XMLKeyedDecodingContainer`, when preparing the `elements` for concrete decoding, we unwrap all `SingleKeyedBox` values that may be contained therein, as any choice elements contained would have already been transformed to a `ChoiceBox` by this point in decoding: any stray `SingleKeyedBox` wrappers can thus be thrown out. ## Source compatibility This is purely an additive change.
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
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.
Introduction
In merging in #119, we fixed most but not quite all of #91! Decoding of null choice elements (represented as enums with empty struct associated values on the Swift side) still results in errors. This PR adds a test to demonstrate and fixes this issue by wrapping each
NullBoxinside of aSingleKeyedBoxat themergephase (called bytransformToBoxTree).Motivation
One of the main lessons from #119 was that we have to wrap choice elements in the decoding phase to hold onto their keys. The keys are needed for directing us to the correct branch of the do-catch pyramid used for decoding.
where one of the associated values could be an empty struct (represented by null):
Although we can throw out keys for non-choice null elements, a mechanism is needed for holding onto the keys while transforming from the
XMLCoderElementtree to theboxTree. Only later will we know if the key is needed (if this wrapped element is transformed to aChoiceBox); if not, we will be able to throw out the key.Proposed solution
The Public API is unchanged. On the implementation side, we catch
NullBoxvalues inmergeand wrap them inSingleKeyedBoxinstances.Detailed Design
In
merge, we wrap eachNullBoxin aSingleKeyedBoxwith the appropriate key bundled in. AnXMLChoiceDecodingContainercan be constructed from theSingleKeyedBoxby converting it to aChoiceBox(just transferring over the contents) - as normal. InXMLKeyedDecodingContainer, when preparing theelementsfor concrete decoding, we unwrap allSingleKeyedBoxvalues that may be contained therein, as any choice elements contained would have already been transformed to aChoiceBoxby this point in decoding: any straySingleKeyedBoxwrappers can thus be thrown out.Source compatibility
This is purely an additive change.