ov006/dScMgMCarlo2_c: reconstruct and promote the translation unit (23/23) - #2077
Merged
Conversation
Rebuilds the dScMgMCarlo2_c minigame scene as a single C++ translation unit and verifies every one of its 23 ROM functions byte-for-byte. The destructor is defined INLINE in the class body. Written out of line mwcc emits the synthesized D0 ahead of the written D1, and the cartridge has D1 first (0x020f8ef4) then D0 (0x020f8f68); linkcheck's pre-link audit refuses a TU whose licensed .text is not in ROM address order. Inline, the destructor can no longer be the key function -- InitResources, the next virtual declared and non-inline, takes that role, so this TU emits _ZTV14dScMgMCarlo2_c, whose slots 16/17 name D1 then D0 and odr-use both. The compiler then emits the pair for us, in cartridge order, and the homeless D2 (byte-identical to D1, no ROM address, no inbound relocation) disappears entirely. No #pragma is carried. Both legacy files that had `opt_propagation off` match without it once their source is shaped the way the cartridge's was: a single-exit predicate for func_ov006_020f95f0, and a materialized `found` flag for func_ov006_020fa3d0. The pragma is file-global last-wins, so a TU has one setting for all 23 members; set at the top it costs four OTHER members their match, and prepending it to those four unmodified legacy sources breaks them there too. It was a per-file crutch, not the TU's setting. Licenses the 13 compiler-generated data symbols (10 ancestor typeinfo records reused verbatim from the already-promoted dScMgBSC_c, which shares the base chain, plus this class's own _ZTI/_ZTS/_ZTV) as deadstrip-data against their cartridge addresses. byte comparison : 23/23 MATCH objisolate check : clean emission order : all 23 in ROM-ascending section order Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
The reconstructed TU is now the canonical source for the whole .text range 0x020f8ef4..0x020fa6ac. config/arm9/overlays/ov006/delinks.txt names one file where it named 23, and those 23 legacy one-function sources are gone. Verified before promoting, with tools/tubuild.py linkcheck: scratch delink + lcf, whole-tree compile, link, module byte comparison, dsd check symbols and a full ROM build. All 23 members byte-exact, every module byte-identical to the cartridge, 0 new symbol errors against a stock baseline captured at origin/main, and the linked ROM identical to the stock build. The result is recorded as scratch-link-verified because the compiler-only deadstrip policy rewrote the object -- 13 RTTI/vtable records this TU emits already have an owner elsewhere in the tree. Verified again after promoting, with tools/rombuild.py against the real tree -- the production counterpart linkcheck cannot run once the TU is enrolled, because its substitution model needs the legacy delinks entries it just replaced: 106/106 modules exact, 11,088 of 11,088 source-built functions reproducing, ROM sha256 d1506e90...c478e8, identical to the stock build. romdata_check compares all 13 of those compiler-emitted records against the cartridge and nothing differs: five ancestor typeinfo records VERIFIED whole, eight PARTIAL as exact prefixes -- including this class's own vtable at 72 of the cartridge's 144 bytes, 18 of 36 slots, because dScMgBase_c still leaves slots 18-35 undeclared. Tree-wide the verdicts are unchanged: 462 verified, 250 partial, 9 differ, 524 unnamed. Object records fall 7,599 -> 7,586. That is arithmetic, not a loss. On main the out-of-line destructor was this class's key function, so D1Ev.cpp and D0Ev.cpp each emitted the same 13 records; the inline destructor moves the key function to InitResources and the one promoted object emits each record once. converted-baseline, attribution overrides and the two match logs are retargeted, never rewritten: the same records, pointing at the surviving source. Every member carries an `// @symbol` marker so tiers_ratchet scores each body on its own text -- CONVERTED holds at 2,568 with nothing backslid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
andrewboudreau
added
the
attribution-override
✅ PR validation — PassedCommitted merge passes; 17 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (17)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (17 changed, 0 lost -- src/actors/dScMgMCarlo2_c.cpp: tangosdev -> andrewboudreau; src/actors/dScMgMCarlo2_c.cpp: mitchellcairns -> andrewboudreau; src/actors/dScMgMCarlo2_c.cpp: tangosdev -> andrewboudreau; +14 more); 23 address range(s) left the byte-verified set while enrolled totals held steady: ov006:0x020f8ef4-0x020f8f68, ov006:0x020f8f68-0x020f8ff0, ov006:0x020f8ff0-0x020f9000, ov006:0x020f9000-0x020f94f4, ov006:0x020f94f4-0x020f9560, +18 more; 5 linkcheck result(s) have unresolved relocations; 22 more function(s) now claim a match that nothing compiles; enroll them in a delinks.txt to have the ROM build check them. Per-file link-check detailAll 1095 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. |
andrewboudreau
commented
Aug 31, 2026
andrewboudreau
left a comment
andrewboudreau
left a comment
Collaborator
Author
There was a problem hiding this comment.
Gatekeeper review -- clearing this to merge. (Same account, so GitHub will not let me press Approve; this comment is the sign-off.)
Base is f35cf8f4d, which is origin/main's head, so the eight green checks measured the real merge tree rather than a stale one — no re-run needed to trust them.
What the gates prove
ROM sha256 d1506e90…, matchesStockRom, 106/106 exact, 11088/11088 reproducing, and romdata record counts unchanged from main. symbolCheckErrors is 9 against a symbolCheckBaselineErrors of 9 — identical, so checkSymbols: false is disclosing a pre-existing baseline rather than hiding a new one. nonLicensedSymbols and unlicensedSections are both empty.
What I checked that they don't
The inline-dtor flip is done properly, not just done. ~dScMgMCarlo2_c() moving into the class body is the mechanism the header explains, and the header change carries its own evidence: __destroy_arr(mArray, 0x28, 0x30, …) is what recovers mArray[0x780] out of the old pad_5120[0x808] blob — 0x28 * 0x30 = 0x780, and the residual 0x88 still lands the shared table's end at 0x51a8. That is a real layout recovery, not a pad rename.
The collateral edit in src/actors/MgPairAGoneAndOn.cpp is safe. func_ov006_020f8ff0(void) → (int *p) follows from the same __destroy_arr call — it is the element destructor, so it takes a pointer. I diffed the one call site in that file across both sides: byte-identical. Nothing else in the tree declares it.
21 of 23 @symbol markers is correct, not two missing. D1/D0 now come from the header's inline definition, so the promoted .cpp has no body to mark for them. The delinks collapse from 23 per-symbol entries to a single complete is consistent with that.
One finding — non-blocking, and it is not yours
The manifest carries 26 tubuild create warning: CONFLICT notes, each ending "kept the first, the other is commented out for review". There are zero commented-out alternates in src/actors/dScMgMCarlo2_c.cpp — grep -ic conflict → 0.
I chased this expecting lost declarations and found the opposite. Every conflicted symbol resolves to exactly one declaration or zero; struct Node is defined once; struct Obj is folded away; and the file carries a hand-resolved extern "C" { … } block with recovered types plus a comment explaining why decl_common.h cannot spell the four list pointers. You resolved all 26 by hand and deleted the alternates, which is strictly better than what the generator emits.
The defect is that tubuild.py writes those notes at create time and nothing updates them when an author resolves afterwards, so the manifest now asserts something false about the tree. Measured across main:
26 TUs carry these notes
20 alternates actually present (notes accurate)
6 alternates absent (notes stale)
ov006/dScMgBSC_c, ov002/BlueCoinSwitch, ov002/EnemySpawner+EnemySwitchTag,
ov002/SoundObject, ov002/WingFeather, ov002/dPathLiftActor_c
Three more are off by one or two (RacingPenguin 34/33, Chuckya 28/27, HauntedChair 11/10) — partial hand-resolution, same cause. Your own dScMgMCarlo_c sits at 21/21, so this is a change in practice, not a regression.
So: pre-existing tooling debt with six landed instances, and this PR is the most thoroughly resolved of the set. Not blocking merge. The ask is a follow-up, either way you prefer — have tubuild.py promote drop notes whose marker is no longer in the file, or amend the note text to say the alternate was commented out at create time. I'd take the first; the second leaves a reader grepping for something that isn't there.
Two smaller things, both fine as they are
struct SceneVtable is a shape-only shadow and says so, with a stated retirement condition ("delete it once dScMgBase_c declares slots 18..35 for real"). It emits nothing — the object audit shows no _ZTV11SceneVtable. Worth honouring that comment when dScMgBase_c lands.
boundary_confidence: medium / corroborated: false is the normal standing for this family and the vtable is doing the boundary work, so I'm not reading it as weak.
Merging. Ordering against #2078 is a non-issue — that one's diff contains zero src/ tokens, so it cannot interact with the 23 per-symbol path deletions here.
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
1 participant
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.
The second Monte Carlo minigame scene, rebuilt as one C++ translation unit and
enrolled in the build.
config/arm9/overlays/ov006/delinks.txtnow names onefile where it named 23, and every byte of
.text0x020f8ef4..0x020fa6acinthe retail overlay is built from it. Same shape as #2071.
What it took
Two members would not match without a per-file
#pragma opt_propagation offthat the two legacy sources carried. That pragma is file-global last-wins,
not positional, so a TU has exactly one setting for all 23 members and only one
place to put it — the top. Set there it costs four other members their match.
So it is not the cartridge's setting. Prepending
offto the unmodifiedlegacy sources of those four breaks them there too, which is what a per-file
crutch looks like: two bodies tuned against a flag one file at a time. Both
match with propagation on once the source is shaped the way the cartridge's
was, and the align diffs say exactly how:
func_ov006_020f95f0— one exit. The cartridge pins the result in r0 for thewhole body; early
return 0s let the compiler rematerialise the zero at eachexit, which is the extra 8 bytes.
func_ov006_020fa3d0— a nested test, not a fused&&. The cartridgematerialises the flag and then tests it (
mov r3,#0/movne r3,#1/cmp r3,#0);found = A && Bfolds the variable away and branches straightout, three instructions short.
The destructor is inline in the header. That moves the key function to
InitResources, so the TU emits_ZTV14dScMgMCarlo2_c, whose slots 16 and 17odr-use D1 and D0 — the compiler emits the pair for us, in cartridge order.
Written out of line, mwcc emits the synthesized D0 ahead of the written D1 and
the pre-link audit refuses a TU whose licensed
.textis out of ROM order.Result: no pragma at all, 23/23.
Verification
Before promoting —
tools/tubuild.py linkcheck: scratch delink + lcf,whole-tree compile, link, module byte comparison,
dsd check symbols, full ROMbuild. All 23 members byte-exact, every module byte-identical to the cartridge,
0 new symbol errors against a stock baseline captured at
origin/main, ROMidentical to the stock build. Recorded as
scratch-link-verifiedbecause thecompiler-only deadstrip policy rewrote the object: 13 RTTI/vtable records this
TU emits already have an owner elsewhere in the tree.
After promoting —
tools/rombuild.pyagainst the real tree. This is theproduction counterpart;
linkcheckcannot run once the TU is enrolled, becauseits substitution model needs the legacy delinks entries it just replaced.
The compiler-generated data —
romdata_checkbyte-compares all 13 recordsagainst the cartridge and nothing differs: five ancestor typeinfo records
VERIFIED whole, eight PARTIAL as exact prefixes, including this class's own
vtable at 72 of the cartridge's 144 bytes — 18 of 36 slots, because
dScMgBase_cstill leaves slots 18–35 undeclared. Widening that is a change tothe base class, not to this TU.
Tree-wide verdicts are unchanged: 462 verified, 250 partial, 9 differ, 524
unnamed. Object records fall 7,599 → 7,586, which is arithmetic rather than a
loss — on main the out-of-line destructor was the key function, so
D1Ev.cppand
D0Ev.cppeach emitted the same 13 records; now one object emits each once.Static gates on the merge tree (
tools/premerge_check.py, baseorigin/main@f35cf8f4d): converted-ratchet, dead-references,duplicate-sources, header-offsets, langmode-ratchet, layout-check, src-tu-refs,
source-coverage — all pass on both sides. Coverage entries 10,885 → 10,863
(-22), bytes flat: consolidation, not a loss.
Every member carries an
// @symbolmarker sotiers_ratchetscores each bodyon its own text. CONVERTED holds at 2,568, nothing backslid.
Attribution
prepush_attribution.pyreports 21 lost, every one of themgithub-actions[bot]— the known mirror artifact where a file's last touch isthe periodic progress-refresh commit. The two entries the validator actually
tracks (D1/D0,
tangosdev) are pinned and read "consolidated, credit intact".Needs the
attribution-overridelabel.🤖 Generated with Claude Code