fix(diagnostics): make reserved-type-name binding check exhaustive (issue 0076)

The reserved/builtin-type-name binding diagnostic was a hand-walked subset
of binding-bearing AST nodes with a silent `else => {}`, so each review
found another syntactic binding form that bypassed it and hit the original
LLVM verifier abort: destructure names (`s2, x := …`), `impl` method
params/locals, and `if` / `while` / `for` / match-arm / `catch` / `onfail`
captures.

Rewrite `checkBindingNames` (src/ir/semantic_diagnostics.zig) as an
EXHAUSTIVE `switch` over every `Node.Data` tag with NO `else` arm — a future
binding-bearing node type now fails to compile until it is handled here, so
coverage is enforced by the compiler instead of a hand-maintained list. The
check stays in the pre-lowering semantic pass rather than moving to the
`Scope.put` scope-registration choke point: lowering is lazy, so an
uncalled function's bindings never reach `Scope.put`, yet they must still be
rejected at their declaration (e.g. the never-called `takes_u8` in 1119).
No lowering special-case; `lower.zig` unchanged.

Regression tests (fail-before: LLVM abort or silent accept → pass-after:
clean diagnostic, exit 1):
- 1121 control-flow: destructure, if/while bindings, for capture+index,
  match-arm capture
- 1122 impl-block method: reserved param AND reserved local
- 1123 catch + onfail tag bindings
- 1124 destructure name reserved in an imported module
Existing 0125 / 1119 / 0135 / 1120 tests kept; full suite 368 passed.
This commit is contained in:
agra
2026-06-03 20:09:46 +03:00
parent df6e830bec
commit fcc76b9391
19 changed files with 375 additions and 32 deletions

View File

@@ -0,0 +1,30 @@
// Reserved/builtin type names are rejected as binding NAMES across every
// control-flow and destructuring form, not just plain `var`/param decls: a
// destructure name (`s2`), an `if`/`while` optional binding (`u8`/`s16`), a
// `for` capture and index name (`bool`/`s32`), and a match-arm capture
// (`string`). Each spelling parses as a `.type_expr`, so the address-of family
// in lowering mis-lowers it (a loaded aggregate passed by value to a `ptr`
// param → LLVM verifier abort). The declaration-site diagnostic comes from one
// EXHAUSTIVE binding-name walk, so no syntactic binding form can slip through.
//
// Regression (issue 0076, attempt-4 coverage). Expected: one error per
// offending name; exit 1 — NOT an LLVM verifier abort.
#import "modules/std.sx";
pair :: () -> (s64, s64) { (1, 2) }
maybe :: () -> ?s64 { return null; }
main :: () -> s32 {
s2, rest := pair(); // destructure name
if u8 := maybe() { } // if optional binding
while s16 := maybe() { break; } // while optional binding
xs := [3]s64.{ 10, 20, 30 };
for xs: (bool) { } // for capture name
for xs: (v, s32) { } // for index name
opt: ?s64 = 5;
r := if opt == { // match-arm capture
case .some: (string) { 0 }
case .none: { 0 }
};
return 0;
}

View File

@@ -0,0 +1,30 @@
// A reserved/builtin type name is rejected as a binding name inside an `impl`
// block's method too — both as a parameter (`u8`) and as a local (`s2`). The
// impl method is reached through the exhaustive binding-name walk's
// `impl_block` arm (→ each method's `fn_decl`), so an `impl` method is no more
// exempt than a free function. Without the diagnostic the reserved local's
// `@s2` mis-lowers (a loaded aggregate passed by value to a `*Box` param →
// LLVM verifier abort).
//
// Regression (issue 0076, attempt-4 coverage). Expected: one error for the
// param and one for the local; exit 1.
#import "modules/std.sx";
Box :: struct { total: s64 = 0; count: s64 = 0; }
update :: (self: *Box, n: s64) { self.total += n; self.count += 1; }
Doer :: protocol { go :: (self: *Self, n: s64); }
impl Doer for Box {
go :: (self: *Box, u8: s64) {
s2 := Box.{ total = 1 };
update(@s2, u8);
self.total += s2.total;
}
}
main :: () -> s32 {
b := Box.{};
b.go(7);
return 0;
}

