From 78269b26772a232608f4bb0e57b87695087a94f1 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 20:40:44 -0400 Subject: [PATCH] [tools] (P7): a bare decline count is logged only by a warning-or-error line in its own block, and a def row with a trailing comment is still a row Fable review of the B3 rework, should-fix: the 8-lines-above MGLOG_W/E window was branch-blind - a Count() in `if (x == 2) { ... }` whose only log sat in a preceding `if (x == 1) { ... }` passed, as did a log in a nested branch just above. All 12 bare sites in the tree are under their own line today, so this was a gate hole, not a present defect. has_own_log() walks upwards from the Count line (still capped at 8 lines): blank and deeper-indented lines (a log's continuation lines, a nested block) are passed over, only a line at the Count's own indentation can be its log, and the first line indented less than the Count - the block's opening, a sibling's `} else {` - ends the walk. WireDeclines.def is now read through the same comment/string strip, and ROW tolerates spaces inside the parentheses: `MGL_WIRE_DECLINE(Name) // note` used to fail `\)\s*$`, dropping the row, and its real site was then reported as naming no row. Tree: 53 rows, 53 with sites, 53 sites, 0 unlogged, rc 0 (unchanged). Fixtures the previous commit passed and this one reds: other-branch log, nested-branch log (rc 0 -> 1); a def row with a trailing comment plus its site (rc 1 -> 0). --- scripts/ci/wire_declines_audit.py | 56 ++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/scripts/ci/wire_declines_audit.py b/scripts/ci/wire_declines_audit.py index c782d1e6..1fe1edf2 100644 --- a/scripts/ci/wire_declines_audit.py +++ b/scripts/ci/wire_declines_audit.py @@ -14,10 +14,12 @@ Rows are APPEND-ONLY (the lanes assert specific counters), so (1) cannot be fixe deleting a row once a release has shipped it. This check is why it cannot rot. Sites reached through MGL_WIRE_DECLINE_AT satisfy both at once. A bare -WireDeclineTally::Count(WireDeclineSite::X) is allowed ONLY when an MGLOG_ statement carrying -the same reason already stands just above it. +WireDeclineTally::Count(WireDeclineSite::X) is allowed ONLY when an MGLOG_W/E statement stands +just above it IN THE SAME BLOCK: at the Count line's own indentation, with no line of lesser +indentation (an enclosing `if (...) {`, a `} else {`) between them. A log in a sibling branch +or a nested one is not this site's line. -Sources are matched on their text with comments and string/character literals blanked out +Everything is matched on the text with comments and string/character literals blanked out (newlines kept, so every report cites the real line): a site or a log that is only a comment, or only a string, is not one. @@ -30,10 +32,12 @@ import sys RENDERER = "MobileGL/MG_Backend/DirectVulkan/Renderer" DEF = os.path.join(RENDERER, "WireDeclines.def") SOURCES = (".cpp", ".inc", ".h") -# How far above a bare Count() an MGLOG_ may stand and still be "the line for this site". +# How far above a bare Count() an MGLOG_ may stand and still be "the line for this site" - +# a cap on top of the block walk, so a log far up the same block does not count either. LOG_WINDOW = 8 -ROW = re.compile(r"^\s*MGL_WIRE_DECLINE\(([A-Za-z][A-Za-z0-9]*)\)\s*$") +# Matched on comment-stripped text, so a row with a trailing `// note` is still a row. +ROW = re.compile(r"^\s*MGL_WIRE_DECLINE\(\s*([A-Za-z][A-Za-z0-9]*)\s*\)\s*$") AT_SITE = re.compile(r"MGL_WIRE_DECLINE_AT\(\s*([A-Za-z][A-Za-z0-9]*)\s*,") COUNT_SITE = re.compile(r"WireDeclineTally::Count\(\s*WireDeclineSite::([A-Za-z][A-Za-z0-9]*)\s*\)") # Warning or error only: an MGLOG_D / MGLOG_V is compiled away in the Release builds that ship, @@ -96,15 +100,40 @@ def strip_code(text: str) -> str: return "".join(out) -def main() -> int: - root = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else ".") +def indent_of(line: str) -> int: + return len(line.expandtabs(4)) - len(line.expandtabs(4).lstrip()) + + +def has_own_log(lines, i: int) -> bool: + """Is there an MGLOG_W/E statement above line i in the same block, within LOG_WINDOW? + + Walk upwards; blank lines and deeper lines (a log's continuation lines, a nested block) + are passed over, but only a line at exactly the Count's indentation can be its log. The + first line indented LESS than the Count is the block's opening (or a sibling branch's + `} else {`) and ends the walk.""" + own = indent_of(lines[i]) + for k in range(i - 1, max(-1, i - 1 - LOG_WINDOW), -1): + line = lines[k] + if not line.strip(): + continue + ind = indent_of(line) + if ind < own: + return False + if ind == own and LOGGED.search(line): + return True + return False + + +def audit(root: str) -> int: def_path = os.path.join(root, DEF) if not os.path.isfile(def_path): print("wire-declines: %s not found" % DEF, file=sys.stderr) return 2 rows = [] - for line in open(def_path, encoding="utf-8"): + with open(def_path, encoding="utf-8") as f: + def_text = strip_code(f.read()) + for line in def_text.splitlines(): m = ROW.match(line) if m: rows.append(m.group(1)) @@ -131,8 +160,7 @@ def main() -> int: for m in COUNT_SITE.finditer(line): name = m.group(1) counted.setdefault(name, []).append((entry, i + 1)) - window = lines[max(0, i - LOG_WINDOW):i] - if not any(LOGGED.search(w) for w in window): + if not has_own_log(lines, i): unlogged_counts.append((entry, i + 1, name)) orphan_rows = [r for r in rows if r not in counted] @@ -145,8 +173,8 @@ def main() -> int: print("wire-declines: %s:%d counts %s, which is not a row in WireDeclines.def" % (where[0], where[1], name), file=sys.stderr) for entry, line, name in unlogged_counts: - print("wire-declines: %s:%d counts %s with no MGLOG_ within %d lines above it - a decline " - "must be readable from the log, not only from a counter" + print("wire-declines: %s:%d counts %s with no MGLOG_W/E in its own block within %d lines " + "above it - a decline must be readable from the log, not only from a counter" % (entry, line, name, LOG_WINDOW), file=sys.stderr) bad = len(orphan_rows) + len(unknown_sites) + len(unlogged_counts) @@ -156,5 +184,9 @@ def main() -> int: return 1 if bad else 0 +def main() -> int: + return audit(os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else ".")) + + if __name__ == "__main__": sys.exit(main())