chore: flag fakematches without a Fakematch: comment

docs/code-cleanup.md

@@ -47,6 +47,16 @@ make it clearer, add a comment that explains why it looks odd.
 - edit files in `asm/`;
 - fix bugs from the original game.
 
+## Fakematches
+
+A fakematch is code that only steers the compiler: `x++; x--;`, `x = x;`, a
+`do { } while (0)` outside a macro, an `if (0)` branch, a dead store. It is
+allowed until someone finds the real form, but it needs a comment that starts
+with `Fakematch:` and says what it changes, ending at most three lines above
+it. `check-cleanup.py` finds the first four shapes without that comment (not
+dead stores or `x += 1; x -= 1;`, so mark those yourself). If
+removing the trick does not change the code, it is not a fakematch: delete it.
+
 ## Names
 
 - **Only name what you can prove.** Good evidence: recovered symbols

scripts/check-cleanup.py

@@ -19,6 +19,7 @@
 
 import argparse
 import ast
+import bisect
 import collections
 import importlib.util
 import json
@@ -75,6 +76,18 @@
 RAW_BYTES = re.compile(r"^(?:const\s+)?(?:u8|s8|(?:un)?signed\s+char|char)\s+(\w+)\s*\[[^\]\n]*\]"
                        r"(?:\s*__asm__\s*\(\s*\"([^\"]*)\"\s*\))?[^=;\n]*=\s*\{([^;]*?)\}\s*;", re.M)
 RAW_BYTES_MAX = 16
+# Code that only steers the compiler (docs/code-cleanup.md, Fakematches). Each use
+# needs a `Fakematch:` comment that ends at most FAKEMATCH_REACH lines above it.
+OPERAND = r"\*?[A-Za-z_][\w.\[\]]*(?:->[\w.\[\]]+)*"
+STEP = r"(?:(" + OPERAND + r")\s*(\+\+|--)|(\+\+|--)\s*(" + OPERAND + r"))"
+FAKEMATCHES = [
+    ("x++; x--", re.compile(r"(?<![\w.\]>])" + STEP + r"\s*;\s*" + STEP + r"\s*;")),
+    ("x = x", re.compile(r"(?:^|[;{}:)])[ \t]*((?:\*\s*)?\(?" + OPERAND + r"\)?)\s*=\s*"
+                         r"((?:\*\s*)?\(?" + OPERAND + r"\)?)\s*;", re.M)),
+    ("do { } while (0)", re.compile(r"\bdo\s*\{")),
+    ("if (0)", re.compile(r"\b(?:if|while)\s*\(\s*0\s*\)")),
+]
+FAKEMATCH_REACH = 3
 SNAKE = re.compile(r"^_*[a-z][a-z0-9_]*$")
 PLACEHOLDER = re.compile(r"^(?:fun|func|FUN|sub|D|D_L\d\d|FUN_L\d\d|jtbl|text_gap|lbl)_[0-9A-Fa-f]{8}\w*?$")
 HEXPART = re.compile(r"^(?=[0-9A-Fa-f]*\d)[0-9A-Fa-f]{6,8}$")
@@ -187,6 +200,65 @@ def param_names(params: str):
 
 # —— Per-file checks (run on changed files) ——
 
