From 91c13f85def5b3de8df7296c2ca8d1d9232b1d24 Mon Sep 17 00:00:00 2001 From: lab Date: Tue, 8 Sep 2026 18:46:23 +0700 Subject: [PATCH] check: a collision that only lowering creates Closes #3485 `fn test_booth_encode_zero` and `test booth_encode_zero` are DIFFERENT NAMESPACES IN t27 and the SAME IDENTIFIER IN C, where a test block becomes `void test_{name}(void)`. The duplicate gate compares raw names, so it was silent while the corpus printed `redefinition of 'test_booth_encode_zero'`. MEASURED PER BACKEND, NOT ASSUMED, and the message names only the backends it is true of: gen-c `void test_X(void)` / `void bench_X(void)` COLLIDES on both gen (zig) `test "X"` is a STRING, not an identifier no collision `fn bench_X()` COLLIDES gen-rust tests are not lowered at all no collision gen-verilog a test is emitted as a comment no collision Repeating the existing "every backend rejects a redeclaration" would have been false for three of the four. POPULATION: functions named `test_*` 66 ... with a test block of the matching name 4 in 2 specs functions named `bench_*` 0 test blocks named `test_*` 532 ... whose stripped name is also a test 0 The 4 split, and neither half is repaired here. The two in igla/race/backend.t27 are NEVER CALLED -- each appears once in the whole tree, as its own declaration -- while the two in math/property_test_template.t27 ARE called from test blocks. One pair is dead code that also collides; the other is a live helper that needs one of two names changed. Both are a spec author's decision. THE `bench` HALF HAS POPULATION ZERO AND IS KEPT ANYWAY, written, tested and shown reachable rather than left out -- the same reasoning already recorded in this function for the type/function finding: a zero nothing could have produced is not evidence. THE RULE KEYS ON `fn`, AND 532 TEST BLOCKS NAMED `test_*` ARE WHY THAT MATTERS. `test "test_thing"` beside `test "thing"` gives `void test_test_thing(void)` and `void test_thing(void)` -- two identifiers, no collision. A mutant letting any declaration kind trigger the rule survived every test until a fixture for that case existed. It gets ITS OWN RATCHET rather than a number shared with the existing one: they are different claims, and a shared count lets one kind be paid for with the other. The Spec Guards `paths:` filter is extended to the new baseline, which otherwise would not have triggered the workflow that reads it. The corpus is unchanged -- 11 642 before and after, all 582 generated headers identical. This is a `check` change. Tests: 4 new Rust cases, 3 new self-check cases (8 of 8 pass), full suite 3537 passed 0 failed. Five mutants, all killed. Co-Authored-By: Claude Opus 5 --- .github/workflows/spec-guards.yml | 2 + bootstrap/src/compiler.rs | 47 +++++++++ bootstrap/stage0/FROZEN_HASH | 2 +- bootstrap/tests/duplicate_declarations.rs | 83 ++++++++++++++++ ...-a-collision-that-only-lowering-creates.md | 11 +++ tools/check_duplicate_declarations.py | 99 +++++++++++++++++-- tools/lowered_collisions_baseline.txt | 16 +++ 7 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 docs/now/2026-09-08-a-collision-that-only-lowering-creates.md create mode 100644 tools/lowered_collisions_baseline.txt diff --git a/.github/workflows/spec-guards.yml b/.github/workflows/spec-guards.yml index 845d2159f..94fba32ba 100644 --- a/.github/workflows/spec-guards.yml +++ b/.github/workflows/spec-guards.yml @@ -52,6 +52,7 @@ on: - 'tools/ring_spec_differential.py' - 'tools/check_duplicate_declarations.py' - 'tools/duplicate_declarations_baseline.txt' + - 'tools/lowered_collisions_baseline.txt' - 'tools/diagnostic_capacity.py' - '.github/workflows/spec-guards.yml' push: @@ -66,6 +67,7 @@ on: - 'tools/ring_spec_differential.py' - 'tools/check_duplicate_declarations.py' - 'tools/duplicate_declarations_baseline.txt' + - 'tools/lowered_collisions_baseline.txt' - 'tools/diagnostic_capacity.py' - '.github/workflows/spec-guards.yml' workflow_dispatch: diff --git a/bootstrap/src/compiler.rs b/bootstrap/src/compiler.rs index 328efea80..600153721 100644 --- a/bootstrap/src/compiler.rs +++ b/bootstrap/src/compiler.rs @@ -23906,9 +23906,48 @@ fn duplicate_top_level_decls(ast: &Node) -> Vec { namespaces.entry(n.as_str()).or_default().insert(ns(k)); } + // A collision that exists only AFTER lowering. `fn test_booth_encode_zero` + // and `test booth_encode_zero` are different namespaces in t27 and the same + // identifier in C, where a test block becomes `void test_{name}(void)`. + // Measured per backend rather than assumed: + // gen-c `void test_X(void)` / `void bench_X(void)` -- COLLIDES + // gen (zig) `test "X"` is a STRING, no identifier -- no collision + // `fn bench_X()` -- COLLIDES + // gen-rust tests are not lowered at all -- no collision + // gen-verilog a test is emitted as a comment -- no collision + // Corpus: 66 functions are named `test_*` and 4 of them meet a test block + // of the matching name; 0 functions are named `bench_*`, so that half is + // stated and tested rather than left out -- a zero nothing could have + // produced is not evidence. + let blocks = |want: &str| { + decls + .iter() + .filter(|(_, k)| *k == want) + .map(|(n, _)| n.as_str()) + .collect::>() + }; + let tests = blocks("test"); + let benches = blocks("bench"); + let mut seen = std::collections::HashSet::new(); let mut out = Vec::new(); for (n, k) in &decls { + if *k == "fn" { + if let Some(rest) = n.strip_prefix("test_") { + if tests.contains(rest) { + out.push(DuplicateDecl::LoweredCollision(n.clone(), "test", "gen-c")); + } + } + if let Some(rest) = n.strip_prefix("bench_") { + if benches.contains(rest) { + out.push(DuplicateDecl::LoweredCollision( + n.clone(), + "bench", + "gen-c and gen (zig)", + )); + } + } + } if !seen.insert(n.clone()) { continue; } @@ -23932,6 +23971,10 @@ enum DuplicateDecl { SameNamespace(String, &'static str, usize), /// A type and a function sharing a name: legal in Rust and C, not in Zig. AcrossNamespaces(String), + /// Two declarations that do NOT collide in t27 and DO collide once a + /// backend has added its prefix: `fn test_x` beside `test x`. + /// (function name, block kind, the backends that collide) + LoweredCollision(String, &'static str, &'static str), } fn collect_declared_types(node: &Node, out: &mut std::collections::HashSet) { @@ -24094,6 +24137,10 @@ drop the parameter from the declaration and keep it at each use, where it is und DuplicateDecl::AcrossNamespaces(name) => format!( "warning: `{name}` is declared as both a type and a function -- zig rejects this; rust and C do not" ), + DuplicateDecl::LoweredCollision(name, block, backends) => format!( + "warning: `fn {name}` and `{block} {}` are different names in t27 and the SAME identifier in {backends}, which emits `{name}` for the {block} block", + name.trim_start_matches(if block == "test" { "test_" } else { "bench_" }) + ), }; result.errors.push(msg); result.warnings += 1; diff --git a/bootstrap/stage0/FROZEN_HASH b/bootstrap/stage0/FROZEN_HASH index 2add220e6..2bdc1da41 100644 --- a/bootstrap/stage0/FROZEN_HASH +++ b/bootstrap/stage0/FROZEN_HASH @@ -1 +1 @@ -d0bfdbd5a5fcdf244aef304291cac652e995e1ffed655469e3c18b3495a328be bootstrap/src/compiler.rs +d5f667361f3d536baf954d1886e4205da03f0857c06df60565bbf31626501520 bootstrap/src/compiler.rs diff --git a/bootstrap/tests/duplicate_declarations.rs b/bootstrap/tests/duplicate_declarations.rs index e3dae93da..a2b24cd47 100644 --- a/bootstrap/tests/duplicate_declarations.rs +++ b/bootstrap/tests/duplicate_declarations.rs @@ -208,3 +208,86 @@ fn a_test_and_a_function_of_one_name_is_not_the_type_function_pair() { "a test is not a function for this purpose:\n{out}" ); } + +// --------------------------------------------------------------------------- +// A collision that exists only AFTER lowering (#3485) +// +// `fn test_booth_encode_zero` and `test booth_encode_zero` are different +// namespaces in t27 and the SAME identifier in C, where a test block becomes +// `void test_{name}(void)`. Measured per backend rather than assumed: +// +// gen-c `void test_X(void)` / `void bench_X(void)` -- COLLIDES +// gen (zig) `test "X"` is a STRING, no identifier -- no collision +// `fn bench_X()` -- COLLIDES +// gen-rust tests are not lowered at all -- no collision +// gen-verilog a test is emitted as a comment -- no collision +// +// 66 functions in the corpus are named `test_*` and 4 meet a test block of the +// matching name; 0 are named `bench_*`, so that half is proved reachable by a +// test rather than left out. +// --------------------------------------------------------------------------- + +#[test] +fn a_function_named_after_a_test_block_collides_in_c() { + let (_, out) = check( + "module L1 {\n fn test_thing(v: i32) -> i32 { return v; }\n test \"thing\" { assert(1 == 1); }\n}\n", + "lowered", + ); + assert!( + out.contains("SAME identifier in gen-c"), + "the collision must be reported, and named as gen-c's:\n{out}" + ); + assert!( + !out.contains("every backend rejects"), + "and it must NOT claim every backend rejects it -- zig and rust do not:\n{out}" + ); +} + +#[test] +fn the_bench_half_is_reachable_and_names_two_backends() { + // Population zero in the corpus. Kept and tested rather than left out: a + // zero nothing could have produced is not evidence. gen-c writes + // `void bench_X(void)` and zig writes `fn bench_X()`, so this one really + // does collide twice. + let (_, out) = check( + "module L2 {\n fn bench_thing(v: i32) -> i32 { return v; }\n bench \"thing\" { assert(1 == 1); }\n}\n", + "loweredbench", + ); + assert!( + out.contains("SAME identifier in gen-c and gen (zig)"), + "the bench collision must name both backends:\n{out}" + ); +} + +#[test] +fn a_function_named_test_something_with_no_such_test_is_quiet() { + // THE DISCRIMINATING CASE. 62 of the corpus's 66 `test_*` functions have no + // test block of the matching name, and a rule keyed on the prefix alone + // would report every one of them. + let (_, out) = check( + "module L3 {\n fn test_thing(v: i32) -> i32 { return v; }\n test \"other\" { assert(1 == 1); }\n}\n", + "nolowered", + ); + assert!( + !out.contains("SAME identifier"), + "a `test_*` name with no matching block is not a collision:\n{out}" + ); +} + +#[test] +fn a_test_block_named_test_something_is_not_a_collision() { + // The rule keys on `fn`, and that is load-bearing rather than incidental. + // `test "test_thing"` and `test "thing"` become `void test_test_thing(void)` + // and `void test_thing(void)` -- two different identifiers. A mutant that + // lets any declaration kind trigger the rule reports them as a collision + // and passes every other test here, because no other fixture has a test + // block whose own name starts with `test_`. + let (_, out) = check( + "module L4 {\n test \"test_thing\" { assert(1 == 1); }\n test \"thing\" { assert(2 == 2); }\n}\n", + "testprefixed", + ); + assert!( + !out.contains("SAME identifier"), + "two test blocks get two different C names:\n{out}" + ); +} diff --git a/docs/now/2026-09-08-a-collision-that-only-lowering-creates.md b/docs/now/2026-09-08-a-collision-that-only-lowering-creates.md new file mode 100644 index 000000000..4e6f4a435 --- /dev/null +++ b/docs/now/2026-09-08-a-collision-that-only-lowering-creates.md @@ -0,0 +1,11 @@ +# NOW -- A collision that only lowering creates (2026-09-08) + +## A collision that only lowering creates (Closes #3485) + +- `fn test_booth_encode_zero` and `test booth_encode_zero` are **different namespaces in t27** and the **same identifier in C**, where a test block becomes `void test_{name}(void)`. The duplicate gate compares raw names, so it was silent while the corpus said `redefinition of 'test_booth_encode_zero'`. +- **Measured per backend rather than assumed**, and the message names only the backends it is true of: `gen-c` emits `void test_X(void)` and `void bench_X(void)` and collides on both; **Zig spells a test name as a STRING** (`test "X"`) and cannot collide there, but does emit `fn bench_X()` and collides on that; `gen-rust` does not lower tests at all; `gen-verilog` emits them as comments. Repeating the existing "every backend rejects a redeclaration" would have been false for three of the four. +- Population: **66 functions named `test_*`, of which 4 meet a test block of the matching name**, in 2 specs. They split -- the two in `igla/race/backend.t27` are **never called** (each appears once in the whole tree, as its own declaration), while the two in `math/property_test_template.t27` **are** called from test blocks. One pair is dead code that also collides; the other is a live helper needing one of two names changed. Neither is repaired here. +- **0 functions are named `bench_*`**, and that half of the rule is written, tested and shown reachable rather than left out -- the same reasoning already recorded for the type/function finding: a zero nothing could have produced is not evidence. +- **The rule keys on `fn`, and 532 test blocks named `test_*` are why that matters.** `test "test_thing"` beside `test "thing"` gives `void test_test_thing(void)` and `void test_thing(void)` -- two identifiers, no collision. A mutant letting any declaration kind trigger the rule survived every test until a fixture for that case existed. +- It gets **its own ratchet** rather than a number shared with the existing one: they are different claims, and a shared count lets one kind be paid for with the other. And the Spec Guards `paths:` filter was extended to the new baseline, which otherwise would not have triggered the workflow that reads it. +- The corpus is unchanged: 11 642 before and after, all 582 headers identical. This is a `check` change. diff --git a/tools/check_duplicate_declarations.py b/tools/check_duplicate_declarations.py index dcd05f99d..264c79cb1 100755 --- a/tools/check_duplicate_declarations.py +++ b/tools/check_duplicate_declarations.py @@ -42,8 +42,16 @@ ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) BASELINE = os.path.join(ROOT, "tools", "duplicate_declarations_baseline.txt") +LOWERED_BASELINE = os.path.join(ROOT, "tools", "lowered_collisions_baseline.txt") STRONG = "every backend rejects a redeclaration" WEAK = "both a type and a function" +# A third finding, and a different claim again: two names that do NOT collide in +# t27 and DO collide once a backend adds its prefix. `fn test_x` beside +# `test x` is one identifier in gen-c, which emits `void test_x(void)` for the +# block. Measured per backend: zig spells a test name as a STRING and rust does +# not lower tests at all, so this one is gen-c's alone -- except for `bench`, +# which zig also lowers to `fn bench_X()`. +LOWERED = "SAME identifier in" def t27c() -> str: @@ -62,12 +70,13 @@ def t27c() -> str: def findings(binary: str, spec: str): - """(strong, weak) counts for one spec.""" + """(strong, weak, lowered) counts for one spec.""" r = subprocess.run([binary, "check", spec], capture_output=True, text=True) text = r.stdout + r.stderr return ( sum(1 for ln in text.splitlines() if STRONG in ln), sum(1 for ln in text.splitlines() if WEAK in ln), + sum(1 for ln in text.splitlines() if LOWERED in ln), ) @@ -95,6 +104,21 @@ def read_baseline(): return base +def read_lowered_baseline(): + if not os.path.exists(LOWERED_BASELINE): + print(f"check_duplicate_declarations: no baseline at {LOWERED_BASELINE}. Exit 2.", + file=sys.stderr) + sys.exit(2) + base = {} + for line in open(LOWERED_BASELINE, encoding="utf-8"): + line = line.split("#", 1)[0].strip() + if not line: + continue + path, n = line.rsplit(None, 1) + base[path] = int(n) + return base + + def self_check(binary: str) -> int: """The gate must fire on a planted duplicate and stay quiet on a clean spec. @@ -106,7 +130,7 @@ def self_check(binary: str) -> int: with open(dup, "w", encoding="utf-8") as fh: fh.write("module SC1 {\n struct A { x : i32, }\n struct A { y : i32, }\n" " fn f(a: A) -> i32 { return a.x; }\n}\n") - s, _ = findings(binary, dup) + s, _w, _lo = findings(binary, dup) print(f" planted duplicate -> strong findings {s} (want >= 1) " f"{'PASS' if s >= 1 else 'FAIL'}") ok &= s >= 1 @@ -115,7 +139,7 @@ def self_check(binary: str) -> int: with open(clean, "w", encoding="utf-8") as fh: fh.write("module SC2 {\n struct C { x : i32, }\n" " fn g(c: C) -> i32 { return c.x; }\n}\n") - s, w = findings(binary, clean) + s, w, _lo = findings(binary, clean) print(f" clean spec -> strong {s}, weak {w} (want 0, 0) " f"{'PASS' if s == 0 and w == 0 else 'FAIL'}") ok &= s == 0 and w == 0 @@ -124,7 +148,7 @@ def self_check(binary: str) -> int: with open(cross, "w", encoding="utf-8") as fh: fh.write("module SC3 {\n struct B { x : i32, }\n" " fn B(v: i32) -> i32 { return v; }\n}\n") - s, w = findings(binary, cross) + s, w, _lo = findings(binary, cross) print(f" type and function -> strong {s}, weak {w} (want 0, 1) " f"{'PASS' if s == 0 and w == 1 else 'FAIL'}") ok &= s == 0 and w == 1 @@ -135,7 +159,7 @@ def self_check(binary: str) -> int: with open(duptest, "w", encoding="utf-8") as fh: fh.write('module SC4 {\n test "same" { assert(1 == 1); }\n' ' test "same" { assert(2 == 2); }\n}\n') - s, w = findings(binary, duptest) + s, w, _lo = findings(binary, duptest) print(f" duplicated test name -> strong {s}, weak {w} (want >= 1, 0) " f"{'PASS' if s >= 1 and w == 0 else 'FAIL'}") ok &= s >= 1 and w == 0 @@ -148,10 +172,46 @@ def self_check(binary: str) -> int: fh.write('module SC5 {\n struct deque_clear { x : i32, }\n' ' fn f(a: deque_clear) -> i32 { return a.x; }\n' ' test "deque_clear" { assert(1 == 1); }\n}\n') - s, w = findings(binary, shared) + s, w, _lo = findings(binary, shared) print(f" test name = struct name-> strong {s}, weak {w} (want 0, 0) " f"{'PASS' if s == 0 and w == 0 else 'FAIL'}") ok &= s == 0 and w == 0 + + # The third finding: names that collide only after a backend adds its + # prefix. gen-c emits `void test_x(void)` for `test x`, so a function + # already called `test_x` is a redefinition there and nowhere else. + low = os.path.join(d, "lowered.t27") + with open(low, "w", encoding="utf-8") as fh: + fh.write('module SC6 {\n fn test_thing(v: i32) -> i32 { return v; }\n' + ' test "thing" { assert(1 == 1); }\n}\n') + s, w, lo = findings(binary, low) + print(f" fn test_x beside test x-> strong {s}, weak {w}, lowered {lo} " + f"(want 0, 0, 1) {'PASS' if s == 0 and w == 0 and lo == 1 else 'FAIL'}") + ok &= s == 0 and w == 0 and lo == 1 + + # The `bench` half has a population of ZERO in the corpus -- 0 functions + # are named `bench_*` -- so it is proved reachable here instead of being + # left out. A zero nothing could have produced is not evidence. + lowb = os.path.join(d, "loweredb.t27") + with open(lowb, "w", encoding="utf-8") as fh: + fh.write('module SC7 {\n fn bench_thing(v: i32) -> i32 { return v; }\n' + ' bench "thing" { assert(1 == 1); }\n}\n') + s, w, lo = findings(binary, lowb) + print(f" fn bench_x beside bench-> strong {s}, weak {w}, lowered {lo} " + f"(want 0, 0, 1) {'PASS' if s == 0 and w == 0 and lo == 1 else 'FAIL'}") + ok &= s == 0 and w == 0 and lo == 1 + + # And the case that keeps THIS population honest: a function named + # `test_*` with no test block of the matching name is 62 of the corpus's + # 66 and must stay quiet. + nolow = os.path.join(d, "nolowered.t27") + with open(nolow, "w", encoding="utf-8") as fh: + fh.write('module SC8 {\n fn test_thing(v: i32) -> i32 { return v; }\n' + ' test "other" { assert(1 == 1); }\n}\n') + s, w, lo = findings(binary, nolow) + print(f" fn test_x, no `test x` -> strong {s}, weak {w}, lowered {lo} " + f"(want 0, 0, 0) {'PASS' if s == 0 and w == 0 and lo == 0 else 'FAIL'}") + ok &= s == 0 and w == 0 and lo == 0 return 0 if ok else 1 @@ -165,14 +225,16 @@ def main() -> int: print("check_duplicate_declarations: no specs found. Exit 2.", file=sys.stderr) return 2 - strong, weak = {}, {} + strong, weak, lowered = {}, {}, {} for p in all_specs: - s, w = findings(binary, p) + s, w, lo = findings(binary, p) rel = os.path.relpath(p, ROOT) if s: strong[rel] = s if w: weak[rel] = w + if lo: + lowered[rel] = lo print(f"specs scanned: {len(all_specs)}") print(f"specs declaring a name twice in one namespace: {len(strong)} " @@ -183,12 +245,33 @@ def main() -> int: f"({sum(weak.values())} names) -- zig rejects these; rust and C do not") for p in sorted(weak): print(f" {weak[p]:3} {p}") + print(f"specs whose names collide only AFTER lowering: {len(lowered)} " + f"({sum(lowered.values())} names) -- `fn test_x` beside `test x`") + for p in sorted(lowered): + print(f" {lowered[p]:3} {p}") if "--list" in sys.argv: return 0 + # The third finding gets a ratchet of its own rather than being folded into + # the first: it is a different claim (gen-c only, and gen-zig for `bench`), + # and a shared number would let one kind be paid for with the other. base = read_baseline() + lbase = read_lowered_baseline() bad = False + for p, n in sorted(lowered.items()): + allowed = lbase.get(p) + if allowed is None: + print(f"NEW: {p} has {n} name(s) that collide after lowering " + f"and is not in {os.path.basename(LOWERED_BASELINE)}") + bad = True + elif n > allowed: + print(f"GREW (lowered): {p} {allowed} -> {n}") + bad = True + for p, allowed in sorted(lbase.items()): + now = lowered.get(p, 0) + if now < allowed: + print(f"progress (lowered): {p} {allowed} -> {now} -- lower the baseline") for p, n in sorted(strong.items()): allowed = base.get(p) if allowed is None: diff --git a/tools/lowered_collisions_baseline.txt b/tools/lowered_collisions_baseline.txt new file mode 100644 index 000000000..89042a502 --- /dev/null +++ b/tools/lowered_collisions_baseline.txt @@ -0,0 +1,16 @@ +# Names that do NOT collide in t27 and DO collide once a backend adds its +# prefix, per file. `fn test_x` beside `test x` is one identifier in gen-c, +# which emits `void test_x(void)` for the block. See #3485. +# +# Measured per backend, not assumed: +# gen-c `void test_X(void)` / `void bench_X(void)` -- COLLIDES +# gen (zig) `test "X"` is a STRING, no identifier -- no collision +# `fn bench_X()` -- COLLIDES +# gen-rust tests are not lowered at all -- no collision +# gen-verilog a test is emitted as a comment -- no collision +# +# A ratchet: a file may only shrink. 66 functions in the corpus are named +# `test_*` and 4 of them meet a test block of the matching name; 0 are named +# `bench_*`, so that half of the rule has a test rather than a population. +specs/igla/race/backend.t27 2 +specs/math/property_test_template.t27 2