Conversation
The generated deserializers track duplicate fields with a HashSet, which hashes each insertion and allocates storage as fields are recorded. Add wkt::internal::SeenFields<N>, a bitset backed by [u64; N], to support duplicate detection without those allocations or hashing. The generator will assign each field a consecutive index and choose N as field_count.div_ceil(64). insert() returns false for an already recorded field, matching HashSet::insert(). Both new() and insert() can be used in const contexts. Multiple words support messages with more than 64 fields. This change adds the helper and its tests. Generator adoption and regenerated code will follow in separate PRs. The generator must assign the same index to a field's JSON name and original proto name to preserve existing duplicate handling. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new SeenFields utility in src/wkt/src/internal/seen_fields.rs (and exposes it in internal.rs). SeenFields is a fixed-size bitset represented as an array of u64 words, designed to track which fields of a message have been encountered during deserialization to detect duplicates without dynamic allocation. It includes comprehensive unit tests verifying its behavior, including boundary conditions and const-context operations. I have no feedback to provide as the implementation is clean, well-tested, and idiomatic.
|
/gcbrun |
|
I created an issue and referenced said issue in your description. I have specific requests for the PR (here) and some questions about the improvements, which we should continue on that issue. It would be nice to see the changes to the generator and the resulting output before merging this. Can you send those as draft PRs and reference them here? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6787 +/- ##
=======================================
Coverage 96.98% 96.99%
=======================================
Files 326 327 +1
Lines 106652 106709 +57
=======================================
+ Hits 103441 103505 +64
+ Misses 3211 3204 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
The generated deserializers track duplicate fields with a HashSet, which hashes each insertion and allocates storage as fields are recorded. Add wkt::internal::SeenFields, a bitset backed by [u64; N], to support duplicate detection without those allocations or hashing.
The generator will assign each field a consecutive index and choose N as field_count.div_ceil(64). insert() returns false for an already recorded field, matching HashSet::insert(). Both new() and insert() can be used in const contexts. Multiple words support messages with more than 64 fields.
This change adds the helper and its tests. Generator adoption and regenerated code will follow in separate PRs. The generator must assign the same index to a field's JSON name and original proto name to preserve existing duplicate handling.
Towards #6788