+def matching_code(text: str) -> str:
+    """Blank out preprocessor lines and NON_MATCHING drafts, keeping line numbers."""
+    out, stack, cont = [], [], False
+    for line in text.split("\n"):
+        s = line.strip()
+        if cont or s.startswith("#"):
+            cont = line.rstrip().endswith("\\")
+            m = re.match(r"#\s*(ifdef|ifndef|if|elif|else|endif)\b(.*)", s)
+            if m and m.group(1) in ("ifdef", "ifndef", "if"):
+                draft = "NON_MATCHING" in m.group(2)
+                stack.append([draft, draft and m.group(1) == "ifdef"])
+            elif m and m.group(1) in ("else", "elif") and stack and stack[-1][0]:
+                stack[-1][1] = not stack[-1][1]
+            elif m and m.group(1) == "endif" and stack:
+                stack.pop()
+            out.append(" " * len(line))
+        else:
+            out.append(" " * len(line) if any(on for _, on in stack) else line)
+    return "\n".join(out)
+
+
+def check_fakematches(rel: str, raw: str, text: str) -> list[Problem]:
+    code = matching_code(text)
+    starts = [0] + [m.end() for m in re.finditer("\n", raw)]
+    line = lambda pos: bisect.bisect_right(starts, pos)
+    marks = [(line(m.start()), line(m.end())) for m in re.finditer(r"/\*.*?\*/|//[^\n]*", raw, re.S)
+             if "Fakematch:" in m.group(0)]
+    out = []
+    for kind, pattern in FAKEMATCHES:
+        for m in pattern.finditer(code):
+            if kind == "x++; x--":
+                first, second = m.group(1) or m.group(4), m.group(5) or m.group(8)
+                if re.sub(r"\s", "", first) != re.sub(r"\s", "", second) or \
+                        (m.group(2) or m.group(3)) == (m.group(6) or m.group(7)):
+                    continue
+            elif kind == "x = x":
+                if re.sub(r"\s", "", m.group(1)) != re.sub(r"\s", "", m.group(2)):
+                    continue
+            elif kind == "do { } while (0)":
+                depth, i = 0, m.end() - 1
+                for i in range(m.end() - 1, len(code)):
+                    depth += {"{": 1, "}": -1}.get(code[i], 0)
+                    if depth == 0:
+                        break
+                if not re.match(r"\}\s*while\s*\(\s*0\s*\)\s*;", code[i:i + 40]):
+                    continue
+            elif m.group(0).startswith("while") and re.search(r"\}\s*$", code[max(0, m.start() - 20):m.start()]):
+                continue  # the end of a do { } while (0)
+            pos = m.start() + len(m.group(0)) - len(m.group(0).lstrip(";{}:) \t\n"))
+            at = line(pos)
+            if any(start <= at and at - end <= FAKEMATCH_REACH for start, end in marks):
+                continue
+            snippet = re.sub(r"\s+", " ", code[pos:m.end()]).strip()[:60]
+            out.append(Problem("fakematch", rel, f"{kind}: `{snippet}` only steers the compiler; "
+                               "say why in a `Fakematch:` comment just above it", at,
+                               f"{kind} {re.sub(chr(32), '', snippet)}"))
+    return out
+
+
 def check_c_file(rel: str, raw: str) -> list[Problem]:
     out: list[Problem] = []
     text = strip_comments(raw)
@@ -247,6 +319,8 @@ def check_c_file(rel: str, raw: str) -> list[Problem]:
             if REGISTER_NAME.match(name):
                 out.append(Problem("register-name", rel, f"local '{name}' is named after a register or a "
                                    "decompiler temp; name it by what it holds", line_of(text, m.start())))
+    if rel.endswith(".c") and not is_sdk(rel):
+        out += check_fakematches(rel, raw, text)
     return out
 
 

scripts/test_public_tools.py

@@ -2107,6 +2107,32 @@ def test_raw_bytes_in_c_fail(self):
         self.assertNotIn("message", out)
         self.assertNotIn("small_table", out)
 
