The header generator gets its guards right, and Vector3 gets its destructor - #1382
Merged
Conversation
✅ PR validation — PassedCommitted merge passes; the attribution-override label accepted 2 credit change(s), 0 lost. Full merge validation
Contributor credit moved (2)
Warnings: attribution-override label: contributor attribution changed or was lost (2 changed, 0 lost -- src/_ZN7Vector3D1Ev.cpp: tangosdev -> andrewboudreau; src/_ZN8Vector3sD1Ev.cpp: tangosdev -> andrewboudreau), accepted without failing the pull request; 1 new/changed file(s) carry an asm body with no HAND-ASM PRIMITIVE or NONMATCHING banner: src/_ZN6Player13InitResourcesEv.cpp; 81 linkcheck result(s) have unresolved relocations; 29 affected source file(s) could not be fully link-checked. Per-file link-check detail26 of 4464 changed file(s) do not match the ROM (NO-SYM).
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. |
andrewboudreau
added
the
attribution-override
andrewboudreau
force-pushed
the
cpp/layout-headergen
branch
2 times, most recently
from
August 10, 2026 18:27
5eb732f to
6b92223
Compare
…ructor The second half of the layout work behind #1377, stacked on the Platform/Enemy half. 183 files. VECTOR3 HAS A DESTRUCTOR, and ChiefChilly is the proof. func_020072c0 and func_02011508 become Vector3::~Vector3 and Vector3s::~Vector3s -- four bytes each, `bx lr`, which is what a trivial destructor compiles to and what every class holding one by value calls. Six classes are migrated here rather than in the sibling PR because this half does not link without them: giving Vector3 a destructor stops ChiefChilly's hand-written .c files compiling, and renaming func_02011508 leaves Unagi, Wiggler, Goomboss and ChainChomp naming a symbol that no longer exists -- which every file still COMPILES against, and only mwldarm rejects ("Undefined : func_02011508, referenced from Wiggler::~Wiggler"). Found by building, not by reading. HEADER GENERATION, four fixes, each found by a header that would not compile: the base include belongs inside the __cplusplus guard (46 headers); the array suffix the generator was dropping (Minimap, HUD, WaterSuction); an override must match its base (MontyMoleRock, BowserPuzzlePiece); and member types belong on both sides of the guard. Also: ModelBase::SetFile and ShadowModel::InitCylinder return int, not void -- which is what was blocking KoopaShell. VERIFIED rombuild -j16, full ROM: 106/106 exact, 0 mismatching, 10,813 source-built, 87.88%. eligible.py against cbd57a8 differs only by the intended rename pair: func_020072c0 and func_02011508 out, _ZN7Vector3D1Ev and _ZN8Vector3sD1Ev in. langmode ratchet PASS. A symbol rename drops `complete` in delinks (enroll.py preserves it by symbol name), so the two renamed entries were re-promoted with --complete-list; without that the source-built count silently falls 10,813 -> 10,810 while every gate still passes. Original work is by the author of #1377; this split preserves the content, not the 37-commit history, which remains on that branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
…emitted Moved to the base of this stack so every PR in it inherits the fix. It was committed on the top PR, which left the two below validating against the broken gate and failing with wrong-dest verdicts on correct files. The relocation-destination gate compiled each changed file and checked its raw relocations. The ROM build does not link that object -- it links the objisolated one -- and isolation changes the destinations, so the gate was answering about a file the build never sees. mwcc's `_ZTV<C>` addresses the vtable OBJECT, so a vptr store carries an addend skipping the offset-to-top and typeinfo words, while symbols.txt's `_ZTV<C>` IS the slot array; objisolate subtracts that preamble. Unisolated, ModelAnim2's secondary vptr store resolves as `_ZTV10ModelAnim2` (0x0208e9b4) + 44 = 0x0208e9e0, which is `_ZTI8dFader_c` -- an unrelated class's typeinfo. Isolated, 8 -> 0 and 44 -> 36 give 0x0208e9b4 and `VTable_Animation_ModelAnim2Thunk` (0x0208e9d8), both correct. MULTIPLE INHERITANCE IS WHY THIS SURFACED NOW. The single-inheritance addend of 8 resolves to a word still inside the same vtable symbol, so it never looked wrong, and the 96 destructors already migrated never tripped it. A secondary vptr store points past the primary slots and lands on whatever follows. Isolation also drops the compiler-emitted passengers, so a migrated destructor reports one slot rather than four or five. That is not lost coverage: the thunks are separately enrolled files with their own delinks entries, checked on their own runs. Fails open -- if objisolate cannot plan an object, the file is checked unisolated rather than losing its verdict. Verified on the three files the validator flagged across this stack: _ZN9ModelAnimD1Ev, _ZN10ModelAnim2D1Ev and _ZN10ModelAnim2D0Ev all report `ok` with the fix and WRONG without it, while rombuild links all three at 106/106 exact either way. tools suite 265 passed, 3 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
…e it Comment only; no codegen change (rombuild 106/106 exact, unchanged). #1381 gave Enemy its real base. This comment was written before that and still said Enemy was "still a flattened struct here rather than `Enemy : Actor`", ending with "Delete this one when Enemy gains its real base." Enemy has gained it, and following that instruction would break the D0 route for every Enemy subclass. mwcc inlines operator delete only when it finds it in the class itself or its IMMEDIATE base; for a subclass of Enemy the immediate base is Enemy, and Actor is the grandparent, so Actor's copy is out of reach. The declaration is load-bearing precisely because Enemy now derives. Caught by a review pass over the post-#1381 headers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
andrewboudreau
force-pushed
the
cpp/layout-headergen
branch
from
August 10, 2026 19:15
da75d40 to
c9a11dd
Compare
This was referenced Aug 10, 2026
andrewboudreau
added a commit
that referenced
this pull request
Aug 11, 2026
…#1393) The file has not compiled since it gained `#include "decl_common.h"`. That header declares extern void func_ov002_020bdd2c(void*); extern void func_02073244(void*, int, int, void (*)(void*)); inside `extern "C"`, and the file then declared the same names again taking `char *` and `(int, int, int, void (*)(void))`. Same C linkage, different parameter types, so it is not a redeclaration but an attempt to overload a C-linkage name -- which mwccarm rejects outright: src/_ZN6Player16CleanupResourcesEv.cpp:17: illegal function overloading ... :18, :19, :20, :23 Errors caused tool to abort. Nothing caught it, because the failure removes the file from every gate that could have. A file that will not compile is not eligible, so it is not enrolled; its delinks entry has no `complete`, so dsd hands the range to the original ROM bytes and the build stays 106/106 without ever touching the source. It went unbuilt through #919, #994, #1032 and #1382 -- the last of which edited it. Fix is to declare only what decl_common.h does not. The three that remain are absent from it; `_ZN7Vector3D1Ev` is spelled with the `this` it actually takes, which is also what makes the `func_02073244` argument type-correct without a cast. THE FUNCTION IS A BYTE MATCH. Once it compiles, all 0x730 bytes at 0x020e32d4 in ov002 reproduce under the pin, with correct relocation destinations -- so this is not a repair of a near-miss but the recovery of a match that has been sitting unreadable in the tree. Enrolled here rather than left as rombytes. VERIFIED build_pin (True, '2004/b56') at 0x020e32d4, size 0x730, ov002 pr_linkcheck ok, 1 slot, correct destinations eligible.py 10,814 / 11,162 -- main is 10,813, and the diff is this symbol alone rombuild -j16 106/106 exact, 10,814 source-built, 0 mismatching, 87.88% -> 87.96% Found while diagnosing why #1390's validation reported a worker error: with the NameError in tools/pr_linkcheck.py fixed (#1392), the gate could finally grade this file, and graded it NO-SYM. The verdict is correct and predates #1390 -- clean main fails to compile it identically. Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Part 2 of 4 splitting #1377. 183 files — under the 200-file cap. Stacked on #1381.
Vector3 has a destructor, and ChiefChilly is the proof
func_020072c0andfunc_02011508becomeVector3::~Vector3andVector3s::~Vector3s— four bytes each,bx lr, which is what a trivial destructor compiles to and what every class holding one by value calls.Six migrations are here rather than in #1379, because this half does not link without them. Giving
Vector3a destructor stops ChiefChilly's hand-written.cfiles compiling; renamingfunc_02011508leaves Unagi, Wiggler, Goomboss and ChainChomp naming a symbol that no longer exists — which every file still compiles against, and onlymwldarmrejects:Found by building, not by reading.
Header generation, four fixes
Each found by a header that would not compile: the base include belongs inside the
__cplusplusguard (46 headers); the array suffix the generator was dropping (Minimap, HUD, WaterSuction); an override must match its base (MontyMoleRock, BowserPuzzlePiece); and member types belong on both sides of the guard.Also
ModelBase::SetFileandShadowModel::InitCylinderreturnint, notvoid— which is what was blocking KoopaShell.Verified
rombuild.py -j16, full ROMeligible.pyvscbd57a83func_020072c0/func_02011508out,_ZN7Vector3D1Ev/_ZN8Vector3sD1EvinA trap worth keeping: a symbol rename drops
completeindelinks.txt, becauseenroll.pypreserves it by symbol name. The two renamed entries had to be re-promoted with--complete-list; without that the source-built count silently falls 10,813 → 10,810 while every other gate still passes.Original work is by the author of #1377; this split preserves the content, not the 37-commit history.
🤖 Generated with Claude Code
https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x