Map Rust u8/u16 to Go uint8/uint16 instead of signed int - #292
Open
jaideeppyne wants to merge 1 commit into
Open
jaideeppyne wants to merge 1 commit into
jaideeppyne wants to merge 1 commit into
Conversation
The Go generator folded u8 and u16 into the signed `int` arm, while u32 maps to `uint32` and u64 to `uint64`. This silently drops the unsignedness for the two smallest unsigned types, which is internally inconsistent and lets a Go service place a negative or out-of-range value into a field that is unsigned on the Rust side — serde then rejects it on deserialization. The repo's own `use_correct_integer_types` fixture already establishes the intended contract: for the same u8/u16 inputs, Swift emits UInt8/UInt16 and Kotlin emits UByte/UShort. Map u8 -> uint8 and u16 -> uint16 so Go matches. Regenerated the Go snapshots; the only changes are int -> uint8/uint16 for u8/u16 fields (i32 fields stay `int`). Full typeshare-core suite passes.
This branch has not been deployed
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.
What
The Go generator maps Rust
u8andu16to signed Goint, even though it mapsu32→uint32andu64→uint64.Why it's a bug
u32/u64keep their unsignedness butu8/u16don't — and Go has nativeuint8/uint16, so there's no Go-idiom reason to widen them to signedint.use_correct_integer_typesfeedsu8/u16and expects type fidelity: Swift emitsUInt8/UInt16, Kotlin emitsUByte/UShort. Only Go drops the signedness.-1or a value above the type's range into an "unsigned" field; serde on the Rust side then rejects it on deserialize. The correctuint8/uint16mapping lets Go's own type system enforce what serde already requires.Currently, right next to each other in
use_correct_integer_types/output.go:How
(Kept surgical to the unambiguous unsigned bug;
usize→intis a separate, platform-width-dependent question and left out of scope.)Tests
Regenerated the Go snapshots with
UPDATE_EXPECT=1. The diff is onlyint→uint8/uint16foru8/u16fields across 7 fixtures (e.g.Age,Red,OptionalU16→*uint16, anduse_correct_integer_types'sE/F);i32fields such asExtraSpecialField1correctly stayint. The snapshot suite is the regression guard — reverting onlygo.rs(keeping the corrected snapshots) makes the Go snapshot tests fail. Fulltypeshare-coresuite passes (361 tests),cargo fmt --checkis clean.Disclosure: this change was prepared with AI assistance and reviewed/verified by me before submission.