+    def test_fakematch_needs_a_comment(self):
+        self.write("src/new_helper.c", textwrap.dedent("""\
+            #define ONCE(x) do { x; } while (0)
+            void new_helper(s32 count) __asm__("FUN_00100100");
+            void new_helper(s32 count) {
+                count++;
+                count--;
+                ONCE(count = 1);
+                do {
+                    count = 2;
+                } while (0);
+                /* Fakematch: the pair keeps count in its own register. */
+                count--;
+                count++;
+            #ifndef NON_MATCHING
+            #else
+                count = count;
+            #endif
+            }
+        """))
+        code, out = self.check()
+        self.assertEqual(code, 1)
+        self.assertIn("src/new_helper.c:4: [fakematch] x++; x--", out)
+        self.assertIn("src/new_helper.c:7: [fakematch] do { } while (0)", out)
+        self.assertEqual(out.count("[fakematch]"), 2)
+
     def test_recovered_names_format_and_config(self):
         entry = {"unit": "missing", "source_path": "src/missing.c", "address": "0x00100000",
                  "current_name": "FUN_00100000", "proposed_name": "OldHelper", "logical_group": "EE/x",

src/audio/decoder/audio_dec_start.c

@@ -15,9 +15,8 @@ void audio_dec_start(struct AudioDec *dec) {
     snd_start_movie_sound(dec->iopBuff, ((copy = temp_2_10) / 0x400) * four, dec->iopZero,
                           dec->hdr.rate, u18);
     dec->state = 2;
-    /* Allocator-shape pair: GCC removes both stores (code-dead), but the
-       read-modify-write sequence drives hdr.ch's register choice to retail's
-       schedule.  Verified 100/100/100 bytes. */
+    /* Fakematch: GCC removes both stores, but the read-modify-write
+       sequence gives hdr.ch retail's register. */
     dec->hdr.ch++;
     dec->hdr.ch--;
 }

src/audio/streaming/snd_play_vag_stream_by_loc_ex_cb.c

@@ -20,6 +20,7 @@ void snd_play_vag_stream_by_loc_ex_cb(s32 loc, s32 size, s32 vol_l, s32 vol_r, s
     int new_var;
     s32 buf[7];
     new_var = (pitch << 16) | (vol_l & 0xFFFF);
+    /* Fakematch: the dead id update moves the packet stores into retail's order. */
     id++;
     id--;
     buf[0] = loc;

src/math/classify_float_bits.c

@@ -1,6 +1,5 @@
 #include "types.h"
 
-/* fdlibm s_isnan.c: EXTRACT_WORDS() as a do { ... } while (0) macro body */
 typedef union {
     f64 value;
     struct {
@@ -9,17 +8,21 @@ typedef union {
     } parts;
 } ieee_double_shape_type;
 
+/* fdlibm math_private.h */
+#define EXTRACT_WORDS(ix0, ix1, d) \
+    do { \
+        ieee_double_shape_type ew_u; \
+        ew_u.value = (d); \
+        (ix0) = ew_u.parts.msw; \
+        (ix1) = ew_u.parts.lsw; \
+    } while (0)
+
 s32 classify_double_nan(f64 x) __asm__("ClassifyDoubleNaN");
 s32 classify_double_nan(f64 x) {
     s32 hx;
     s32 lx;
-    ieee_double_shape_type ew_u;
 
-    do {
-        ew_u.value = x;
-        hx = ew_u.parts.msw;
-        lx = ew_u.parts.lsw;
-    } while (0);
+    EXTRACT_WORDS(hx, lx, x);
     hx &= 0x7fffffff;
     hx |= (u32)(lx | (-lx)) >> 31;
     hx = 0x7ff00000 - hx;

src/math/geometry/point_is_clockwise.c

@@ -11,6 +11,7 @@ int point_is_clockwise(int a0, int a1, int a2, int a3, int t0, int t1) {
 
     dx = a0 - a2;
     dy = a1 - a3;
+    /* Fakematch: the block keeps the two differences below dx and dy. */
     do {
         int term1 = (t0 - a2) * dy;
         int term2 = (t1 - a3) * dx;

src/math/rotations/extract_matrix_rotation_quaternion.c

@@ -10,11 +10,11 @@ void extract_matrix_rotation_quaternion(s32 arg0, s32 arg1) {
 
     buf = sp_slot;
     copy_matrix_rotation(buf);
-    /* The single-iteration do/while wrappers are allocator-shape levers:
-       both compile to plain calls (code-identical) but select retail's
-       callee-saved register roles. */
+    /* Fakematch: the single-iteration do/while wrappers compile to plain
+       calls but select retail's callee-saved register roles. */
     do {
         matrix_to_quaternion(arg0, sp_slot);
+        /* Fakematch: see above. */
         do {
             copy_matrix3x4(arg1, sp_slot);
         } while (0);

src/overlays/l03/gameplay/entities/taxi.c

@@ -1336,15 +1336,17 @@ void FUN_L03_002da870(struct Moby *m) {
                     D_L03_001BA6D0.collected[(s16)((struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i))->save_id] =
                         ((struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i))->mission + 2;
                     o = (struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i);
-                    /* Same record as the || test in the level's other reward code; the
-                       do/while (0) blocks keep the loop notes that make gcc predict the
-                       0xFF branch taken and keep the group load ahead of the table load. */
+                    /* Fakematch: same record as the || test in the level's other reward
+                       code; the do/while (0) blocks keep the loop notes that make gcc
+                       predict the 0xFF branch taken and keep the group load ahead of the
+                       table load. */
                     if (o->mission == 0xFF)
                         goto record;
                     do {
                         group = o->mission;
                     } while (0);
                     if (D_L03_0015FC88[group] != 0xFF && D_0014C050[group + current_level_index * 16] == 0xFF) {
+                        /* Fakematch: see above. */
                         do {
                         record:
                             D_L03_001BB330.collected[(s16)o->save_id] = o->mission + 2;
@@ -1732,8 +1734,8 @@ void FUN_L03_002db558(struct Moby *m) {
                 D_L03_001BA6D0.collected[(s16)((struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i))->save_id] =
                     ((struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i))->mission + 2;
                 o = (struct Moby *)((v->reward << 8) + D_L03_0015FFD8_i);
-                /* Same record test as above; the goto and the do/while (0) loop note keep
-                   the group byte's zero-extend ahead of the table address. */
+                /* Fakematch: same record test as above; the goto and the do/while (0)
+                   loop note keep the group byte's zero-extend ahead of the table address. */
                 if (o->mission == 0xFF)
                     goto rec2;
                 do {