[Swift] Fix scalar-key lookupByKey comparison and sortVectorOf* comparator - #9204
Open
bianyeyu wants to merge 1 commit into
Open
[Swift] Fix scalar-key lookupByKey comparison and sortVectorOf* comparator#9204bianyeyu wants to merge 1 commit into
bianyeyu wants to merge 1 commit into
Conversation
…rator Scalar-key sorted vector support was never correctly implemented: lookupByKey compared the stored key against 0 instead of the search key, so non-zero-key lookups always returned nil; sortVectorOf* fed scalar key slots into the string-oriented Table.compare, so vectors were not sorted by key and high-bit scalar keys could trap the runtime. Use an explicit three-way comparison for lookups (unsigned subtraction would trap) and a direct scalar comparator for sorting.
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.
Summary
Swift code generation has never correctly implemented scalar-key sorted-vector support (it landed with the Swift generator in #5603). Two independent bugs make the feature unusable:
lookupByKeycompares the stored key against0instead of the search key. The scalar branch generatedso the binary search compares each stored key to
0, not to the search key. For unsigned keys every present non-zero value takes thecomp > 0branch (negative values takecomp < 0for signed keys), and a serialized0key is treated as a hit for any query. In the common case — a vector without a serialized0key — a lookup by a non-zero key returnsnil.sortVectorOf*feeds scalar key slots into the string-orientedTable.compare. The sort does not order the vector by key: the inline scalar bytes are treated as a string uoffset, so the comparator reads unrelated buffer bytes. If the decoded string length is negative,Table.compare'sfor i in 0...minValuetraps (Range requires lowerBound <= upperBound); whether that happens depends on nearby buffer contents — high-bit keys such as largeUInt64values make it more likely, but the same keys can also just sort in the wrong order.Both bugs have been present since the Swift generator landed (#5603, January 2020).
Fix
lookupByKey: generate an explicit three-way comparison between the stored key value and the search key. Subtraction is deliberately avoided — unsigned subtraction can underflow and traps in Swift.sortVectorOf*: branch on key type; scalar keys now compare the key values directly with<. The string-key code path is unchanged.Verification
scripts/generate_code.py; only the two Swiftmonster_test_generated.swiftfiles change, and all string-key paths are byte-identical.tests/swift/Tests/Flatbuffers/ScalarKeySortedVectorTests.swift:lookupByNonZeroScalarKey— lookups by non-zeroUInt16keys hit the right element; absent keys returnnil.sortVectorOfScalarKeySortsAscending—sortVectorOfStatproduces ascending key order and the result is searchable.sortVectorOfHighUInt64Keys—sortVectorOfReferrablewith0xFFFFFFFFFFFFFFF1…FFF3-class keys sorts into ascending order.