Call 42 members by name instead of by mangled symbol - #2726
Conversation
Each of these files reached a member through an `extern "C"` declaration of its
Itanium mangled name -- `_ZN22ExpandingHeapAllocator16AllocateForwardsEjj(this,
size, align)` -- which is what `tiers.py` counts against `no_mangled_refs`, the
last criterion these 42 needed for CONVERTED.
Three spellings, picked by what the header actually declares:
member, receiver type matches this->AllocateForwards(size, align)
member, receiver is a stand-in ((dEnemyBase_c *)c)->UpdateYoshiEat(...)
namespace, no implicit this Sound::PlayBank3(0x62, *(const Vector3 *)&v)
The middle form is the common case and deserves a note. Most of these files
declare a local stand-in struct for the class rather than including the real
header, so the struct is unrelated to the one that owns the member and a plain
`c->UpdateYoshiEat()` does not resolve. A C-style cast to the declared type
costs no instruction and removes the mangled name; adopting the real types is
the next layer of the migration, not this one.
`IRQ::DisableIRQs` and `Sound::PlayBank3` are namespace-scoped, so the mangled
name carries no implicit `this` and the first argument is a real argument --
converting them as members ate it. `dActor_c::DistToCPlayer` and
`FarthestPlayer` already called `ClosestPlayer()` as a member; only a dead
`extern` of the mangled name was left, so it is simply gone.
Every file is byte-verified under 2004/b56 individually and in the full build:
[3/6] mwccarm: 8760 enrolled source file(s), -j16
8718 reused from cache, 42 compiled
source-built functions: 11,206 reproducing: 11,206 mismatching: 0
module fidelity: 106/106 exact, 100.000000% of compared bytes
ROM-build analysis: PASS
check_decl_agreement: no new disagreements (44 banked ones are now gone, since
the removed externs were themselves the contradicting declarations).
tiers_ratchet --check: PASS, baseline 2812 -> current 2855.
port_refcheck: 423 references, 0 stale.
Nine of the touched calls are HAL seams that `port/hal/*.cpp` spells as C
functions, so the MSVC port smoke build was run too: 230/230, all five smoke
executables link.
16 of the 58 candidates are not here. They fail to compile for reasons the call
spelling cannot fix -- a local `struct Vector3` colliding with the real header,
a header declaring `ApplyOpacity()` with an arity the call site contradicts, and
one mwccarm internal compiler error. They are left calling the mangled name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Per-file link-check detailAll 42 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. |
|
Merge-tree check: Each of these PRs is gated alone, so green-alone does not imply green-merged. I built the three-way merge locally and ran the full set:
File sets are disjoint across all three pairs, so there is no textual conflict either. One thing specifically checked, since it would have been invisible otherwise: #2727 carries a 🤖 Generated with Claude Code |
Five-way merge tree gated (main + #2725 + #2726 + #2727 + #2728 + #2729)Each of these five PRs is green on its own, but this repo has produced a red Disjointness. 122 changed files across the five; all 10 pairs share zero files, Cold rebuild ( The rebuild is deliberately cold. A warm run on this same tree reported
🤖 Generated with Claude Code |
Adversarial review — seven-PR set (#2725–#2731)Re-reviewed as a critic rather than as the author. Every check below is new work, not a 1. Relocation destinations — the hazard the byte gate cannot seeA relocated word is a wildcard: a
0 WRONG-DEST across 125 files and 61 TU functions. This also closes the cast-receiver 2. Merge tree, cold
3. Change-kind census139 files changed in the merge tree, all 4. Local-only gates, run on the merge tree (CI runs none of these on a merge tree)
5. The merged decl baseline is not a silent clobber
0 hidden. Every difference is in the safe direction. (Separately: ~84 stale banked entries 6. The MSVC port build — differential, not absolute
The other six PRs introduce zero new port link errors. What remains is the pre-existing 7. Per-PR findings
VerdictNo defect found, and no change requested. Both incomplete migrations are incomplete on The one thing this review cannot supply is the thing the review gate actually wants: I am the 🤖 Generated with Claude Code |
Two conflicts, both resolved by hand. src/game/actors/daTrs_c/_ZN7daTrs_c16CleanupResourcesEv.cpp (modify/delete): this branch deletes the one-function file because the class TU d_a_trs_cleanup_render.cpp absorbed the body; main's a0fc674 (tangosdev#2726) had converted three mangled-symbol calls in it into real member calls. Checked the absorbed copy before taking the delete: it already carries that conversion, and carries it further -- dActor_c::FindWithID(), mFoundActor->MarkForDestruction() through a typed dActor_c * member instead of an fBase_c cast, and an unqualified UnloadCapModel() inherited from dCapEnemy_c instead of a dCapEnemy_c cast. Nothing from tangosdev#2726 is reverted, so the file is taken as deleted. notes/tu-filename-reconstruction-pilot.md (content): main's 0fb9ed8 (tangosdev#2724) link-ified the pilot tables; this branch repointed the TERESA and BOSS_TERESA rows at d_a_trs_factories.cpp. Kept main's links and headings with this branch's targets. Main's spelling of those two rows links at d_a_trs_teresa.cpp and d_a_trs_boss_teresa.cpp, which this promotion deletes -- taking it wholesale fails check_dead_references.py on the repo-rooted path inside the link text (verified by probe). config/decl-agreement-baseline.json auto-merged without a swap: a regenerated baseline is a strict subset of the merged one (+0 entries), so nothing phantom was invented and nothing was laundered. The 28 entries added against main are the func_ov063_021166ac.cpp move, the CleanupResources absorption, and the plurality flips that follow from this branch retiring the Player::ShowMessage and dCapEnemy_c mangled externs. The 97 now-stale banked rows are left alone: the gate is a ratchet, not a cleanup, and reports them as "not a failure". config/converted-baseline.json: 2813 entries, count field agrees, set unique and sorted; the only change is OnPendingDestroy moving to the "#" form inside the class TU. Validation on the merged tree: rombuild.py -j16 --no-rom, cold cache: 8752 enrolled, 0 reused, 8752 compiled; 11,206 source-built functions reproducing, 0 mismatching; module fidelity 106/106 exact, 100.000000%; ROM-build analysis PASS check_decl_agreement.py: no new declaration disagreements check_dead_references.py: no new dead references, no broken links check_src_tu.py: every reference resolves check_tubuild_conflicts.py --list: every recorded conflict matches Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
1 participant
42 files that reached a member through an
extern "C"declaration of its Itanium mangled name now call it by name. That declaration is whattiers.pycounts againstno_mangled_refs, which was the last criterion each of these needed for CONVERTED.CONVERTED 2812 -> 2855. No header changes;
src/only.Three spellings, chosen by what the header declares
this->AllocateForwards(size, align)((dEnemyBase_c *)c)->UpdateYoshiEat(...)thisSound::PlayBank3(0x62, *(const Vector3 *)&v)The middle form is the common case and is the part worth reviewing. Most of these files declare a local stand-in struct for the class instead of including the real header, so that struct is unrelated to the one owning the member and a plain
c->UpdateYoshiEat()does not resolve. A C-style cast to the declared type costs no instruction and removes the mangled name. Adopting the real types is the next layer of the migration, not this one.Two smaller cases:
IRQ::DisableIRQsandSound::PlayBank3are namespace-scoped, so the mangled name carries no implicitthisand argument 0 is a real argument -- converting them as members ate it.dActor_c::DistToCPlayeranddActor_c::FarthestPlayeralready calledClosestPlayer()as a member. Only a deadexternof the mangled name remained, so it is simply deleted; those two diffs are a single removed line each.Gates
Every file was byte-verified individually with
build_pin.verifyunder 2004/b56, and again in the full build:42 compiledis the whole changed set, so this is a real recompile rather than a cache pass-through.check_decl_agreement.py-- no new disagreements. 44 banked ones are now gone, because the removed externs were themselves the contradicting declarations.tiers_ratchet.py --check-- PASS.port_refcheck.py-- 423 references, 0 stale.port/hal/*.cppspells as C functions, and no ROM-side gate compilesport/, so this one was run by hand.What is deliberately not here
16 of the 58 candidates fail to compile for reasons the call spelling cannot fix, and are left calling the mangled name:
struct Vector3/struct ModelBasecolliding with the real header once it is included (structural -- needs the type adoption pass)ModelBase::ApplyOpacity()declared with an arity the call site contradicts, i.e. a header defect to settle separatelyCClass.cline 3328)They cost nothing to revisit later; each was reverted to its original matching state.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c