Dedupe triplicated wire-size/pack/unpack field lists via WireRecord#17
Open
gavdoubleu wants to merge 6 commits into
Open
Dedupe triplicated wire-size/pack/unpack field lists via WireRecord#17gavdoubleu wants to merge 6 commits into
gavdoubleu wants to merge 6 commits into
Conversation
Proposal/Reply/PendingInfection/CoordinatedEncounter/Visitor each hand-listed their fields separately in a size constant, pack, and unpack. Add WireRecord seam (single field list -> size/pack/unpack derived); refactor all five to use it. status_byte, variable-length tails stay hand-composed around it. applyOnePendingInfection now takes const PendingInfection& instead of 8 args. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ngInfection Comment claimed a post-unpack lookup set home_array_index; no such lookup exists, it was always hardcoded to -1 (already its default, since it's excluded from kInfectionWire). Corrected comment, dropped the redundant assignment/copy.
WireRecord (f8ca6ae) derives each MPI record's wire size/pack/unpack from a single field list, but nothing checked that list still covers every wire-relevant field of its struct - a field added later and forgotten in the list would silently stop going over the wire, with no build or test failure. Add a static_assert next to each kXWire definition: - EncounterProposal/EncounterReply/PendingInfection are all-scalar, so sizeof(T) is a workable drift proxy. - CoordinatedEncounter and VisitorData end in a std::set/std::vector tail packed manually outside WireRecord, so sizeof(T) is dominated by the container's own layout instead; use offsetof(T, tail_member) up to where the WireRecord-covered header ends instead. Coarser than an exact field-by-field checksum (a same-or-smaller field landing in existing trailing padding could still slip through unnoticed), but needs no hand-maintained field list, so it does not reintroduce the triplicated-listing failure mode WireRecord was written to remove. Verified by temporarily adding an unlisted field to EncounterProposal and confirming the build breaks.
…time check CoordinatedEncounter is non-standard-layout (std::set<PersonId> member), so static_assert(offsetof(CoordinatedEncounter, participants) == 32) relied on conditionally-supported behaviour: compiles under GCC/Clang w/ a -Winvalid-offsetof warning today, but the computed offset isn't a portable guarantee across standard libraries (README lists Clang 10+ as supported, not just GCC/libstdc++). Swap it for a runtime check in test_domain_communicator_detail.cpp using well-defined pointer subtraction on a real object, which needs no standard-layout assumption. Also add a static_assert(is_standard_layout_v<VisitorData>) guard next to the still-legitimate offsetof tripwire in domain_communicator.cpp, so VisitorData gaining a similarly non-standard-layout member fails loudly at compile time instead of quietly regressing the same way.
Reflow arg/initialiser-list wrapping and static_assert indentation across wire-record and event-logging call sites; no semantic change.
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
2 participants
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
Test plan