Use ROM-proven daObjWaterfall_c C++ identity - #2012
Merged
Merged
Conversation
✅ PR validation — PassedCommitted merge passes; 1 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (1)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (1 changed, 0 lost -- src/_ZN16daObjWaterfall_cD0Ev.cpp: tangosdev -> andrewboudreau); 5 linkcheck result(s) have unresolved relocations. Per-file link-check detailAll 1141 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
added
the
attribution-override
The only conflict was notes/cpp-tu-current-state.md, whose live counts are generated. Taken from main here and regenerated by tools/cpp_tu_state.py in the following commit rather than hand-resolved, per that file's own "Do not hand-edit live counts" header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
Three follow-ups to the daObjWaterfall_c identity change: - include/decl_common.h: drop `extern int _ZTV13WaterfallMist[];`. That symbol no longer exists in any symbols.txt after this PR renamed the 0x021094a0 vtable to _ZTV16daObjWaterfall_c, so the declaration is dead. (66 other dead _ZTV declarations in this header predate this PR and are deliberately left alone.) - notes/data/tu-merge-candidates.json: the ov002 tu_index 37 record now names the class daObjWaterfall_c and its five current file paths. - notes/data/c-cpp-classification.tsv: src/WaterfallMist_Spawn.c -> src/WaterfallMist_Spawn.cpp. The factory keeps its evidence-bounded C-ABI name; only the extension moved. Both notes/data snapshots follow the path-refresh precedent from #1985: paths (and here the class label) are corrected, generator-derived scoring fields are left for the next full regeneration. notes/cpp-tu-current-state.md regenerated with tools/cpp_tu_state.py --write-note; --check-note reports current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
Collaborator
Author
Gatekeeper review: cartridge evidence for the
|
| word | value | meaning |
|---|---|---|
0x0210945c |
0x0209a764 |
vptr -> _ZTVN3abi20__si_class_type_infoE (arm9) |
0x02109460 |
0x02109468 |
pointer to the type name string |
0x02109464 |
0x0208e390 |
pointer to _ZTI8dActor_c (arm9) -- the single base |
Single-inheritance from dActor_c is therefore stated by the cartridge, not
inferred from the header.
2. The name string. File offset 0x5be08 -> VA 0x02109468 holds
16daObjWaterfall_c\0. The leading 16 is the mangled-name length prefix
for the 16-character identifier daObjWaterfall_c -- it is not part of the
name. (This is the usual trap: _ZTS payloads are length-prefixed, so the
class is daObjWaterfall_c, and the correctly mangled symbols are
_ZTI16daObjWaterfall_c / _ZTS16daObjWaterfall_c / _ZTV16daObjWaterfall_c.)
3. The vtable, and that it is this class's vtable. The two words before
the address point are the standard prologue:
0x02109498 0x00000000 offset-to-top
0x0210949c 0x0210945c -> the RTTI record above
0x021094a0 0x020b6eac address point, slot 0 (InitResources)
So the table configured at 0x021094a0 is bound by the cartridge to the
daObjWaterfall_c typeinfo. Renaming _ZTV13WaterfallMist ->
_ZTV16daObjWaterfall_c is a correction, not a relabelling.
4. The factory reaches that exact table. The literal-pool word at
0x020b6f14, inside WaterfallMist_Spawn, is 0x021094a0 -- the address point,
not the prologue. The vptr store in the recovered factory is the ROM's store.
5. The coined name is absent from the cartridge. A byte census of the
string WaterfallMist across every image under extracted/ (arm9, arm9 decomp,
arm7, and all overlays) returns 0 occurrences. daObjWaterfall_c is
ROM-proven; WaterfallMist is a coined placeholder that only ever existed in
this repository.
Notes on scope, for the record
- Exactly one symbol is configured at
0x021094a0both before and after this
PR, so this is a rename and not the delete-the-coined-alias case from ov063: give daTBasket_c and daTrsTrap_c their ROM names and real vtable slots #1978.
There is no second name to drop. WaterfallMist_Spawn/WaterfallMist_SpawnInfoare deliberately left
alone. Those are C-ABI names with no mangled evidence behind them; the
ov018MotherPenguin/daPgMthr_cprecedent keeps the coined actor name on
exactly this kind of symbol.- The vtable move is atomic: all five text/data symbols and the
_ZTVmove in
the same commit, so no_ZTVentry is left pointing at the old spelling. - The
include/daObjMarioCap_c.hcomment edit is a genuine correction. I read
0x021093dc->0x021093a0->_ZTS11daObjLava_cat0x021093ac: the
preceding table really isdaObjLava_c's, so the old comment's claim that
0x021094a0was "the wrong table, one entry along" was factually wrong.
Follow-up pushed to this branch
origin/main merged in (only conflict was notes/cpp-tu-current-state.md,
resolved by regenerating with tools/cpp_tu_state.py --write-note; --check-note
reports current), plus three cleanups the rename left behind:
- dropped the now-dead
extern int _ZTV13WaterfallMist[];from
include/decl_common.h(the 66 other dead_ZTVdeclarations in that header
predate this PR and were left alone); - refreshed the
notes/data/tu-merge-candidates.jsonov002 record and
notes/data/c-cpp-classification.tsvrow for the new paths and class label,
following the path-refresh precedent from File WingFeather TU sources under game actors #1985.
Gates re-run on the merge result: port_refcheck 405/405, check_src_tu
(83 TUs, 1344 mangled refs, all resolve), check_src_tu_compiles 88/88,
check_dead_references (no new dead references), check_header_offsets
(3 changed headers, 0 mismatched, 0 unparsed; daObjWaterfall_c spans 0xdc),
langmode ratchet PASS, CONVERTED ratchet PASS (2552/2552, no backslide
exception added), tu_manifest list shows ov002/daObjWaterfall_c.
One cross-PR consequence
#1993 banks a deadstrip disposition for _ZTI13WaterfallMist whose stated
reason is "compiler-generated typeinfo record with no ROM symbol ... the
cartridge never carried a separate copy." Section 1 above disproves that: the
cartridge carries the record at 0x0210945c. The rationale was written against
the coined name, which had no _ZTI, and so it concluded the ROM had none.
That row needs revisiting before #1993 lands.
Resolves the only conflict, notes/cpp-tu-current-state.md, from the merged tree itself rather than by taking a side: git ls-tree on the merge result counts .c=6374 .cpp=4876 (total 11250), which is main's post-#1995 totals adjusted by this branch's one WaterfallMist.cpp -> daObjWaterfall_c.cpp flip. The note is derived (tools/cpp_tu_state.py --write-note) and nothing gates it, but lagging counts are still worth not committing.
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.
Summary
Verification