Platform is 0x320, Enemy derives from Actor, and the structors that follow - #1381
Merged
Conversation
…ollow The first half of the layout work behind #1377, split so each PR clears the validator's 200-file cap. 172 files; the second half is the header-generation fixes and the Vector3 destructor. PLATFORM'S DATA ENDS AT 0x31e, SIZEOF 0x320, and it takes more than one derived class to see why. BowserFireSeaArena starts its own Model at 0x324, which alone reads like the class ending there. But four classes derive from Platform DIRECTLY -- one non-Platform vtable store each, so no intermediate -- and each places a 4-byte-aligned CLASS member at 0x320, impossible if Platform occupied 0x320..0x323: PyramidTop daObjDlPyramid_c Model @ 0x320 SwitchPillar daObjC0Water_c TextureTransformer @ 0x320 MovingBarSmall daObjBk_Lift_c ShadowModel @ 0x320 WallSign daObjKanban_c MovingCylinderClsnWithPos @ 0x320 Each is read straight off that class's destructor, which destroys its own member at 0x320 before storing _ZTV8Platform and running the base. One layout satisfies all five: data ends 0x31e, sizeof 0x320. Ending at 0x324 satisfies BowserFireSeaArena and contradicts the other four. This corrects #1374, which was mine, and which took BowserFireSeaArena's evidence alone. Also here: Enemy derives from Actor -- what 51 subclasses were waiting on -- with five more Enemy fields named by its subclasses and its vtable symbol; the inline base destructors for ActorDerived and Scene; Platform's own destructor forced out of line by a call it never makes, because an inline destructor emits nothing in a TU that only includes the header; the arm9 D0s that were missing an operator delete rather than fakematching; and objisolate learning the multiple-inheritance secondary vptr addend -- `>= VTABLE_PREAMBLE` rather than `==`, so 44 -> 36 for ModelAnim corrects instead of being refused, which the ROM confirms independently (_ZTV9ModelAnim 0x0208e980 against VTable_Animation_ModelAnimThunk 0x0208e9a4, differing by exactly 44 - 8). VERIFIED rombuild -j16, full ROM: 106/106 exact, 0 mismatching, 10,813 source-built, 87.88%. eligible.py against cbd57a8: name list IDENTICAL -- nothing lost, nothing gained. langmode ratchet PASS. pr_linkcheck: no new WRONG or NO-REPRO. 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
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Warnings: 22 linkcheck result(s) have unresolved relocations; 2 affected source file(s) could not be fully link-checked. Per-file link-check detail2 of 1039 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. |
This was referenced Aug 10, 2026
andrewboudreau
added a commit
that referenced
this pull request
Aug 10, 2026
…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
added a commit
that referenced
this pull request
Aug 10, 2026
…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
added a commit
that referenced
this pull request
Aug 10, 2026
…ructor (#1382) * The header generator gets its guards right, and Vector3 gets its destructor 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 * pr_linkcheck: check the object the linker consumes, not the one mwcc 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 * Enemy.h: the operator delete must stay, and the comment said to delete 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 * symbols: drop the size-0 duplicates the Vector3 dtor rename left behind --------- 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 1 of 4 splitting #1377. 172 files — under the validator's 200-file cap, so this auto-validates instead of asking for a hand review.
main← this ← #1381 ← #1379 ← #1380. The four reconstruct #1377's tree exactly (same git tree hash).Platform's data ends at
0x31e, sizeof0x320It takes more than one derived class to see why.
BowserFireSeaArenastarts its ownModelat0x324, which alone reads like the class ending there. But four classes derive from Platform directly — one non-Platform vtable store each, so no intermediate — and each places a 4-byte-aligned class member at0x320, impossible if Platform occupied0x320..0x323:0x320daObjDlPyramid_cModeldaObjC0Water_cTextureTransformerdaObjBk_Lift_cShadowModeldaObjKanban_cMovingCylinderClsnWithPosEach is read straight off that class's destructor, which destroys its own member at
0x320before storing_ZTV8Platformand running the base. One layout satisfies all five; ending at0x324satisfies BowserFireSeaArena and contradicts the other four.This corrects #1374, which was mine, and which took BowserFireSeaArena's evidence alone.
Also here
Enemyderives fromActor— what 51 subclasses were waiting on — plus five more Enemy fields named by its subclasses, and its vtable symbol.ActorDerivedandScene.operator delete, not fakematching.>= VTABLE_PREAMBLErather than==, so 44 → 36 forModelAnimcorrects instead of being refused. The ROM confirms it independently —_ZTV9ModelAnim0x0208e980againstVTable_Animation_ModelAnimThunk0x0208e9a4, differing by exactly 44 − 8.Verified
rombuild.py -j16, full ROMeligible.pyvscbd57a83pr_linkcheckOriginal work is by the author of #1377; this split preserves the content, not the 37-commit history, which remains on that branch.
🤖 Generated with Claude Code
https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x