mirror of
https://github.com/Druthulu/BFM-decomp
synced 2026-09-28 06:49:47 -04:00
Drew T
329ab4cbe7
TWO REAL BUGS in classify(), and an HONEST CORRECTION of their blast radius (P9/R14).
- BUG 1 (under-count). classify() decides definition-vs-declaration by scanning to the first `{` or
`;`. A K&R definition puts its parameter declarations BEFORE the brace:
s32 func_8015AE2C(arg0)
s32 arg0; <- a `;` before the `{`
{ ... }
so it was read as a forward declaration and dropped into NO bucket — not REAL, not a stub,
invisible. And a K&R def is MANDATORY whenever a zero-arg engine_core.h thunk calls the function,
i.e. exactly the heavy-jr cores our own banking recipe produces: func_8015AE2C (562x134),
func_8015A3C8 (493x132), func_80166994 (369x134) were all compiled, linked and BYTE-IDENTICAL in
the shipped build while counting as zero. Fix: skip over K&R parameter declarations (a bare
`<type> <name>;` carrying no parens — that is what distinguishes it from a wrapped ANSI
prototype's continuation line, which always carries the `)`).
- BUG 2 (over-count). `real |= dedup_members(BINARY)` folded in EVERY registered dedup member without
checking it is actually instantiated. A member still sitting as an INCLUDE_ASM stub was counted
REAL *and* stayed in `stubs` — double-counting into `matchable` and inflating `byteident`
(532 phantom instances, per the scanner audit). Fix: subtract `stubs`. The registry is advisory;
the source tree is authoritative.
- COVERAGE ASSERTION (the rule ratified 2026-07-14): ground truth = every function splat emitted a
.s for. Anything classify() cannot place in ANY bucket is now reported LOUDLY (stderr + the .md),
because a silent skip is a defect, not a no-op. Currently: 0 unplaced.
- CORRECTION (this is the part that matters — I over-claimed and the bytes refuted me). The scanner
audit reported ~243k instructions "counted as nothing", and I repeated it. WRONG. weighted_metrics()
— which produces the HEADLINE instr-weighted and distinct-code numbers — does NOT call classify()
at all. It tests `func not in src_stubs(binary)`: since the fleet is 136/136 byte-identical,
anything not wrapped in INCLUDE_ASM must be compiled C emitting the exact original bytes. That test
never parses a definition, so it is IMMUNE to this bug. Verified: old-vs-new on the same tree gives
identical weighted numbers. The published 65.6% / 44.9% were CORRECT ALL ALONG; only the secondary
REAL count and fn-count % were wrong.
THE LESSON, sharper than the one we started with: a metric DERIVED FROM A PROVEN INVARIANT beats a
metric that RE-PARSES THE WORLD. weighted_metrics() leans on the byte-gate and inherits its
correctness; classify() re-derives the same fact by parsing C and inherited a bug instead. Prefer
the former wherever an invariant exists.