TU: promote ov006/dScMgSingle3DBase_c — nine functions, vtable-proven boundary - #2064
Conversation
…tion boundary Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
Consolidates nine ov006 functions at 0x0210a4b0..0x0210a8c0 into one
translation unit, src/actors/dScMgSingle3DBase_c.cpp, replacing six
mangled-name single-function sources and three func_<mod>_<addr> ones.
The boundary is proved by the vtable, not by tu_map. tu_map cut the unit
at eight functions because func_ov006_0210a708 still carries a
func_<module>_<address> name and so gets no class label. But exactly 14
vtables in ov006 reference 0x0210a708 at slot 33, and the same 14
reference 0x0210a600 at slot 26 -- a function tu_map did include.
Identical reference sets, so both are members of this class. The ninth
function byte-matched on the first compile at the extended boundary.
Two naming corrections carried in the source and header:
- Ordinal 3 was labelled dScMgFlower_c_OnHitByCannonBlastedChar. It is
dScMgSingle3DBase_c's slot 26, an override of
dScMgBase_c::OnHitByCannonBlastedChar (ov004:0x020b04e0). The old
label was one class too deep -- the same off-by-one already
documented for AfterInitResources.
- The header annotated slots 26 and 33 as new virtuals at this class.
Both are re-overrides: dScMgBase_c's own vtable already carries
bodies at each (ov004:0x020b04e0, ov004:0x020b265c).
Signature reconciliation: the two legacy sources disagreed on the type of
the loaded-file buffer (int in one, void* in the other) while passing it
to the same functions. Unified on the pointer form, removing four casts.
Byte-neutral -- the other eight functions still MATCH, which proves it.
Eleven compiler-emitted symbols (vtable, typeinfo, typeinfo names for the
whole base chain) are licensed as deadstrip-data with canonical homes
read from the symbol table.
Verification, on the promoted tree:
linkcheck --baseline --module ov006: ov006 built 525344 == retail
525344, ROM sha256 d1506e90... identical to stock, dsd check symbols
0 NEW / 0 resolved against the 9 pre-existing errors.
rombuild -j16: mismatching 0, module fidelity 106/106 exact,
100.000000% of compared bytes, ROM-build analysis PASS.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
✅ PR validation — PassedCommitted merge passes; 7 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (7)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (7 changed, 0 lost -- src/actors/dScMgSingle3DBase_c.cpp: tangosdev -> github-actions[bot]; src/actors/dScMgSingle3DBase_c.cpp: tangosdev -> github-actions[bot]; src/actors/dScMgSingle3DBase_c.cpp: andrewboudreau -> github-actions[bot]; +4 more); 9 address range(s) left the byte-verified set while enrolled totals held steady: ov006:0x0210a4b0-0x0210a4e8, ov006:0x0210a4e8-0x0210a534, ov006:0x0210a534-0x0210a600, ov006:0x0210a600-0x0210a608, ov006:0x0210a608-0x0210a664, +4 more; 8 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 91 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. |
Two corrections to the section, both measured rather than reasoned. 1. The section told authors to mark ctor/dtor variants "anyway". No landed promoted TU does: daObjKinokoTag_c, daObjFm_Battan_c, daObjKm3_Kurumajiku_c and daEyBm_c mark zero structors between them. In an inline-destructor TU there is nothing to mark -- D1 and D0 have no definition text in the .cpp at all -- and a bare marker placed elsewhere is actively harmful, since the slice runs from it to the next marker and would charge the following member's text to the destructor. Inlining is also usually deliberate: out of line, mwcc emits the synthesized D0 ahead of the written D1, reversing cartridge order, and linkcheck refuses a TU whose licensed .text is not in ROM address order. The rule is now: do not mark them, keep the inline definition in a directly included header, and check the fragment survives. 2. The #2064 bullet said its unmarked func_ov006_0210a534 "cannot be rescued by a boundary anyway". Measured, the omission does cost -- just not there. With no marker after it, the preceding func_ov006_0210a600 fragment runs to end of file and swallows a534's volatile body, so an eight-byte 'return 1;' can never score readable. Rewritten to make the general point: the member that pays is rarely the member you left unmarked. Reviewer checklist item 7 updated to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
Changes requested — five one-line comment fixes. The boundary itself I reproduced from the ROM, and it is right.This is the best-evidenced TU promotion that has come through this queue. I had it reviewed by someone who did not write it, with instructions not to take the PR body on trust, and the central claim was re-derived from the cartridge image rather than read off your JSON. Everything I am asking for is a comment. Not one byte moves. The boundary is real, and here is the independent derivation
Every slot number your header and body assert is confirmed. Then the reference scan: The "fourteen vtables, identical reference sets" claim is exactly true — the class plus its thirteen children, matching the thirteen headers that Both neighbours were checked, which is the question that actually decides a boundary:
Key function confirmed too. The inline All eleven licensed symbols were checked against the tree's own symbol tables by address — all eleven correct, all outside the licensed What I need changed1. Not on this branch, not on main, not ever. And 2. You already fixed the identical sentence in 3. The generated banner contradicts the file's own body. The header says 4. Add Your D1/D0 being unmarked is correct and I am not asking you to change it — 5. Two disclosures I would like, not blockers
The retyping added a Optional and genuinely minor: lines 150/154 carry Gates, and what nobody verifiedRe-gated against current main, since main moved out from under your base: The only new commit in the drift is #2063, test-only, no overlap; Being explicit about the limits of the above, because your PR body is precise and this review should be too: no build was run. Also worth recording: that Nothing above touches the decomp work. Fix the five comments and I will merge it. |
* docs: the `// @symbol` marker convention for promoted TUs
tools/tiers.py scores a promoted TU per member, and `// @symbol` is the only
thing that tells it where one member ends. Without a marker score_member falls
back to scoring that member against the ENTIRE FILE, so one volatile object,
one unk_ field or one mangled call anywhere in the TU strips the tier from
every clean member in it. The convention is load-bearing and was documented
nowhere.
Adds section 6 to notes/tu-promotion-conventions.md and a reviewer-checklist
item. Measured against this tree:
src/actors/daObjKinokoTag_c.cpp 9 members 7 markers 6 banked
src/actors/daObjFm_Battan_c.cpp 9 members 7 markers 5 banked
src/actors/daObjKm3_Kurumajiku_c.cpp 5 members 3 markers 4 banked
src/actors/daEyBm_c.cpp 13 members 0 markers 2 banked
The two banked daEyBm_c members are D1/D0, rescued by the second fallback --
the inline `virtual ~daEyBm_c() {}` at include/daEyBm_c.h:54.
Also corrects a stale line: #2004 closed and Kurumajiku landed via #2057, so
that class is precedent now, not a draft data point.
Documentation-only. No tool, gate, config, ledger or src/ file is touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
* docs: structors are unmarkable by design, not under-marked
Two corrections to the section, both measured rather than reasoned.
1. The section told authors to mark ctor/dtor variants "anyway". No landed
promoted TU does: daObjKinokoTag_c, daObjFm_Battan_c, daObjKm3_Kurumajiku_c
and daEyBm_c mark zero structors between them. In an inline-destructor TU
there is nothing to mark -- D1 and D0 have no definition text in the .cpp at
all -- and a bare marker placed elsewhere is actively harmful, since the
slice runs from it to the next marker and would charge the following
member's text to the destructor. Inlining is also usually deliberate: out of
line, mwcc emits the synthesized D0 ahead of the written D1, reversing
cartridge order, and linkcheck refuses a TU whose licensed .text is not in
ROM address order. The rule is now: do not mark them, keep the inline
definition in a directly included header, and check the fragment survives.
2. The #2064 bullet said its unmarked func_ov006_0210a534 "cannot be rescued
by a boundary anyway". Measured, the omission does cost -- just not there.
With no marker after it, the preceding func_ov006_0210a600 fragment runs to
end of file and swallows a534's volatile body, so an eight-byte 'return 1;'
can never score readable. Rewritten to make the general point: the member
that pays is rarely the member you left unmarked.
Reviewer checklist item 7 updated to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Review follow-ups on #2064. Comment and manifest text only -- no code byte moves; rombuild still reports 106/106 exact, 100.000000%. 1. include/dScMgSingle3DBase_c.h: the new slot-26/33 note pointed at src_tu/actors/dScMgSingle3DBase_c.cpp. The promoted file is src/actors/dScMgSingle3DBase_c.cpp. 2. include/dScMgSingle3DBase_c.h: the destructor was still labelled "(key function)", contradicting this TU's own manifest rows. An inline destructor is emitted in every TU that needs it and anchors nothing; the key function is AfterInitResources, the first DECLARED non-inline virtual. Rewritten to say so, keeping the MUST STAY INLINE measurement intact. 3. src/_ZN10dBgActor_cD0Ev.cpp: the "same sentence in ..." pointer named src/_ZN19dScMgSingle3DBase_cD0Ev.cpp, a file this PR deletes. Repointed at the promoted path, matching the already-correct fix in notes/actor-vtables.md. 4. src/actors/dScMgSingle3DBase_c.cpp: the banner said 8 function(s) and listed eight legacy sources while the body carries nine; src/func_ov006_0210a708.c was missing from the assembled-from list. (boundary_evidence[0]'s "8" stays -- it is a faithful quote of tu_map, immediately corrected by boundary_evidence[3].) 5. src/actors/dScMgSingle3DBase_c.cpp: added the missing // @symbol func_ov006_0210a534 marker. _marked_member_fragment slices marker-to-next-marker, so without it the func_ov006_0210a600 fragment ran to end of file and swallowed a534's volatile body and raw addresses, keeping a600 -- an eight-byte `return 1;` -- from scoring readable. The unmarked D1/D0 are correct and unchanged. Two disclosures the review asked for: - config/tu_manifest.d/ov006/dScMgSingle3DBase_c.json: new boundary_evidence entry recording that func_ov006_0210a534 is the one member with NO vtable corroboration -- its membership rests on tu_map's contiguous run plus address containment, a weaker evidence class than the other eight. - src/actors/dScMgSingle3DBase_c.cpp: the signature reconciliation also added a `const` neither legacy file had (DecompressLZ16(const void*, void*)). Nothing in the ROM distinguishes const from non-const here, so it is now stated as a judgement call rather than left implied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
One more, found while reviewing the stacked #2067: a missing
|
| address | symbol | marker |
|---|---|---|
0x0210a4b0 |
_ZN19dScMgSingle3DBase_cD1Ev |
lifecycle fallback — fine |
0x0210a4e8 |
_ZN19dScMgSingle3DBase_cD0Ev |
lifecycle fallback — fine |
0x0210a534 |
func_ov006_0210a534 |
missing |
0x0210a600 |
func_ov006_0210a600 |
ok |
0x0210a608 |
_ZN19dScMgSingle3DBase_c21AfterCleanupResourcesEj |
ok |
0x0210a664 |
_ZN19dScMgSingle3DBase_c12BeforeRenderEv |
ok |
0x0210a698 |
_ZN19dScMgSingle3DBase_c14BeforeBehaviorEv |
ok |
0x0210a6e4 |
_ZN19dScMgSingle3DBase_c18AfterInitResourcesEj |
ok |
0x0210a708 |
func_ov006_0210a708 |
ok |
It matters more than one missing marker usually would, because of where it sits. _marked_member_fragment slices from one marker to the next one, and your last marker is at line 257 in a 323-line file. func_ov006_0210a534 is defined after it, so there is no next marker — the fragment for func_ov006_0210a600 runs 257 → EOF and swallows func_ov006_0210a534 whole. Two members are affected by the one omission: the unmarked one falls to the whole-file fallback, and the marked one gets scored against text that is not its own.
notes/tu-promotion-conventions.md §6 (landed as #2065) is the rule; it calls this "the cheapest rule in this note to satisfy and the most expensive one to skip", which is exactly the shape of it here.
I have asked for the same fix at larger scale on #2067, which needs eleven. Both are on your stack, so one push covers both.
|
All five review fixes pushed as ea9f055, plus the two disclosures you asked for. Comment and manifest text only — no code byte moves;
Disclosures:
I left lines 150/154 ( |
1 participant
Promotes
ov006/dScMgSingle3DBase_c— the base class of thirteen minigame scenes — from nine hand-placed one-function files into one C++ translation unit that emits its own destructor pair, vtable and RTTI records the way the cartridge's own build did.What lands
Nine functions,
.text 0x0210a4b0..0x0210a8c0(1040 bytes), onecompletedelink entry replacing nine:_ZN19dScMgSingle3DBase_cD1Ev_ZN19dScMgSingle3DBase_cD0Evfunc_ov006_0210a534func_ov006_0210a600_ZN19dScMgSingle3DBase_c21AfterCleanupResourcesEj_ZN19dScMgSingle3DBase_c12BeforeRenderEv_ZN19dScMgSingle3DBase_c14BeforeBehaviorEv_ZN19dScMgSingle3DBase_c18AfterInitResourcesEjfunc_ov006_0210a708The destructor pair is no longer hand-placed under
// @symbolmarkers. The class's key function pulls D1/D0 out of line through vtable slots 16/17, and mwcc emits both — which is the point of the whole workstream.The boundary is corrected, and the correction is proved
tools/tu_map.pyreported this unit as eight functions,0x0210a4b0..0x0210a708, and splitfunc_ov006_0210a708into a single-function unit of its own. That is a tu_map defect, not a real boundary: the function's symbol is still spelledfunc_<module>_<address>, so it gets no class label, so tu_map cuts there.The vtable says otherwise. Scanning every
_ZTVin ov006:0x0210a708is referenced at slot 33 by exactly fourteen vtables —dScMgSingle3DBase_cand all thirteen of its children.0x0210a600— slot 26, which tu_map did place inside the unit — is referenced by the same fourteen.Identical reference sets. One function tu_map kept and one it dropped are members of the same class. The unit is nine functions, and the ninth byte-matches inside the TU on the first compile.
Two accuracy defects fixed on the way
func_ov006_0210a600was labelled one class too deep. Its legacy source carriedrecovered name: dScMgFlower_c_OnHitByCannonBlastedChar.dScMgFlower_cis one of the fourteen classes that reference it — a body every sibling shares belongs to the common ancestor. Same off-by-one-class shape already documented forAfterInitResources.include/dScMgSingle3DBase_c.hcalled slots 26 and 33 "new at this class". They are re-overrides:_ZTV11dScMgBase_calready carries bodies at both (ov004:0x020b04e0 and ov004:0x020b265c, read out of the ROM). They stay undeclared only becausedScMgBase_cleaves slots 18-35 undeclared, and a derived class cannot override a slot the base has not spelled.Compiler-generated output: eleven licensed symbols
The TU emits three of its own records —
_ZTI/_ZTS/_ZTV19dScMgSingle3DBase_c— plus eight ancestor RTTI passengers. All eleven aredeadstrip-data, each naming the cartridge home it defers to, soromdata_checkbecomes binding for exactly those symbols and compares them relocations-applied before isolation discards them. Every canonical address is derived fromtubuild.all_symbol_homes()._ZTV19dScMgSingle3DBase_cscores PARTIAL, and that is expected rather than a disagreement:dScMgBase_cleaves slots 18-35 undeclared, so mwcc emits an 18-slot table where the cartridge has 36 — a verified byte-exact prefix. Owning it outright is the follow-up, mapped innotes/dScMgBase_c-slots-18-35.mdandnotes/dScMgSingle3DBase_c-vtable-ownership.md.Evidence
tubuild verify— 9/9 MATCH, objisolate clean, relocation destinations clean, emission order ROM-ascending.tubuild linkcheck— SCRATCH-LINK-VERIFIED; ov006 built 525344 == retail 525344; TU range IDENTICAL; ROM sha256d1506e90…c478e8, identical to the stock build.dsd check symbols --failreports 9 errors, all of which the--baselinecontrol reproduces exactly: 0 new, 0 resolved. That check is not green and is not reported as green.rombuildafter promotion — module fidelity, source fidelity and the compiler-only policy, against the real tree.Notes for review
completemarker.config/converted-baseline.jsonmoves six rows topromoted-path#symbolidentities, so the readability ratchet keeps function granularity — six out, six in, count invariant. Edited bytu_promote.py, not bytiers_ratchet.py --update, which would sweep unrelated baseline staleness into the diff.int, the othervoid *, and both passed the same value to the sameDecompressLZ16. One TU cannot hold both; the pointer form is the true one and removes four casts. Byte-neutral, andverifyre-confirms all nine.notes/actor-vtables.mdis repointed at the promoted path socheck_dead_referencesstays green.origin/main@9bfcd5d8(after TU: first compiler-built vtable — promote ov047/daObjKm3_Kurumajiku_c to intact-object production #2057 and docs: an unkeepable data_* entry MUST be dropped — correct anti-pattern 3 #2061 landed). The only conflict wasattribution.json, where both sides append to the same trailing line; resolved as the union of both blocks — main's fivedaObjKm3_Kurumajiku_crows and this branch's nine — hand-edited at the markers rather than resolved whole-file.config/converted-baseline.jsonauto-merged.tools/premerge_check.py HEADagainst the rebased base: all eight static gatespass → pass, nothing goes green → red.source-coveragenotesentries: 10933 -> 10925 (-8), which is the nine-into-one consolidation with bytes flat.