Promote ov006/dScMgMCarlo_c to a single 23-function C++ TU - #2071
Merged
Conversation
✅ PR validation — PassedCommitted merge passes; 23 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (23)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (23 changed, 0 lost -- src/actors/dScMgMCarlo_c.cpp: andrewboudreau -> github-actions[bot]; src/actors/dScMgMCarlo_c.cpp: andrewboudreau -> github-actions[bot]; src/actors/dScMgMCarlo_c.cpp: tangosdev -> github-actions[bot]; +20 more); 23 address range(s) left the byte-verified set while enrolled totals held steady: ov006:0x020f7634-0x020f76a8, ov006:0x020f76a8-0x020f7730, ov006:0x020f7730-0x020f7740, ov006:0x020f7740-0x020f7994, ov006:0x020f7994-0x020f7a00, +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 1098 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
force-pushed
the
cpp/minigame-tu-2
branch
from
August 31, 2026 08:46
7c0f9a9 to
3194e6d
Compare
andrewboudreau
force-pushed
the
cpp/minigame-tu-3
branch
from
August 31, 2026 08:46
de7e85a to
fee055d
Compare
Collaborator
Author
Verified from the cartridge — content approved. One more re-cut, and this one is trivial.Reviewed at 32 files, +1667/−1106 both ways. So everything below, verified once, still stands — nothing needs re-checking after the next move either, as long as the commits are replayed rather than hand-merged. Where this stands right now#2067 is merged — squashed to That squash makes this PR
Cut from Three-way merge I already ran I will re-run it against the real merge tree after the re-cut, but I do not expect a surprise. What I verified independentlyMarkers: 21, and the two absences are correct23 declared functions, 21 markers. The two without one are Second TU in a row that arrives already conforming to The linked range is exactly contiguous and abuts both neighboursChaining the 23 removed delink ranges: 23 for 23, no gap, no overlap. And in the rewritten Both endpoints preserved, both neighbours abut exactly. All 13 licensing addresses read back correctly from the ROMThis is the part Thirteen for thirteen. The RTTI chain, walked from the vtable
Agreed on your point about The converted ratchet is one clean re-keyOne line, set size unchanged. The other four class files and all eighteen And on your correction — accepted, and it is the more useful reading. This one went pass → FAIL on the merge tree; #2067's eleven were latent. Worth keeping apart precisely because it tells us what the gate is: it catches a swallowed span only once a banked entry falls inside it. That is a real backstop, not a full one, and it is the argument for markers being mandatory rather than merely tidy. The
|
src_tu/actors/dScMgMCarlo_c.cpp -> src/actors/dScMgMCarlo_c.cpp, replacing
23 per-function legacy sources (18 flat-C, 5 already C++). The whole .text
range 0x020f7634..0x020f8e44 now reproduces from one object.
CONSOLIDATION IS WHERE THE WORK WAS. Each of the 23 legacy files compiled
in isolation with its own private shadow structs and extern declarations,
so they were free to contradict each other and never did so out loud. Put
in one TU they produced ~25 compile errors, every one a real disagreement
about the same object. Each was settled against evidence:
- `Node` and the list-`Obj` are the SAME struct. Five partial views, all
agreeing on every offset they share; unified into one layout with the
fields commented.
- A THIRD, unrelated `Obj` was not a list node at all -- it was a
shape-only shadow of this scene class's OWN vtable, used for the
slot-18 self-dispatch in InitResources. Revived as `SceneVtable` so
the two meanings stop colliding. It becomes a real base-class virtual
call once dScMgBase_c declares slots 18-35.
- data_ov006_02142500 is the list HEAD and _02142504 the TAIL, proven by
func_ov006_020f7740 walking head->next and tail->prev. Render starts
from the tail, so both its loops step `prev`, not `next` -- corrobo-
rated independently by func_ov006_020f8a3c's raw `*(char**)(r3 + 4)`.
- _Z14ApproachLinearRiii and _Z15ApproachLinear2Rsss take REFERENCES;
the mangled names say so, and a reference and a pointer pass the same
address, so the accurate spelling costs no code.
- func_ov006_020f8540 was declared (void) but called with an argument.
ARM passes it in r0 and the body ignores it; spelling the parameter
keeps the call site honest and changes no byte.
- include/dScMgBase_c.h already declared data_ov004_020beb68 as void*,
which settled that conflict against the TU's char*.
The destructor is defined inline in the class body, for the same reason as
dScMgBSC_c: out of line, mwcc emits the synthesized D0 ahead of the written
D1, and the cartridge has D1 first (0x020f7634) then D0 (0x020f76a8).
Inlining hands the key-function role to InitResources -- the next virtual
declared, non-inline, and defined here -- which is what emits
_ZTV13dScMgMCarlo_c. Slots 16 and 17 name D1 then D0, odr-using both, so
the compiler emits the pair in cartridge order, and the homeless D2 is gone.
decl_common.h loses the scalar declarations of the four list globals
(_021424fc/_02142500/_02142504/_02142508). They were arbitrary winners
among 19 contradictory isolated spellings; every consumer is in this TU,
which now declares all four with the recovered pointer type.
Verification:
tubuild verify ov006/dScMgMCarlo_c 23/23 MATCH, objisolate clean,
reloc-destinations clean,
ROM-ascending section order
tubuild linkcheck (pre-promotion) SCRATCH-LINK-VERIFIED; 106/106
exact; full ROM built and IDENTICAL
to the stock build. dsd check
symbols --fail still fails on the
same 9 errors the baseline control
has -- 0 new, so not attributable
to this TU, but NOT green and not
reported as such.
rombuild -j16 (post-promotion) 11,088 source-built functions,
reproducing 11,088, mismatching 0;
106/106 exact, 100.000000%; the ROM
it builds is sha256 d1506e90...,
byte-identical to the stock build.
layout_check / eligible / langmode / port_refcheck / dead-references: clean
All 13 compiler-emitted RTTI and vtable symbols are licensed in
compiler_only_output, every canonical address confirmed against
config/arm9 symbols.txt, and romdata_check byte-compares each against the
cartridge: 6 VERIFIED, 7 PARTIAL (exact prefixes), 0 DIFFER.
_ZTV13dScMgMCarlo_c reproduces 72 of the cartridge's 144 bytes -- 18 of 36
slots -- because dScMgBase_c still leaves slots 18-35 undeclared. That is a
verified prefix, not a disagreement, and widening it is the dScMgBase_c
follow-up, not this change.
The cartridge's own _ZTS strings corroborate the names: reading
extracted/dsd/arm9_overlays/ov006.bin at overlay base 34340544 gives
13dScMgMCarlo_c at 0x0213d594 and 19dScMgSingle3DBase_c at 0x0213bd00 --
so this TU emits no homeless typeinfo record.
Attribution: a 23-delete + 1-add collapse ends 22 git lineages, so
attribution.json takes 23 explicit override keys on the surviving path,
each carrying the first matcher the gate itself computes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
tiers.score_member slices a member at `// @symbol` markers and, finding
none after a marker, runs to end of file. Only 5 of the 21 in-file members
carried one, so _ZN13dScMgMCarlo_c6RenderEv was scored over everything
below it -- the flat-C bodies with their raw casts and mangled callees --
and the CONVERTED ratchet read that as a backslide of a banked entry:
src/actors/dScMgMCarlo_c.cpp#_ZN13dScMgMCarlo_c6RenderEv
No raw offset arithmetic; Calls things by real names, not mangled _Z
Neither is true of Render's own body. The 16 missing markers are added, so
each member is scored against itself. Comments only:
tiers_ratchet --check PASS baseline 2568 current 2568
rombuild -j16 106/106 exact, 100.000000%; 11,088/11,088
reproducing, 0 mismatching; the ROM is still
sha256 d1506e90..., identical to the stock build
dScMgBSC_c.cpp had the same gap -- 6 markers for 17 members -- and none of
its banked members mis-scored, so it was a latent hazard rather
than a regression; it is fixed one commit down, in #2067, where it lives.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
src/func_ov006_020f8e44.cpp declared the array's element destructor as `void(void)` while src/actors/dScMgMCarlo_c.cpp recovers it as `void(int *)`. Both compile -- the factory casts to void* at the call -- but that file is the corroboration for mArray at 0x51a8 being 0x50 x 0x30 with that element dtor, so it is the one place the disagreement is expensive to leave. rombuild: 11,088/11,088 reproducing, 0 mismatching, 106/106 exact, 100.000000%; ROM sha256 unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
andrewboudreau
force-pushed
the
cpp/minigame-tu-3
branch
from
August 31, 2026 09:19
fee055d to
ea07601
Compare
Collaborator
Author
|
Re-cut onto
The patch is unchanged, which is checkable rather than assertable: Byte-identical, 125583 bytes both sides. Everything verified at the old head carries over untouched. The one content change:
|
rombuild -j16 |
11,088/11,088 reproducing, 0 mismatching, 106/106 exact, 100.000000% |
| ROM sha256 | d1506e90efae5e2d2cf119926a4ac2a291bd5ca78349d09d5024e1a918c478e8 (unchanged) |
tiers_ratchet --check |
CONVERTED PASS, baseline 2568, current 2568 |
premerge_check |
8/8 pass→pass on merge tree 4e0d92d28a7d; entries 10907 -> 10885 (-22) |
prepush_attribution |
11189 tracked, 23 consolidated with credit intact, 0 changed, 0 lost |
port_refcheck |
405 references, 0 stale |
(-22 rather than -40 because BSC's half of the consolidation is now in the base.)
Related: the splitter defect is filed
#2072, tools-only off origin/main. Both ends of the bug are in the test file as you asked, from the real inputs: the declaration keeps its Allman body, and the TUBUILD CONFLICT comment carries a real alternate body instead of the two words struct Obj. Fixing the walk turned up two more shapes — an elaborated return type (struct dActor_c *dCapEnemy_c::RespawnIfHasCap()) being filed as a shadow declaration, and definitions wrapped in namespace/extern "C" refusing with a message that named nothing. Measured over all 11150 legacy sources: 82 headless bodies repaired, 10 more files split cleanly, 683 refusals that now name the offending block and line, 0 regressions, and nothing enrolled in a TU is affected.
Collaborator
Author
|
Merged as Final state: I did not re-run Neither file is one of the eight gates Thanks for taking the That is #2067, #2072 and #2071 landed in sequence today, each one re-cut onto a clean base rather than hand-merged. The discipline of replaying commits instead of resolving them is why the |
andrewboudreau
added a commit
that referenced
this pull request
Aug 31, 2026
…oted path (#2073) `dead references` is red on main. tools/test_tubuild.py, landed in #2072, cites `src/func_ov006_020f8224.c` as the real-world input its Allman-brace case was built from. #2071 promoted that file into the ov006/dScMgMCarlo_c TU an hour later, so the path is gone and the prose reference dangles. Neither PR could see it. #2072's dead-references run happened while the file still existed; #2071's premerge_check ran against a merge tree that predated #2072. The two are individually green and red in combination -- the merge-tree hazard, one step removed. Fix names the input by symbol instead of by path. `func_ov006_020f8224` survives both promotion and rename, and check_dead_references reads only `a/b`-shaped path tokens, so a symbol cannot dangle. The docstring says why, so the path does not get helpfully restored later. Not bundled, deliberately: `--update` would also drop the now-stale `src/game/actors` baseline entry. That is real but unrelated cleanup in a tracked config file, and main is red now.
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.
Stacked on #2067 (which is stacked on #2064). Review the top commit pair only; the earlier commits are #2067's.
src_tu/actors/dScMgMCarlo_c.cpp→src/actors/dScMgMCarlo_c.cpp, replacing 23 per-function legacy sources (18 flat-C, 5 already C++). The whole.textrange0x020f7634..0x020f8e44now reproduces from one object.Consolidation is where the work was
Each legacy file compiled in isolation with its own private shadow structs and
externdeclarations, so they were free to contradict each other and never had to say so. Put in one TU they produced ~25 compile errors — every one a real disagreement about the same object. Each was settled against evidence, not against the compiler:Nodeand the list-Objare the same struct. Five partial views, agreeing on every offset they share; unified into one layout with the fields commented.Objwas not a list node at all — it was a shape-only shadow of this scene class's own vtable, used for the slot-18 self-dispatch inInitResources. Revived asSceneVtableso the two meanings stop colliding. It becomes a real base-class virtual call oncedScMgBase_cdeclares slots 18–35.data_ov006_02142500is the list HEAD and_02142504the TAIL, proven byfunc_ov006_020f7740walkinghead->nextandtail->prev.Renderstarts from the tail, so both its loops stepprev, notnext— corroborated independently byfunc_ov006_020f8a3c's raw*(char**)(r3 + 4)._Z14ApproachLinearRiii/_Z15ApproachLinear2Rssstake references. The mangled names say so, and a reference and a pointer pass the same address, so the accurate spelling costs no code.func_ov006_020f8540was declared(void)but called with an argument. ARM passes it in r0 and the body ignores it; spelling the parameter keeps the call site honest and changes no byte.include/dScMgBase_c.halready declareddata_ov004_020beb68asvoid*, which settled that conflict against the TU'schar*.decl_common.hloses the scalar declarations of the four list globals (_021424fc/_02142500/_02142504/_02142508). They were arbitrary winners among 19 contradictory isolated spellings; every consumer is in this TU, which now declares all four with the recovered pointer type.The destructor is inline, and that is what makes the vtable ours
Out of line, mwcc emits the synthesized D0 ahead of the written D1; the cartridge has D1 first (
0x020f7634) then D0 (0x020f76a8). Inlining hands the key-function role toInitResources— the next virtual declared, non-inline, defined here — which is what emits_ZTV13dScMgMCarlo_c. Slots 16 and 17 name D1 then D0, odr-using both, so the compiler emits the pair in cartridge order, and the homeless D2 is gone.Verification
tubuild verify ov006/dScMgMCarlo_ctubuild linkcheck(pre-promotion)rombuild -j16(post-promotion)d1506e90efae5e2d2cf119926a4ac2a291bd5ca78349d09d5024e1a918c478e8— byte-identical to the stock buildpremerge_checkon the merge treedsd check symbols --failstill fails on the same 9 pre-existing errors the baseline control also has — 0 new, so not attributable to this TU, but it is not green and is not reported as such.Compiler-generated output, byte-compared
All 13 RTTI/vtable symbols are licensed in
compiler_only_output, every canonical address confirmed againstconfig/arm9/.../symbols.txt, andromdata_checkcompares each against the cartridge: 6 VERIFIED, 7 PARTIAL (exact prefixes), 0 DIFFER._ZTV13dScMgMCarlo_creproduces 72 of the cartridge's 144 bytes — 18 of 36 slots — becausedScMgBase_cstill leaves slots 18–35 undeclared. That is a verified prefix, not a disagreement; widening it is thedScMgBase_cfollow-up, not this change.The cartridge's own
_ZTSstrings corroborate the names: readingextracted/dsd/arm9_overlays/ov006.binat overlay base 34340544 gives13dScMgMCarlo_cat0x0213d594and19dScMgSingle3DBase_cat0x0213bd00— so this TU emits no homeless typeinfo record.Second commit:
@symbolboundariestiers.score_memberslices a member at// @symbolmarkers and, finding none after a marker, runs to end of file. Only 5 of 21 in-file members carried one, soRenderwas scored over every flat-C body below it and the CONVERTED ratchet read that as a backslide — neither raw-cast nor mangled-callee is true ofRender's own body. 16 markers added; comments only, ROM sha256 unchanged.src/actors/dScMgBSC_c.cpphas the same gap (6 markers for 17 members). Nothing banked mis-scores there today, so it is latent rather than red — it belongs to #2067, not here.Attribution
A 23-delete + 1-add collapse ends 22 git lineages, so
attribution.jsontakes 23 explicit override keys on the surviving path, each carrying the first matcher the gate itself computes.prepush_attribution: 47 consolidated with credit intact, 0 changed, 0 lost.🤖 Generated with Claude Code