Skip to content

[R38] Serialize physical ValueTuple fields once - #39

Open
JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r38
Open

JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r38

Conversation

@JetF0x

@JetF0x JetF0x commented Sep 12, 2026

Copy link
Copy Markdown

Addresses review finding R38.

Fixes R38. Generated (int Z, int A) values previously contained four integers (A, Item1, Item2, Z), while the runtime upgrade and collection codecs used the two physical fields. Typed upgrade reads failed with unconsumed bytes, and upgrade-written values did not apply through generated deserialization. Long tuples also duplicated flattened Item8 and later fields alongside Rest, even without aliases.

Share physical field selection across property, message-parameter, and nested-struct models. Map the first seven logical tuple elements to CorrespondingTupleField, then recurse into physical Rest through the existing struct model. Preserve ordinal field ordering for all structs and preserve aliases in declared C# types. Roslyn 5.6 probes confirmed that TupleElements is flattened and that TupleUnderlyingType still exposes flattened fields, so neither can simply be enumerated as runtime storage. Runtime reflection codecs already select the correct fields and are unchanged. The message-ID algorithm is unchanged.

Validation

  • Added 77 regression cases. The same tests against the original generator had 63 failures and 14 passes; the corrected generator passes 77/77. Test schemas compile with both Cat.Network analysis and generation enabled.
  • Golden bytes cover unnamed, named, explicit default ItemN and mixed aliases; nullable and nested tuples; 8-, 9- and 15-element tuples; explicit ValueTuple<T1,...,T7,TRest>; nullable nested tuples inside Rest; and tuples inside an ordinary struct. Examples: named pair 16 → 8 bytes, unnamed eight-int tuple 36 → 32, named nine-int tuple 80 → 36, and named fifteen-int tuple 156 → 60.
  • Verify generated property bytes against golden bytes and reflection-based upgrade/list codecs; generated-to-upgrade reads and upgrade-to-generated writes; list item and dictionary key/value round trips; real v0 unnamed → v1 named, v1 → v2 alias-renamed, and sequential v0 → v2 migrations.
  • Verify exact outgoing RPC/broadcast parameter bytes, canonical incoming dispatch, nested/long/nullable values, and preservation of the existing alias-sensitive message IDs.
  • dotnet test Cat.Network.sln -c Release --no-restore: 305 passed (232 runtime, 20 generator, 53 analyzer), zero failures or skips. SDK 10.0.400, .NET 8.0.19 runtime, Roslyn 5.6.0, C# 14. git diff --check passed. The existing runtime nullable warning in MemoryRelayTransport.cs is unrelated. Restore used normal NuGet TLS outside the sandbox after its TLS credential failure; no SDK, CI, package, or project dependency changes.

Compatibility

This corrects generated wire layouts for tuples with nondefault aliases and all tuples longer than seven elements, including unnamed tuples and affected nested/nullable values. Short unnamed/default-name-only tuple layouts stay compatible when their nested layouts are unchanged. Collection-item and typed upgrade encodings already used physical fields and do not change.

Regenerate and update communicating endpoints together for affected schemas. Previously stored duplicate-field payloads are not automatically detected or converted: decode using the original schema and generator, or an explicit converter for the original layout, then re-encode. A schema-version bump plus reader.Get<T>() alone cannot read those legacy duplicate bytes; a regression verifies that limitation without requiring generated deserialization to silently ignore them. Once payloads use the corrected shape, alias-only property changes can use the normal typed migration API.

Tuple aliases remain part of the existing message signature display and therefore its hashed ID. Renaming a message parameter's aliases still changes its message ID even though its parameter bytes are now identical. README documents both compatibility boundaries.

Composition and limits

This branch was tested independently of the other review fixes; combined integration and hosted CI are not claimed. When combining R26 / #31, use the shared selector in its field enumerations while retaining cancellation-token propagation, the SymbolEqualityComparer.Default ancestor path, the 128-level recursion bound, and the early return on unsupported nested fields. Do not replace R26's guarded loop or restore older model-construction signatures from this branch.

Retain the R12 / #18 unsupported-struct policy and the R10 / #14 nested collection string-wire correction. The tuple matrix uses integer elements to isolate this defect; it does not establish string-codec compatibility or extend the accepted struct policy. Preserve model metadata and construction changes from R15, R20, R22, and R23 when resolving overlapping model edits. No new diagnostics or public APIs are introduced, and this change does not implement a general legacy-wire converter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant