Add check_decl_return_types.py: one symbol, one declared return type (tool only, not wired into CI)

[andrewboudreau]

Adds tools/check_decl_return_types.py and its test suite. It is deliberately not wired into CI — see "Why this is not a workflow yet" below.

The gap

Every recovered C++ method is declared twice, in two files that never see each other:

include/decl_common.h    extern void _ZN11dScMgBase_c16OnAimedAtWithEggEv(void*);
include/dScMgBase_c.h    virtual int  OnAimedAtWithEgg();          /* slot 29 */

Neither header includes the other, so the compiler never compares them. A caller reaching the symbol through either one compiles cleanly regardless of what the other says. The linker binds on the mangled name and does not carry a return type.

So the disagreement survives everything we run: the byte gates pass because each translation unit is internally consistent, and check_header_offsets only reads field layout. The failure it allows is a caller that reads r0 from a callee which never wrote it, or the reverse — silent at compile time and at link time.

Nothing in CI catches this today. git grep decl_common tools/ finds only pr_linkcheck.py and resolve_placeholders.py, neither of which reads return types.

It reproduces a hand census exactly

I measured this by hand first, across the refs where it turned up. The tool run over the same four:

ref	disagreements	joined
origin/main	2	7
origin/cpp/minigame-slot28	2	7
origin/cpp/minigame-slot29	3	8
origin/cpp/minigame-slot30	4	9

Same numbers, same symbols. And it joins wider than my hand census managed — eight declarations here glue the sigil to the name (virtual Vector3 &GetPos();, virtual void *Unk_020c76d0();), which a whitespace-separated pattern reads as nothing at all. The hand census missed all eight; they agree, so the answer did not change, but the reach did.

The four rows on slot30:

class::method	class header	decl_common.h
dScMgBase_c::BeforeInitResources	bool	int
dScMgBase_c::AfterInitResources	void	int
dScMgBase_c::OnAimedAtWithEgg	int	void
dScMgBase_c::OnAimedAtWithEggReturnVec	int	void

Two directions, so this is not "one header is stale". decl_common.h is the minority witness in all four — dScMgD3DBase_c.h, dScMgSlot3_c.h and dScMgBase_c.h agree with each other every time.

bool vs int is not cosmetic: a bool return goes through a widening cast under mwccarm 2004/b56 that an int return does not, so norm_type keeps them distinct on purpose.

Why this is not a workflow yet

Two of the four disagreements — BeforeInitResources and AfterInitResources — are on main right now. A workflow calling this would fail every push from the moment it landed.

The two moves available are: fix the rows, or loosen the check. Loosening it is not on the table — a gate written to pass is not a gate. So this lands as a tool, and wiring it is a follow-up that becomes possible once the four rows are fixed at a single site (include/decl_common.h; a half-flip does not compile, it is an illegal function overloading error, which is the one mercy here).

Same two-step as #2082 → #2083 and #2091 → #2101.

What it reports about itself

The accounting prints on failure as well as under --summary, because someone reading a green run needs the reach as much as someone reading a red one:

  decl_common.h rows              64
    no return type (vtable/structor) 14
    not a class in include/          40
    class method, not virtual         3
    ambiguous (overloads)             0
    JOINED and compared               7

The unjoined rows are not silently counted as agreement. Most of them structurally cannot disagree: vtable and typeinfo data carry no return type, structors have none either, and the "not a class" rows are SDK namespaces (GX, Sound, cstd) whose free functions have no vtable slot. The three non-virtual rows I checked by hand — Player::SetPlayableSeqCount, dBgW::Disable, dBgW::IsEnabled — are genuinely non-virtual and agree anyway.

There is a blind-spot counter for virtual lines the matcher failed to read. It is currently zero, and getting it there is where the real work went — it went negative twice, which is how two matcher bugs surfaced that were also corrupting the map, not merely miscounting:

• a comment's parenthesis: virtual ~fBase_c(); /* slots 16 (D1), 17 (D0) */ matched as return type ~fBase_c(); /* slots and method name 16, inventing a virtual named 16 in 116 class bodies;
• a destructor borrowing its ( from the constructor declared lines below: virtual ~dActor_c(); matched as method name dActor_c, injecting a virtual named after the class.

Either one could have joined a decl_common.h row and compared garbage. Both are pinned by tests.

Design notes

• Parses backward out of the mangled name, rather than mangling each declared virtual and looking it up. A forward matcher has to guess parameter encodings to build the string and silently misses every method it guesses wrong. The mangled name already carries the answer.
• No --changed mode, on purpose. The invariant spans one file plus every class header, so editing a class header can break agreement with a decl_common.h row the diff never touches. A whole-tree run is cheap and is the only honest one.
• Overloads are reported as ambiguous, not resolved. Picking one would be a guess, and a wrong guess is a false failure on a correct tree. Currently zero.
• Brace matching in class bodies is done by hand — a non-greedy {.*?} stops at the first inner close, and these bodies nest. A test plants that as a false-green regression.

Testing

python -m unittest tools.test_check_decl_return_types — 23 tests, all passing. They cover the real disagreements, the shapes that must not fire, the two map-pollution regressions above, and the tool's own honesty about its reach (one test asserts a multi-line declaration IS parsed — I had documented the opposite, and the test is what caught the doc being wrong).

Not added to any workflow, so no CI file changes here.

[tangos-validator]

✅ PR validation — Passed

noverify: no source/build-data changes in this PR

_{Each changed src/*.c|*.cpp is compiled and its relocated bytes compared to the binary data on a private build box. Passing requires every changed file to reproduce the ROM byte-for-byte with correct relocation targets — this catches WRONG-DEST relocations and non-reproducing near-misses that ledger-scoped linkcheck skips.}