Point seven PathPtr shadow structs at the real header - #2729
Conversation
Ten .cpp files declared their own struct PathPtr instead of including
include/PathPtr.h, and all ten disagreed: `int a, b`, `char pad_00[8]`,
`char b[8]`, `char pad[8]`, one with no fields at all, and one declaring
`int GetNode(...) const` where the real header says `void`. That is the
shadow-declaration problem notes/plan-cpp-language-mode.md phase 4 exists
to retire. Seven of the ten now include the header and delete the copy.
The other three are NOT a spelling problem and are left alone. They declare
`PathPtr path;` as a POD local and then call the constructor explicitly by
mangled name further down the body:
PathPtr path; /* shadow has no constructor */
...
_ZN7PathPtrC1Ev(&path); /* ROM constructs it HERE */
The real header declares PathPtr(), so the same declaration would emit a
second constructor call at the declaration point. Both compile; the bytes
differ. Fixing them means moving the local to where the ROM constructs it,
which also moves its allocation rank, so it wants its own measured pass.
Gates: full COLD rebuild (--no-cache, all 8760 enrolled files), mismatching 0,
106/106 exact, ROM-build analysis PASS. CONVERTED 2812 -> 2816. Eligible set
identical at 11243. No new declaration disagreements. langmode_audit local
struct bodies 1676 -> 1671. port_refcheck 423/423, MSVC port smoke 230/230.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Per-file link-check detailAll 7 changed file(s) compile to the ROM byte-for-byte with correct relocation targets.
The private worker commits a test merge, builds the stock ROM profile, compares every executable module, measures matched and source-built code, checks contributor lineage, and verifies affected relocations. The mod profile is opt-in and is not part of this merge gate. |
Five-way merge tree gated (main + #2725 + #2726 + #2727 + #2728 + #2729)Each of these five PRs is green on its own, but this repo has produced a red Disjointness. 122 changed files across the five; all 10 pairs share zero files, Cold rebuild ( The rebuild is deliberately cold. A warm run on this same tree reported
🤖 Generated with Claude Code |
Adversarial review — seven-PR set (#2725–#2731)Re-reviewed as a critic rather than as the author. Every check below is new work, not a 1. Relocation destinations — the hazard the byte gate cannot seeA relocated word is a wildcard: a
0 WRONG-DEST across 125 files and 61 TU functions. This also closes the cast-receiver 2. Merge tree, cold
3. Change-kind census139 files changed in the merge tree, all 4. Local-only gates, run on the merge tree (CI runs none of these on a merge tree)
5. The merged decl baseline is not a silent clobber
0 hidden. Every difference is in the safe direction. (Separately: ~84 stale banked entries 6. The MSVC port build — differential, not absolute
The other six PRs introduce zero new port link errors. What remains is the pre-existing 7. Per-PR findings
VerdictNo defect found, and no change requested. Both incomplete migrations are incomplete on The one thing this review cannot supply is the thing the review gate actually wants: I am the 🤖 Generated with Claude Code |
Conflict: src/func_ov019_0211127c.cpp, modify/delete. This branch deletes it (the class TU absorbs the body); main's c1b266f (tangosdev#2729) retired its TU-local `struct PathPtr` shadow in favour of `#include "PathPtr.h"`. Taken as a delete only after confirming the absorbed body already carries main's change and goes further: include/daPgRcer_c.h includes PathPtr.h and declares `PathPtr mPath` at 0x364, and the absorbed func_ov019_0211127c calls `((daPgRcer_c *)c)->mPath.GetNode(node, j)` -- the real header type through a named member, not a cast onto a local shadow. Nothing to port. Two gates were red and both were the promotion's own bookkeeping, not the code. config/decl-agreement-baseline.json: this branch had renamed four bank keys from `src/_ZN13RacingPenguin*.cpp` to `src/_ZN10daPgRcer_c*.cpp`, paths its own promotion never creates, stranding them; the other thirty-odd entries were left keyed to the per-function shards the promotion deletes. Rebuilt as main's baseline minus what this branch heals plus what it newly banks, measured by regenerating on a pristine origin/main tree and on this one and diffing: 40 added, every one `src/game/actors/d_a_pg_rcer.cpp`; 81 removed, every one in a file this branch deletes or edits (68 in the deleted ov019 shards, 12 in src/__sinit_ov019_021127a4.c where consolidating N declarations into one flipped the plurality onto the sinit's `void *` / `V3` spellings, 1 in include/decl_common.h). Main's own 98 stale entries are left untouched -- the full scan still reports them, as it does on main. config/tu_manifest.d/ov019/daPgRcer_c.json: 33 conflict notes still read `tubuild create warning: CONFLICT:` while the promoted source carries zero TUBUILD CONFLICT markers, so the check paired 33 notes against nothing. The entry's own reconcile note already claimed they had been marked (RESOLVED) and only one of the 34 actually was; rewritten to the wording tools/tubuild.py emits, per check_tubuild_conflicts.py's own instruction. That note also named src_tu/actors/daPgRcer_c.cpp, a file that does not exist; repointed at the promoted source. Verified: rombuild -j16 --no-rom PASS, 804 compiled, 11,206 reproducing / 0 mismatching, module fidelity 106/106 exact at 100.000000%, ROM-build analysis PASS -- so the shadow-struct retirement added no constructor call. check_decl_agreement PASS, check_tubuild_conflicts --list PASS, check_dead_references PASS, check_src_tu PASS, both gates' unit suites OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
1 participant
First slice of phase 4 from
notes/plan-cpp-language-mode.md— retiring shadow declarations.Ten
.cppfiles declared their ownstruct PathPtrrather than includinginclude/PathPtr.h, and all ten disagreed with each other:func_ov062_0211b3ac.cppint a, b;_ZN14UnchainedChomp13InitResourcesEv.cppchar pad_00[8];+ 4 methods_ZN7SkiLift13InitResourcesEv.cppchar b[8];_ZN8MantaRay8BehaviorEv.cppchar pad[8];+ 3 methodsfunc_ov019_0211127c.cppfunc_ov092_021313b0.cppint GetNode(...) const— the header saysvoidinclude/PathPtr.his a real reconstructed header: named fields with offsets, documented meaning, andsizeof == 0x8asserted. Seven of the ten now include it and delete their copy.The other three are not a spelling problem
They declare
PathPtr path;as a POD local — their shadow has no constructor — and then call the constructor explicitly by mangled name further down the body:The real header declares
PathPtr(), so that same declaration emits a constructor call at the declaration point, giving two calls in the wrong order. Both versions compile; only the bytes tell you. Fixing them means moving the local down to where the ROM constructs it, which also moves its allocation rank, so it wants its own measured pass rather than being forced into this one.Gates
rombuild --no-rom --no-cachemismatching: 0,106/106 exact, PASStiers_ratchet --checkeligible.pycheck_decl_agreementlangmode_auditport_refcheckThe cold rebuild is deliberate: an incremental run reported
0 compiledhere, because the shared object cache is keyed on content and these objects were already built.0 compiledis this repo's classic false-green, so the count above is from a run with no cache at all.Branches from
main; touches no file in #2725, #2726, #2727 or #2728.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c