TU: inline the destructors, then promote twenty more TUs - #1882
TU: inline the destructors, then promote twenty more TUs#1882andrewboudreau wants to merge 3 commits into
Conversation
Twenty text-verified TUs were blocked on one thing, and it was source form,
not tooling. Each defined its destructor OUT OF LINE:
Seaweed::~Seaweed()
{
}
mwcc answers that form with `D2, D0, D1`. The cartridge has `D1` then `D0`
and no `D2` at all. Production isolation places an object's `.text` sections
into the spanning delink in emission order, so that disagreement is not
cosmetic -- it is the whole reason these entries could not be promoted.
Moving the empty body into the class body (`virtual ~Seaweed() {}`) is the
form the cartridge's own header must have had: it emits `D1, D0`, in that
order, first, and emits no `D2`. The homeless `_ZN<C>D2Ev` each of these was
carrying -- a symbol with no `symbols.txt` entry, dead-stripped only by an
explicit compiler-only policy row -- simply stops existing, so ten of those
policy rows come out here.
`tools/tu_order_check.py` compiles each TU once and reports both questions
together: does every licensed function still reproduce its ROM bytes, and are
they emitted in ROM order. All twenty answer ALL MATCH, ROM order.
Five of them re-emit `_ZN7Vector3D1Ev`, the deliberately empty inline
destructor `include/types.h` gives `Vector3`. That is vague linkage doing
exactly what it is supposed to, and the `deadstrip-duplicate` disposition
already models it: rombuild proves this object's copy against the cartridge's
bytes at `arm9:0x020072c0` before discarding it. `--write-policy` writes those
rows, and only the two shapes with a mechanical justification -- a homeless
D2, and a duplicate with exactly one configured home. Anything else it
reports and leaves for a human, because the reason column is the point.
ov013/ClockPaintingPendulum is left out: inlining its destructor made the TU
emit no destructor variants at all, because nothing in it odr-uses them. That
is the documented second condition -- the TU must actually instantiate the
class -- and it is a separate piece of source work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VregK5ZWRa2NbUcneaprG6
The compiler places these constructors and destructors now. Each of these
twenty overlays used to carry one file per mangled symbol -- 145 of them --
each holding a single function under a `// @symbol` marker, with production
isolation keeping exactly one variant out of whatever mwcc emitted alongside.
That whole apparatus exists because the reconstructed TU could not be linked
as a TU. Now it can be: the previous commit gave each of these the source form
that emits `D1, D0` in the cartridge's order, so the spanning delink entry
lines up and the per-function files have nothing left to do.
src/actors/Seaweed.cpp:
complete
.text start:0x0208d4c8 end:0x0208d7bc
replaces eight entries, and the eight files behind them are gone.
`ChillBully+daIDonketu_c.cpp` became `ChillBully_daIDonketu_c.cpp` on the way
in. src_tu/ spells a multi-class TU with `+`, and nothing ever links those,
so the convention was safe right up to promotion -- dsd writes bare, unquoted
object names into the linker command file and mwldarm's parser reads `+` as
punctuation:
mwldarm.exe: File not found: ChillBully
mwldarm.exe: Expecting: (
which fails the entire ROM, not one overlay. tu_promote now refuses a
promoted_source outside `[A-Za-z0-9_./-]` so the next one costs a second
rather than a link.
module fidelity: 106/106 exact, 100.000000% of compared bytes
source-built functions: 11,065 reproducing: 11,065 mismatching: 0
145 attribution overrides carry each absorbed symbol's authors onto the TU
that now holds it, so the consolidation reads as consolidated, not lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VregK5ZWRa2NbUcneaprG6
145 files stopped existing, so everything naming one had to move with them. `port/slice_gate9.txt` listed ArrowSignRight's five per-function objects plus its spawn; they are one file now, so the slice names that file once. The gate still describes the same slice -- a real actor spawning through its factory and dispatching through its vtable -- with fewer moving parts. tools/test_tubuild.py was pinned end to end to ov045/PoleLift, which this branch promoted out of src_tu/ entirely, so four of its cases were asserting against a file that no longer exists. Repointed at ov002/daObjAbuku_c: the same seven-function shape, still text-verified, still 7/7 contribution- equivalent, and blocked from promotion for an unrelated reason (it owns delinked data), so it will stay a valid subject for a while. Two of its numbers were pinned to whatever PoleLift happened to emit. The unlicensed inventory read 15 when it was written, 12 once someone measured it, and 6 for the new subject -- three different answers to a question about one TU's incidental output. The comment now says so and says to re-measure rather than tune. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VregK5ZWRa2NbUcneaprG6
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Warnings: 145 address range(s) left the byte-verified set while enrolled totals held steady: ov002:0x020b05d0-0x020b0600, ov002:0x020b0600-0x020b0644, ov002:0x020b0644-0x020b064c, ov002:0x020b064c-0x020b0650, ov002:0x020b0650-0x020b0658, +140 more; 125 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 20 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. |
…he class
Twenty-five reconstructed translation units become canonical here, absorbing
346 legacy one-function sources. `tools/rombuild.py -j16 --no-rom` reports
106/106 exact, 11,065 functions reproducing, 0 mismatching.
WHY THESE TWENTY-FIVE NEEDED AN EDIT FIRST
`tools/tu_order_check.py` across the 51 text-verified entries said "text-
verified" is not "promotable": 14 were clean, 34 emitted `D2, D0, D1` where
the ROM has `D1` then `D0` and no `D2` at all, 2 emitted extra symbols and 1
emitted nothing. The 34 all shared one cause -- an OUT-OF-LINE empty
destructor body, which makes mwcc emit the D2 base variant. Moving the empty
body into the class declaration (`virtual ~C() {}`) is the form #1882
established as period-accurate, and it emits `D1, D0` in ROM order with no
D2. All 34 were fixed by that one edit; zero still emit the wrong order.
Nine did not survive the rest of the gate and are NOT in this commit:
* Eight hit #1882's documented second condition -- nothing in the TU
odr-uses the destructor, so with the body inline the TU emits none at
all: Flag, LightBeam, Trap, ClockPaintingPendulum, IceSlideManager,
RacingPenguin, RotatingPlatformWdw, KoopaFlag. Reverted.
* ov014/ChainChomp is refused for a different, genuine reason: a surviving
.text section references the compiler-only `_ZN7Vector3D1Ev` at 0xa0, so
that duplicate cannot be dropped. Left text-verified.
Three obsolete `_ZN<C>D2Ev` policy rows come out (HealingHeart, Number,
WingFeather): isolation refuses a policy for a symbol the object no longer
defines. WingFeather's `_ZN7Vector3D1Ev` row claimed `deadstrip`, which was
a latent false claim -- the symbol HAS a ROM home at arm9:0x020072c0, so it
is `deadstrip-duplicate` with that home named.
FOUR TUs CARRIED STALE SPELLINGS AND WOULD NOT LINK
The shadow sources predate renames that landed on main, so promoting them
broke the link on four undefined symbols. Each is repointed at the name the
tree uses now, which is also the more readable form:
* daObjRc_Dorifu_c, daObjKm1_Dorifu_c, RickshawPlatformBs -- the two
`func_ov002_020b4b6c` / `func_ov002_020b4d58` helpers became
`daObjDorifu_c::CleanupResources` / `::InitResources` in #1832. The
hand-rolled `ResourceDescriptor` shim and its `extern "C"` block are
gone; each file now declares `extern daObjDorifuResources data_...[5]`
and calls the base method, matching the per-function sources it absorbs.
* Chuckya -- `func_ov002_020ada40` became
`_ZN12dEnemyBase_c20KillByInvincibleCharERK10Vector3_16R6Player5Fix12IiE`
in #1839 and `data_02082128` is `IDENTITY_MATRIX4X3`. Its local
declaration of the first is dropped: `include/decl_Enemy.h`, already
included, declares it, and two extern "C" declarations with different
parameter types is `illegal function overloading`.
THREE PATH-KEYED GATES, INCLUDING TWO THIS BRANCH WAS ALREADY RED ON
An agent review of the previous commit found two CI gates that a promotion
turns red and that commit did not mention. Both are addressed here for the
whole branch, not just this batch:
* dead references: promotion deletes the paths prose names. 36 references
across 11 files are repointed at the owning TU (the `src/actors/*.cpp`
that now contains the code); `port/docs/mmio-inventory.md`'s Squasher
line number is corrected to its new line. Two docstrings in
`tools/check_src_tu*.py` are DELIBERATELY historical -- they describe
what was true when a past PR landed -- so they are baselined instead of
rewritten, which is what `--update` is for.
* converted ratchet: 278 banked paths are removed and 372 added
(1958 -> 2052). Only ~80 of the removals are this branch's; ~99 were
already red at the branch base and 4 on main. Every removed path lives
on inside its owning TU, which 106/106 with mismatching 0 proves.
* langmode: `cv_launder` rises 3 -> 4, `cv_launder_sites` 6 -> 7. The
audit enumerates `src/` only, so promoting a TU moves its codegen levers
from unscanned into scanned territory. The single new site is
`src/actors/ActorBase.cpp:283`, `volatile int *p = data_02099f24;`,
which arrived by `git mv` in the previous commit and is load-bearing for
that function's bytes. The root `langmode-baseline.json` override is
refreshed; every other count in it FALLS (launder_or_forced 213 -> 212,
extern_vtable 347 -> 169, pad_layout 614 -> 523), so this is a net
tightening.
The seven remaining "Manually curated shadow translation unit." banners are
corrected -- five of them from the previous commit, which this pass missed
because its regexes did not cover that wording.
WHAT THE NUMBERS DID
`port_refcheck` stays at 385, all resolving. The ROM-data headline moves
373/202/53 -> 376/202/50, and a per-symbol `--data-json` diff against the
previous commit shows ZERO symbols worse, ZERO better, ZERO gained and ZERO
lost: the record count falls 7,054 -> 6,795 purely because consolidation
removes duplicate per-function records for the same symbol. This is the
metric that has misled this project before, so it was diffed rather than
read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HiJPTehCABiu3NQUCz7uDq
…ns as deletions (#2007) Two defects in the CONVERTED-tier gate, both of the same species: the gate counted something other than what it says it counts. 1. tools/tiers.py scored hardware registers as match hacks `no_codegen_trick` ORed LAUNDER, VOLATILE and ASM, and VOLATILE was a bare `\bvolatile\b`. On a Nintendo DS the only way to reach VRAM, the geometry engine or the IPC/DMA/divider registers is a volatile-qualified pointer, so the criterion failed the code that had no alternative: src/_ZN8dScene_c22ResetHardwareRegistersEv.cpp 74 hits, all 0x0400xxxx src/_ZN2GX13SetBankForTexEt.cpp 25 hits, all VRAM banks src/_ZN3G2x12SetBGyAffineEPVtP9Matrix2x2iiii.cpp the block is a PARAMETER A reconstructed TU absorbing any of those inherits the failure whole, so it compounds with the TU work rather than sitting still. The shapes are separable by WHAT is volatile-qualified. MMIO qualifies the pointed-to type, so a `*` follows it. A match hack qualifies an OBJECT -- `volatile int li;`, `volatile Vector3 v;`, the `(s32)(volatile s32)rsc` round-trip, `Node *volatile arr[4]` where the pointer not the pointee is volatile. VOLATILE = re.compile(r"\bvolatile\b(?![\s\w:]*\*)") Files scoring a codegen trick: 655 -> 254. All 401 released were checked to be MMIO-only, and the 254 kept still contain every match-hack form -- the negative direction was measured, not assumed. CONVERTED 2,511 -> 2,568 functions (22.20% -> 22.71%); no_codegen_trick 10,623 -> 11,020. Additions only. 2. tools/tiers_ratchet.py reported a TU promotion as a vanished file A promotion consolidates N per-symbol src/_ZN....cpp files into the one src/actors/X.cpp they always were; git records N deletions plus one addition, and every one read as `GONE -- not a tracked source file any more`. Measured on PR #1882 (tu/inline-dtor-order, 9c6396c): 90 of 90 backslid paths were TU `legacy_source` entries whose TU is "status": "promoted" and whose `promoted_source` exists on the branch. Zero were real deletions. A GONE path is now resolved through the manifest (via tools/tu_manifest.py, never the files) and reported as a MOVE naming the absorbing file and what that file does with the five criteria. A promotion is NOT free. The criteria are file-wide, so a clean function merged into a file with one bad line loses its status, and that still exits 1. Only a move into a file that itself passes all five is silent. In practice a promotion lands in the failing case by construction -- a reconstructed TU must spell _ZN7fBase_cnwEj, _ZN8dActor_cC2Ev and _ZN8dActor_cD2Ev directly or its range will not link -- so no_mangled_refs can never pass for one. That is structural; the answer is --update --reason, not exempting mangled refs. Re-banked config/converted-baseline.json in the same commit: 1,957 -> 2,567, +610 / -0. Set-diffed against HEAD to confirm zero removals. tools/test_tiers.py pins both readings in both directions (29 tests) and converted-ratchet.yml now runs it, plus watches tu_manifest.py and config/tu_manifest.d/**. Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
Triage: leaving this open, but it is the weakest case in the chain. This PR promotes 20 TUs. #1993 (
The one it does not cover is
Note the second half of that: the same validator constraint that blocked #1993 also blocks this PR from promoting Also note Left open only because it is technically the sole carrier of the |
|
Gatekeeper review — keep open, and the blocker on your one uncovered promotion has just been lifted. Status: this PR is not superseded by #1993. I checked the coverage rather than assuming it. The #1880 → #1882 → #1884 chain promotes 66 TUs against #1993's 26, and five promotions are uncovered by anything else in the queue: The PoleLift finding. #1993 deliberately dropped PoleLift in That is no longer true as of today. So a single PR can now promote PoleLift and update the four fixture assertions in the same change. Please verify that empirically before betting the branch on it — #1994's own diff is only the comment, so the executable half lives in the self-hosted worker outside this repo, and the title's "now" is a claim I could not confirm from the repo alone. What I need before this can merge, in order:
Do not run |
|
Status note, so nobody spends a validator slot chasing this. The red That is three days old. It predates #2007's re-bank, #2011, and today's merges (#1991, #1978, #2000, #1985). This is almost certainly the base-desync shape rather than a real backslide: a PR cut before a re-bank merges against a baseline that predates it and inherits the red. The fix for that is to merge Two corrections to what I told people earlier, both mine and both wrong:
Why this is parked rather than refreshed#1993 is the restack of this chain, and it is the queue chokepoint: it regenerates Order I am walking: #2002 → #2005 → #1995 → #1993, then re-assess what of this chain is still unlanded. Current signal is that the remainder is smaller than the diffstat suggests — a parallel review of #1914 found all 37 of its uniquely-promoted sources already present on Nothing here is closed and nothing is being discarded. Coverage on main is holding at 2,066,772 B built from source, 0 bytes handed back to the cartridge, and I will re-check it after every merge. |
|
Closing — see #1880 for the full reasoning. This PR sits on #1880's rejected Two things specific to this PR: 1. It reintroduces coined names main has already recovered. Main superseded this TU in
Merging this resurrects two 2. The The destructor-inlining insight is sound and worth redoing from current main — the 11 removed |
1 participant
Stacked on #1880. Review that one first.
The blocker was source form
#1880 said the remaining ready entries were blocked on one thing and it was
not tooling: their TUs define the destructor out of line, so mwcc emits
D2, D0, D1while the ROM hasD1thenD0and noD2. Productionisolation places an object's
.textsections into the spanning delink inemission order, so that disagreement decides whether the range links.
Moving the empty body into the class body is the whole fix:
That is the form the cartridge's own header must have had. It emits
D1, D0,in that order, first — and emits no
D2, so ten hand-writtencompiler-only_outputdead-strip rows for homeless_ZN<C>D2Evsymbols comeout of the manifest at the same time.
Twenty TUs took it, and
tools/tu_order_check.py(new) says so for each:one compile, and it answers both questions together — do the licensed
functions still reproduce their ROM bytes, and are they emitted in ROM order.
What lands
145 hand-placed structor files deleted. Twenty overlays each carried one
file per mangled symbol under a
// @symbolmarker. The compiler placesthem now.
Five
deadstrip-duplicaterows for_ZN7Vector3D1Ev, written bytu_order_check --write-policy. It writes only the two shapes with amechanical justification — a homeless D2, and a vague-linkage duplicate with
exactly one configured home — and reports anything else for a human, because
the reason column is the point.
ChillBully+daIDonketu_c.cpp→ChillBully_daIDonketu_c.cpp. src_tu/spells a multi-class TU with
+; nothing ever links those, so the conventionwas safe right up to promotion. dsd writes bare unquoted object names into
the LCF and mwldarm reads
+as punctuation:which fails the whole ROM, not one overlay.
tu_promotenow refuses apromoted_sourceoutside[A-Za-z0-9_./-], so the next one costs a second.Gates
ROM data went up, measured against this branch's base:
layout_checkclean ·port_refcheck400/400 ·check_dead_referencesno new·
check_src_tu_compiles88/88 ·prepush_attribution154 consolidated withcredit intact, 0 changed, 0 lost ·
pytest tools/test_{objisolate,rombuild,romdata_check,tubuild}.py96 passed.Two things a reviewer should know
The middle commit does not build on its own. Inlining the destructor makes
the header's inline body collide with the legacy out-of-line
_ZN<C>D1Ev.cppstill sitting in
src/, and the promotion commit is what deletes those. Thetwo are one change split for rename detection (the twenty
git mvs recordR100 that way); the branch head is what is gated. Do not bisect through
c18849e8aexpecting a build.tools/test_tubuild.pywas pinned end to end toov045/PoleLift, whichthis branch promotes out of
src_tu/entirely. Repointed atov002/daObjAbuku_c— same seven-function shape, still text-verified, still7/7 contribution-equivalent, and blocked from promotion for an unrelated
reason (it owns delinked data), so it stays a valid subject. Its unlicensed
inventory count has now been three different numbers (15 when written, 12 once
measured, 6 for the new subject); the comment says to re-measure rather than
tune it.
Not in this batch
ov013/ClockPaintingPendulumtook the inline change and then emitted nodestructor variants at all — nothing in the TU odr-uses them. That is the
documented second condition (the TU must actually instantiate the class) and
is separate source work, so it is left out.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VregK5ZWRa2NbUcneaprG6