[ https://issues.apache.org/jira/browse/THRIFT-4989?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16964810#comment-16964810 ]
Jens Geyer commented on THRIFT-4989: ------------------------------------ {quote} The Swift implementation of TCompactProtocol assumes that the properties of autogenerated classes for Thrift Structs will be ordered by field key [...] {quote} I'd say that reyling on *any* particular field order is - at best - relying on undocumented behaviour. I'd even go further and say it is a bug, since other language bindings may not follow such assumptions today and/or in the future.. TL&DR: Its not about the question which order is correct, *rather we should question assuming any order* and fix that. > Run time exception when using TCompactProtocol > ---------------------------------------------- > > Key: THRIFT-4989 > URL: https://issues.apache.org/jira/browse/THRIFT-4989 > Project: Thrift > Issue Type: Bug > Components: Swift - Compiler > Affects Versions: 0.13.0 > Reporter: DAvid Furey > Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > When serialising a struct using the TCompactProtocol we have discovered a > run-time error on line > [https://github.com/apache/thrift/blob/08a6eb6e1f21aa15682e414e124d7ee1a51a39c7/lib/swift/Sources/TCompactProtocol.swift#L474] > The Swift implementation of TCompactProtocol assumes that the properties of > autogenerated classes for Thrift Structs will be ordered by field key, but > the current behaviour of the generator preserves the order from the original > thrift definitions These are not required to be ordered by field key. > Either the Swift lib needs to check for out-of-order properties, or the > generator should order properties. Since the generator is only executed once > and the Swift lib runs every time a message is serialised, I think it makes > more sense to make this change in the generator. > I've made the necessary change, and will soon open a PR. -- This message was sent by Atlassian Jira (v8.3.4#803005)