View File

@@ -0,0 +1,28 @@
// A reserved/builtin type name is rejected as the error-tag binding of a
// `catch` (`u8`) and of an `onfail` (`s64`). Both are reached through the
// exhaustive binding-name walk's `catch_expr` / `onfail_stmt` arms. The tag is
// a scalar, so before the diagnostic these spellings were silently accepted
// (they never reached the address-of mis-lowering) — the binding must still be
// rejected at its declaration.
//
// Regression (issue 0076, attempt-4 coverage). Expected: one error for each
// binding; exit 1.
#import "modules/std.sx";
E :: error { Bad }
must :: (n: s32) -> !E {
if n < 0 { raise error.Bad; }
return;
}
classify :: (n: s32) -> !E {
onfail s64 { } // onfail tag binding
must(n) catch u8 { return; }; // catch tag binding
return;
}
main :: () -> s32 {
classify(-1) catch { };
return 0;
}

View File

@@ -0,0 +1,15 @@
// A reserved type name used as a DESTRUCTURE binding name (`s2`) is rejected
// even when it lives in an IMPORTED module — the exhaustive binding-name walk
// descends the `namespace_decl` an `mod :: #import` wraps and renders the
// diagnostic against that module's source (issue 0077's universal-coverage
// rule applied to the destructure form). Without it the binding reaches
// lowering and aborts LLVM verification.
//
// Regression (issues 0076 + 0077, attempt-4 coverage). Expected: one clean
// diagnostic pointing at the imported module's `s2, rest := ...`, exit 1.
#import "modules/std.sx";
mod :: #import "1124-diagnostics-imported-reserved-destructure/mod.sx";
main :: () -> s32 {
return mod.run();
}

View File

@@ -0,0 +1,8 @@
#import "modules/std.sx";
pair :: () -> (s64, s64) { (1, 2) }
run :: () -> s32 {
s2, rest := pair(); // destructure name in an IMPORTED module
return 0;
}

View File

