Oneof cleanup and generation improvements. #477
Conversation
If the oneof fields are continuous in the parent, optimize the generation to avoid having to pass a start/stop range.
Handle gaps in the oneof field numbers not needing to use ranges when traversing.
| let firstIndex = parentFieldNumbersSorted.index(of: first)! | ||
| var isContinuousInParent = true | ||
| for i in 0..<fields.count { | ||
| if fieldsSortedByNumber[i].number != parentFieldNumbersSorted[firstIndex + i] { |
There was a problem hiding this comment.
I find this logic a little confusing. It would be clearer (to me) to use the same logic as you use for extension checks below, namely: Are any of the parent field numbers between our first and last field numbers?
There was a problem hiding this comment.
The catch is the parent list includes the fields in this oneof, so it would be a little more complicated.
Another solution (which I didn't try coding) could be to see if sortedFields == parentFields[firstIndex..<firstIndex+fields.count].
There was a problem hiding this comment.
Or subtract our fields from the parent fields (set logic), then do the same check as extensions?
There was a problem hiding this comment.
Was able to use the slice, how does this seem?
tbkka
left a comment
tbkka
left a comment
There was a problem hiding this comment.
This is a nice cleanup. The generated code here reads a lot more naturally.
| break | ||
| } | ||
| } | ||
| var isContinuousInParent = sortedOneofFieldNumbers == Array(parentFieldNumbersSorted[firstIndex..<(firstIndex + fields.count)]) |
There was a problem hiding this comment.
Compiler doesn't like the compare without the Array() around it (saying you can't compare an Array with and ArraySlice.
2 participants
Fixes #369.