diff --git a/scripts/ci/wire_declines_audit.py b/scripts/ci/wire_declines_audit.py index 34903a4e..952c8d43 100644 --- a/scripts/ci/wire_declines_audit.py +++ b/scripts/ci/wire_declines_audit.py @@ -16,9 +16,9 @@ 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_W/E statement stands just above it IN THE SAME BLOCK: a line that BEGINS with the log call, 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, and neither -is a guarded one-liner (`if (g) MGLOG_W(...)`). +indentation, with no line of lesser indentation (an enclosing `if (...) {`, a `} else {`, a +`#else`) and no same-indent `return`/`break`/`case`/... between them. A log in a sibling branch +or a nested one is not this site's line, and neither is a guarded one-liner (`if (g) MGLOG_W`). 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, @@ -58,6 +58,13 @@ RAW_PREFIX = re.compile(r"(?:^|[^A-Za-z0-9_])(?:u8|u|U|L)?R$") # by a digit separator - and read the other way the `"` inside it opens a phantom string that # blanks the rest of the line, sites included. CHAR_PREFIX = re.compile(r"(?:^|[^A-Za-z0-9_])(?:u8|u|U|L)$") +# A statement control cannot flow past. A log ABOVE one of these at the Count's indentation is +# on a path that ends there, so it is not the Count's log. +FLOW_STOP = re.compile(r"^\s*(?:case\b|default\s*:|break\s*;|return\b|continue\s*;|goto\b|throw\b)") +# Conditional-compilation lines the block walk steps over: a log and its Count that straddle a +# `#if MOBILEGL_BUILD_DISAGGREGATED` are in one block on both sides of the preprocessor. `#else` +# and `#elif` are NOT here - a log on the other side of one is in a different build, not this one. +PP_SKIP = re.compile(r"^\s*#\s*(?:if|ifdef|ifndef|endif)\b") def strip_code(text: str) -> str: @@ -123,20 +130,25 @@ def indent_of(line: str) -> int: 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.""" + Walk upwards; blank lines, `#if`/`#ifdef`/`#ifndef`/`#endif` 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 {`, or a `#else`) and ends the walk; so does + a same-indent `case`/`default`/`break`/`return`/`continue`/`goto`/`throw`, past which + control cannot reach the Count.""" 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(): + if not line.strip() or PP_SKIP.match(line): continue ind = indent_of(line) if ind < own: return False - if ind == own and LOGGED.match(line): - return True + if ind == own: + if LOGGED.match(line): + return True + if FLOW_STOP.match(line): + return False return False @@ -202,8 +214,9 @@ def audit(root: str) -> int: # --self-test: each fixture is (name, def rows text, Fixture.cpp text, expected rc, text the # audit must print - so a red is red for the reason the fixture is about). The red ones are -# the holes the B3 fix round closed; the green one proves the audit still accepts the two -# shapes the tree uses. +# the holes the B3 fix rounds closed; the green ones prove the audit still accepts the shapes +# the tree uses, and the two shapes (a prefixed char literal, a `#if` between log and Count) +# that a fail-closed strip or walk would have reddened for no defect. NO_SITE = "has NO SITE" NO_LOG = "with no MGLOG_W/E in its own block" _DEF_GHOST = "MGL_WIRE_DECLINE(GhostRow)\n" @@ -257,6 +270,35 @@ SELF_TEST_FIXTURES = ( " g ? MGLOG_E(\"or only then\") : (void)0;\n" " WireDeclineTally::Count(WireDeclineSite::GhostRow);\n" "}\n", 1, NO_LOG), + ("log on the far side of a same-indent return", _DEF_GHOST, + "bool F(int x) {\n" + " if (x == 1) goto declined;\n" + " MGLOG_W(\"x is not one\");\n" + " return true;\n" + " declined:\n" + " WireDeclineTally::Count(WireDeclineSite::GhostRow);\n" + " return false;\n" + "}\n", 1, NO_LOG), + ("log in the previous case of a same-indent switch", _DEF_GHOST, + "void F(int x) {\n" + " switch (x) {\n" + " case 1:\n" + " MGLOG_W(\"one\");\n" + " break;\n" + " case 2:\n" + " WireDeclineTally::Count(WireDeclineSite::GhostRow);\n" + " break;\n" + " }\n" + "}\n", 1, NO_LOG), + ("log only on the other side of a #else", _DEF_GHOST, + "void F(int x) {\n" + "#if MOBILEGL_BUILD_DISAGGREGATED\n" + " MGLOG_W(\"x\");\n" + "#else\n" + " MGLOG_D(\"compiled away\");\n" + "#endif\n" + " WireDeclineTally::Count(WireDeclineSite::GhostRow);\n" + "}\n", 1, NO_LOG), ("prefixed char literals (L'\"', u8'\"') before a site on the same line", "MGL_WIRE_DECLINE(WideQuote)\nMGL_WIRE_DECLINE(Utf8Quote)\n", "bool F(wchar_t w, char8_t c) {\n" @@ -264,6 +306,15 @@ SELF_TEST_FIXTURES = ( " if (c == u8'\"') MGL_WIRE_DECLINE_AT(Utf8Quote, \"a utf-8 double quote\");\n" " return true;\n" "}\n", 0, "0 unlogged, 0 unknown"), + ("count under its log across a column-0 #if/#endif", _DEF_GHOST, + "void F(int x) {\n" + " if (x) {\n" + " MGLOG_W(\"x\");\n" + "#if MOBILEGL_BUILD_DISAGGREGATED\n" + " WireDeclineTally::Count(WireDeclineSite::GhostRow);\n" + "#endif\n" + " }\n" + "}\n", 0, "0 unlogged, 0 unknown"), ("good sites (AT form, bare Count under its own W/E, def row with trailing comment)", "MGL_WIRE_DECLINE(GoodAt) // the macro form\nMGL_WIRE_DECLINE(GoodBare)\n", "bool F(int x) {\n"