@@ -0,0 +1,41 @@
error: 's2' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:18:5
|
18 | s2, rest := pair(); // destructure name
| ^^^^^^^^^^^^^^^^^^^
error: 'u8' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:19:5
|
19 | if u8 := maybe() { } // if optional binding
| ^^^^^^^^^^^^^^^^^^^^
error: 's16' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:20:5
|
20 | while s16 := maybe() { break; } // while optional binding
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: 'bool' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:22:5
|
22 | for xs: (bool) { } // for capture name
| ^^^^^^^^^^^^^^^^^^
error: 's32' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:23:5
|
23 | for xs: (v, s32) { } // for index name
| ^^^^^^^^^^^^^^^^^^^^
error: 'string' is a reserved type name and cannot be used as an identifier
--> examples/1121-diagnostics-reserved-name-control-flow.sx:25:10
|
25 | r := if opt == { // match-arm capture
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26 | case .some: (string) { 0 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27 | case .none: { 0 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
28 | };
| ^^^^^

View File

@@ -0,0 +1,11 @@
error: 'u8' is a reserved type name and cannot be used as an identifier
--> examples/1122-diagnostics-reserved-name-impl-method.sx:19:24
|
19 | go :: (self: *Box, u8: s64) {
| ^^
error: 's2' is a reserved type name and cannot be used as an identifier
--> examples/1122-diagnostics-reserved-name-impl-method.sx:20:9
|
20 | s2 := Box.{ total = 1 };
| ^^^^^^^^^^^^^^^^^^^^^^^^

View File

@@ -0,0 +1,11 @@
error: 's64' is a reserved type name and cannot be used as an identifier
--> examples/1123-diagnostics-reserved-name-catch-onfail.sx:20:5
|
20 | onfail s64 { } // onfail tag binding
| ^^^^^^^^^^^^^^
error: 'u8' is a reserved type name and cannot be used as an identifier
--> examples/1123-diagnostics-reserved-name-catch-onfail.sx:21:5
|
21 | must(n) catch u8 { return; }; // catch tag binding
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

View File

@@ -0,0 +1,5 @@
error: 's2' is a reserved type name and cannot be used as an identifier
--> examples/1124-diagnostics-imported-reserved-destructure/mod.sx:6:5
|
6 | s2, rest := pair(); // destructure name in an IMPORTED module
| ^^^^^^^^^^^^^^^^^^^

View File

@@ -11,10 +11,9 @@
> `*self`-mutation-losing copy).
>
> **Fix:** a declaration-site diagnostic in the existing semantic pass
> `src/ir/semantic_diagnostics.zig` (`UnknownTypeChecker`). New
> `checkBindingName` rejects any parameter name or `var` binding name (local or
> global, `:=` / typed-local forms) whose spelling collides with a reserved type
> name; `isReservedTypeName` defers to the parser's own classifier
> `src/ir/semantic_diagnostics.zig` (`UnknownTypeChecker`). `checkBindingName`
> rejects any binding name whose spelling collides with a reserved type name;
> `isReservedTypeName` defers to the parser's own classifier
> (`types.Type.fromName`) so the rejected set never drifts from the set that
> would parse as a type — the named builtins (`bool`, `string`, `void`, `f32`,
> `f64`, `usize`, `isize`, `Any`) and `[su]N` over sx's 164 range. Bare value
@@ -22,10 +21,32 @@
> the `.identifier`-only address-of paths are correct once type-shaped names can
> never be bound. The rejected `bareVarName` approach was never landed.
>
> **Coverage is structural (attempt 4).** Earlier landings hand-walked a subset
> of binding-bearing nodes with a silent `else => {}`, so each review found a new
> leaking syntactic form (destructure names, `impl` method params/locals, `if` /
> `while` / `for` / match-arm / `catch` / `onfail` captures) that bypassed the
> check and hit the original LLVM verifier abort. `checkBindingNames` is now an
> **exhaustive `switch` over every `Node.Data` tag with NO `else` arm**: a future
> binding-bearing node type fails to compile until it is handled here, so
> coverage is enforced by the compiler rather than by a hand-maintained list. The
> check stays in the pre-lowering semantic pass (NOT moved to the `Scope.put`
> scope-registration choke point) because lowering is lazy — an UNCALLED
> function's bindings never reach `Scope.put`, yet they must still be rejected at
> their declaration (e.g. `examples/1119`'s never-called `takes_u8`).
>
> **Regression tests:**
> - `examples/0125-types-type-named-var-rejected.sx` — `:=` form (`s2`) rejected.
> - `examples/1119-diagnostics-reserved-type-name-as-identifier.sx` — parameter
> (`u8`), typed-local (`s64`, `bool`), and `:=` (`string`) forms rejected.
> - `examples/1121-diagnostics-reserved-name-control-flow.sx` — destructure name,
> `if` / `while` optional bindings, `for` capture + index names, match-arm
> capture.
> - `examples/1122-diagnostics-reserved-name-impl-method.sx` — `impl`-block method
> reserved param AND reserved local.
> - `examples/1123-diagnostics-reserved-name-catch-onfail.sx` — `catch` and
> `onfail` error-tag bindings.
> - `examples/1124-diagnostics-imported-reserved-destructure.sx` — destructure
> name reserved in an IMPORTED module (renders against that module's source).
> - `examples/0135-types-self-streaming-nonreserved.sx` — positive: `*self`
> streaming with non-reserved names (`hasher`, `ctx`) accumulates correctly via
> both `update(@h, …)` and `h.update(…)`.

View File

@@ -84,91 +84,176 @@ pub const UnknownTypeChecker = struct {
}
}
/// Reserved-type-name binding walk (issues 0076, 0077). Visits every binding
/// site reachable from `node` — `var` / `:=` / typed-local declarations and
/// function / lambda / struct-method parameters, at any nesting depth — and
/// rejects each name that collides with a reserved/builtin type name. Walks
/// into expressions too, so a lambda nested in a call arg / struct literal is
/// reached. Deliberately filter-free (every module is walked) and context-
/// free (spelling is the sole criterion), distinct from the main-file-scoped
/// unknown-type walk. A node carrying its own `source_file` (every module's
/// top-level decls do) becomes the emit file for its whole subtree, restored
/// on exit so a sibling in another module isn't rendered against it.
/// Reserved-type-name binding walk (issues 0076, 0077). Visits every node
/// reachable from `node` and rejects each *binding name* — `var` / `:=` /
/// typed-local declarations, destructure names, function / lambda / method
/// parameters, `if` / `while` optional bindings, `for` capture + index
/// names, match-arm captures, and `catch` / `onfail` tag bindings — whose
/// spelling collides with a reserved/builtin type name. Such a spelling
/// parses as a `.type_expr`, so the address-of family in `lower.zig` never
/// sees the scoped local and mis-lowers it (a loaded aggregate passed
/// by value to a `ptr` param → LLVM verifier abort, or a silent
/// mutation-losing copy). Rejecting the name here, before lowering, keeps
/// the `.identifier`-only address-of paths correct with no lowering
/// special-case.
///
/// The `switch` is EXHAUSTIVE — every `Node.Data` tag is listed and there
/// is NO `else` arm. A future binding-bearing node type therefore fails to
/// compile here until it is handled, so coverage is enforced by the
/// compiler rather than by remembering to extend a hand-maintained list.
/// (The check can't live at the scope-registration choke point in
/// `lower.zig`: lowering is lazy, so an UNCALLED function's bindings never
/// reach `Scope.put` — yet they must still be rejected at their
/// declaration.) Deliberately filter-free (every compiled module is walked)
/// and context-free (spelling is the sole criterion), distinct from the
/// main-file-scoped unknown-type walk. A node carrying its own
/// `source_file` (every module's top-level decls do) becomes the emit file
/// for its whole subtree, restored on exit so a sibling in another module
/// isn't rendered against it (issue 0077).
fn checkBindingNames(self: UnknownTypeChecker, node: *const Node) void {
const saved_file = self.diagnostics.current_source_file;
defer self.diagnostics.current_source_file = saved_file;
if (node.source_file) |sf| self.diagnostics.current_source_file = sf;
switch (node.data) {
// ── Binding-introducing nodes: check the name(s), then recurse. ──
.var_decl => |vd| {
self.checkBindingName(vd.name, node.span);
if (vd.value) |v| self.checkBindingNames(v);
},
.destructure_decl => |dd| {
for (dd.names) |n| self.checkBindingName(n, node.span);
self.checkBindingNames(dd.value);
},
.fn_decl => |fd| {
for (fd.params) |p| self.checkBindingName(p.name, p.name_span);
self.checkParamNames(fd.params);
self.checkBindingNames(fd.body);
},
.lambda => |lm| {
for (lm.params) |p| self.checkBindingName(p.name, p.name_span);
self.checkParamNames(lm.params);
self.checkBindingNames(lm.body);
},
.const_decl => |cd| self.checkBindingNames(cd.value),
// A namespaced import (`mod :: #import "..."`) is wrapped here, its
// module decls held inline. Descend so an imported module's
// reserved-name binding is rejected too (issue 0077).
.namespace_decl => |nd| for (nd.decls) |d| self.checkBindingNames(d),
.struct_decl => |sd| for (sd.methods) |m| self.checkBindingNames(m),
.block => |b| for (b.stmts) |s| self.checkBindingNames(s),
.param => |p| {
self.checkBindingName(p.name, p.name_span);
if (p.default_expr) |de| self.checkBindingNames(de);
},
.if_expr => |ie| {
if (ie.binding_name) |bn| self.checkBindingName(bn, node.span);
self.checkBindingNames(ie.condition);
self.checkBindingNames(ie.then_branch);
if (ie.else_branch) |e| self.checkBindingNames(e);
},
.while_expr => |we| {
if (we.binding_name) |bn| self.checkBindingName(bn, node.span);
self.checkBindingNames(we.condition);
self.checkBindingNames(we.body);
},
.for_expr => |fe| {
if (fe.capture_name.len != 0) self.checkBindingName(fe.capture_name, node.span);
if (fe.index_name) |idx| self.checkBindingName(idx, node.span);
self.checkBindingNames(fe.iterable);
if (fe.range_end) |re| self.checkBindingNames(re);
self.checkBindingNames(fe.body);
},
.match_expr => |me| {
self.checkBindingNames(me.subject);
for (me.arms) |arm| self.checkBindingNames(arm.body);
for (me.arms) |arm| {
if (arm.capture) |cap| self.checkBindingName(cap, node.span);
if (arm.pattern) |p| self.checkBindingNames(p);
self.checkBindingNames(arm.body);
}
},
.match_arm => |arm| {
if (arm.capture) |cap| self.checkBindingName(cap, node.span);
if (arm.pattern) |p| self.checkBindingNames(p);
self.checkBindingNames(arm.body);
},
.catch_expr => |ce| {
if (ce.binding) |b| self.checkBindingName(b, node.span);
self.checkBindingNames(ce.operand);
self.checkBindingNames(ce.body);
},
.onfail_stmt => |os| {
if (os.binding) |b| self.checkBindingName(b, node.span);
self.checkBindingNames(os.body);
},
// impl / protocol-default / foreign-class method bodies: each
// method introduces its own params + locals. A `#jni_main` /
// `#objc_class` bodied method is lowered (M1.2), so its reserved
// param/local names mis-lower the same as any other.
.impl_block => |ib| for (ib.methods) |m| self.checkBindingNames(m),
.protocol_decl => |pd| for (pd.methods) |m| {
if (m.default_body) |body| {
for (m.param_names) |pn| self.checkBindingName(pn, node.span);
self.checkBindingNames(body);
}
},
.foreign_class_decl => |fcd| for (fcd.members) |member| switch (member) {
.method => |m| if (m.body) |body| {
for (m.param_names) |pn| self.checkBindingName(pn, node.span);
self.checkBindingNames(body);
},
.field, .extends, .implements => {},
},
// ── Container / control-flow / expression nodes: recurse children
// so a binding nested anywhere below is still reached. ──
// A namespaced import (`mod :: #import "..."`) is wrapped here, its
// module decls held inline; descend so an imported module's
// reserved-name binding is rejected too (issue 0077).
.namespace_decl => |nd| for (nd.decls) |d| self.checkBindingNames(d),
.const_decl => |cd| self.checkBindingNames(cd.value),
.struct_decl => |sd| {
for (sd.methods) |m| self.checkBindingNames(m);
for (sd.constants) |c| self.checkBindingNames(c);
for (sd.field_defaults) |fdef| if (fdef) |d| self.checkBindingNames(d);
},
.root => |r| for (r.decls) |d| self.checkBindingNames(d),
.block => |b| for (b.stmts) |s| self.checkBindingNames(s),
.push_stmt => |ps| {
self.checkBindingNames(ps.context_expr);
self.checkBindingNames(ps.body);
},
.jni_env_block => |jb| {
self.checkBindingNames(jb.env);
self.checkBindingNames(jb.body);
},
.defer_stmt => |ds| self.checkBindingNames(ds.expr),
.onfail_stmt => |os| self.checkBindingNames(os.body),
.return_stmt => |r| if (r.value) |v| self.checkBindingNames(v),
.raise_stmt => |rs| self.checkBindingNames(rs.tag),
.assignment => |a| {
self.checkBindingNames(a.value);
self.checkBindingNames(a.target);
},
.multi_assign => |ma| for (ma.values) |v| self.checkBindingNames(v),
.destructure_decl => |dd| self.checkBindingNames(dd.value),
.multi_assign => |ma| {
for (ma.targets) |t| self.checkBindingNames(t);
for (ma.values) |v| self.checkBindingNames(v);
},
.call => |c| {
self.checkBindingNames(c.callee);
for (c.args) |a| self.checkBindingNames(a);
},
.ffi_intrinsic_call => |fic| for (fic.args) |a| self.checkBindingNames(a),
.binary_op => |b| {
self.checkBindingNames(b.lhs);
self.checkBindingNames(b.rhs);
},
.chained_comparison => |cc| for (cc.operands) |o| self.checkBindingNames(o),
.unary_op => |u| self.checkBindingNames(u.operand),
.field_access => |fa| self.checkBindingNames(fa.object),
.index_expr => |ix| {
self.checkBindingNames(ix.object);
self.checkBindingNames(ix.index);
},
.slice_expr => |sx| {
self.checkBindingNames(sx.object);
if (sx.start) |s| self.checkBindingNames(s);
if (sx.end) |e| self.checkBindingNames(e);
},
.struct_literal => |sl| {
for (sl.field_inits) |fi| self.checkBindingNames(fi.value);
if (sl.init_block) |ib| self.checkBindingNames(ib);
},
.array_literal => |al| for (al.elements) |e| self.checkBindingNames(e),
.tuple_literal => |tl| for (tl.elements) |e| self.checkBindingNames(e.value),
.force_unwrap => |fu| self.checkBindingNames(fu.operand),
.null_coalesce => |nc| {
self.checkBindingNames(nc.lhs);
@@ -176,13 +261,63 @@ pub const UnknownTypeChecker = struct {
},
.deref_expr => |de| self.checkBindingNames(de.operand),
.try_expr => |te| self.checkBindingNames(te.operand),
.catch_expr => |ce| {
self.checkBindingNames(ce.operand);
self.checkBindingNames(ce.body);
},
.comptime_expr => |ce| self.checkBindingNames(ce.expr),
.insert_expr => |ins| self.checkBindingNames(ins.expr),
.spread_expr => |se| self.checkBindingNames(se.operand),
else => {},
// ── Leaves & pure type-expression nodes: no binding sites below. ──
// Type-expression subtrees carry only type names (no value
// bindings); enum / union / error-set declarations carry only field
// types + comptime constants. Listing each tag explicitly (rather
// than an `else`) is what forces a future binding-bearing node to be
// reconsidered here.
.int_literal,
.float_literal,
.bool_literal,
.string_literal,
.identifier,
.enum_literal,
.type_expr,
.enum_decl,
.union_decl,
.error_set_decl,
.import_decl,
.array_type_expr,
.slice_type_expr,
.parameterized_type_expr,
.pointer_type_expr,
.many_pointer_type_expr,
.optional_type_expr,
.error_type_expr,
.caller_location,
.pack_index_type_expr,
.comptime_pack_ref,
.null_literal,
.break_expr,
.continue_expr,
.undef_literal,
.inferred_type,
.builtin_expr,
.compiler_expr,
.foreign_expr,
.library_decl,
.framework_decl,
.function_type_expr,
.closure_type_expr,
.tuple_type_expr,
.ufcs_alias,
.c_import_decl,
=> {},
}
}
/// Check each parameter's binding name (`fn` / lambda params are stored as
/// `Param` values, not child nodes, so they're walked here rather than via
/// the node `switch`). A param default expression can itself nest bindings
/// (a lambda default), so recurse into it.
fn checkParamNames(self: UnknownTypeChecker, params: []const ast.Param) void {
for (params) |p| {
self.checkBindingName(p.name, p.name_span);
if (p.default_expr) |de| self.checkBindingNames(de);
}
}