diff --git a/examples/1120-diagnostics-imported-reserved-type-name.sx b/examples/1120-diagnostics-imported-reserved-type-name.sx new file mode 100644 index 0000000..e8b5fe9 --- /dev/null +++ b/examples/1120-diagnostics-imported-reserved-type-name.sx @@ -0,0 +1,16 @@ +// A value binding spelled as a reserved type name (`s2`, the `sN` arbitrary- +// width int syntax) is rejected at its declaration site even when it lives in +// an IMPORTED module — the reserved-name binding diagnostic covers every +// compiled module, not just the main file. Without universal coverage the +// binding reaches lowering and aborts LLVM verification (a loaded aggregate +// passed by value to a `*Box` param). +// +// Regression (issue 0077): the imported-module facet of issue 0076. Expected: +// one clean diagnostic pointing at the imported module's `s2 := ...`, exit 1 — +// NOT an LLVM verifier abort. +#import "modules/std.sx"; +mod :: #import "1120-diagnostics-imported-reserved-type-name/mod.sx"; + +main :: () -> s32 { + return mod.run_imported_reserved_name(); +} diff --git a/examples/1120-diagnostics-imported-reserved-type-name/mod.sx b/examples/1120-diagnostics-imported-reserved-type-name/mod.sx new file mode 100644 index 0000000..0c9d670 --- /dev/null +++ b/examples/1120-diagnostics-imported-reserved-type-name/mod.sx @@ -0,0 +1,16 @@ +#import "modules/std.sx"; + +Box :: struct { total: s64 = 0; count: s64 = 0; } + +update :: (self: *Box, n: s64) { + self.total += n; + self.count += 1; +} + +run_imported_reserved_name :: () -> s32 { + s2 := Box.{ total = 0, count = 0 }; + update(@s2, 5); + s2.update(7); + print("imported s2 total={} count={}\n", s2.total, s2.count); + return 0; +} diff --git a/examples/expected/1120-diagnostics-imported-reserved-type-name.exit b/examples/expected/1120-diagnostics-imported-reserved-type-name.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/examples/expected/1120-diagnostics-imported-reserved-type-name.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1120-diagnostics-imported-reserved-type-name.stderr b/examples/expected/1120-diagnostics-imported-reserved-type-name.stderr new file mode 100644 index 0000000..f5469a0 --- /dev/null +++ b/examples/expected/1120-diagnostics-imported-reserved-type-name.stderr @@ -0,0 +1,5 @@ +error: 's2' is a reserved type name and cannot be used as an identifier + --> examples/1120-diagnostics-imported-reserved-type-name/mod.sx:11:5 + | +11 | s2 := Box.{ total = 0, count = 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/examples/expected/1120-diagnostics-imported-reserved-type-name.stdout b/examples/expected/1120-diagnostics-imported-reserved-type-name.stdout new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/1120-diagnostics-imported-reserved-type-name.stdout @@ -0,0 +1 @@ + diff --git a/issues/0076-stack-struct-addrof-passed-by-value.md b/issues/0076-stack-struct-addrof-passed-by-value.md index 1579131..06f247d 100644 --- a/issues/0076-stack-struct-addrof-passed-by-value.md +++ b/issues/0076-stack-struct-addrof-passed-by-value.md @@ -31,8 +31,18 @@ > both `update(@h, …)` and `h.update(…)`. > > Pre-existing example `examples/0904-...` declared locals `s1`/`s2` (incidental -> names); renamed to `filled`/`empty`. Scope: main-file decls only, matching the -> pass's existing trusted-imports convention. +> names); renamed to `filled`/`empty`. +> +> **Coverage extension (issue 0077).** The first landing scoped the binding +> check to main-file decls (matching the unknown-type check's trusted-imports +> convention); an imported module could still declare `s2 := …` and hit the +> original LLVM verifier abort. The reserved-name binding diagnostic now runs +> over EVERY compiled module — imported user modules (descending the +> `namespace_decl` an `mod :: #import` wraps) AND the stdlib `library/` — and +> the two `u1` locals in `library/modules/ui/renderer.sx` were renamed +> accordingly. The unknown-type check (issue 0064) stays main-file-only. See +> issue 0077 for the imported-module facet and its pinned regression test +> `examples/1120-diagnostics-imported-reserved-type-name.sx`. ## Symptom (how it first surfaced) diff --git a/issues/0077-imported-reserved-type-name-binding.md b/issues/0077-imported-reserved-type-name-binding.md new file mode 100644 index 0000000..e5733d7 --- /dev/null +++ b/issues/0077-imported-reserved-type-name-binding.md @@ -0,0 +1,103 @@ +# 0077 — reserved type-name binding diagnostic skips imported modules + +> **Status: RESOLVED.** +> +> **Root cause:** the reserved-name binding diagnostic (issue 0076) only ran +> over main-file decls (`UnknownTypeChecker.run`'s `main_file` filter). An +> imported module's `s2 := …` was never checked and reached lowering, where the +> address-of family loaded the whole aggregate and passed it by value to a +> `*Box` param — LLVM verifier abort. +> +> **Fix:** the binding check (`checkBindingNames` in +> `src/ir/semantic_diagnostics.zig`) now walks EVERY compiled module — no +> main-file filter — visiting every `var`/`:=`/typed-local binding name and +> function/lambda/struct-method parameter at any depth, and descending the +> `namespace_decl` that a `mod :: #import` wraps so imported-module decls are +> reached. The walk tracks each module's `source_file` (via the diagnostic +> list's `current_source_file`, saved/restored per node) so the diagnostic +> renders against the imported module's text. Rejection still defers to the +> parser's `name_class.Type.fromName` classifier (no drift). The unknown-type +> check (issue 0064) stays main-file-only. No lowering special-case; the +> `.identifier`-only address-of paths are unchanged. +> +> **Stdlib audit:** the only reserved-name bindings under `library/` were two +> `u1` locals in `library/modules/ui/renderer.sx` (lines 203, 382); the UV-coord +> locals were renamed `u_min`/`u_max`/`v_min`/`v_max`. No reserved parameter +> names or other reserved bindings exist in `library/` or `examples/`. +> +> **Regression test:** `examples/1120-diagnostics-imported-reserved-type-name.sx` +> (+ companion `1120-diagnostics-imported-reserved-type-name/mod.sx`) — an +> imported module declaring `s2 := …` now emits the clean diagnostic at the +> import's declaration site (exit 1), not an LLVM abort. + +## Symptom + +An imported module can still declare a parameter or `var` binding whose name is a +reserved/builtin type name. Observed: the imported-module repro below reaches +lowering and fails LLVM verification by passing a loaded struct value to a +`*Box` parameter. Expected: the same declaration-site diagnostic used for +main-file issue 0076 should reject the imported module's `s2` binding before +lowering. + +## Reproduction + +Create these two files under the repo root, then run +`./zig-out/bin/sx run .sx-tmp/issue0077_main.sx`. + +`.sx-tmp/issue0077_mod.sx`: + +```sx +#import "modules/std.sx"; + +Box :: struct { total: s64 = 0; count: s64 = 0; } + +update :: (self: *Box, n: s64) { + self.total += n; + self.count += 1; +} + +run_imported_reserved_name :: () -> s32 { + s2 := Box.{ total = 0, count = 0 }; + update(@s2, 5); + s2.update(7); + print("imported s2 total={} count={}\n", s2.total, s2.count); + return 0; +} +``` + +`.sx-tmp/issue0077_main.sx`: + +```sx +#import "modules/std.sx"; +mod :: #import ".sx-tmp/issue0077_mod.sx"; + +main :: () -> s32 { + return mod.run_imported_reserved_name(); +} +``` + +Current output on `flow/sx-foundation/F0.1`: + +```text +LLVM verification failed: Call parameter type does not match function signature! + %load = load { i64, i64 }, ptr %alloca, align 8, !dbg !461 + ptr call void @update(ptr %0, { i64, i64 } %load, i64 5), !dbg !462 +``` + +## Investigation prompt + +Investigate and fix issue 0077 in the sx compiler. The suspected area is +`src/ir/semantic_diagnostics.zig`, especially +`UnknownTypeChecker.run` and its `main_file` filter. Attempt 2 for issue 0076 +added `checkBindingName`, but it only runs over main-file declarations; imported +modules are still trusted and can hit the original LLVM verifier failure. The +fix likely needs to apply reserved-type-name binding diagnostics to all user +source modules that are lowered, while preserving the existing trusted-stdlib +or library convention only where intentionally required. Also audit existing +reserved-name bindings in `library/` (for example `u1 := ...` in +`library/modules/ui/renderer.sx`) and rename any source that will become +newly illegal under the corrected rule. + +Verification: run the two-file repro above and expect a clean diagnostic at the +imported module's `s2 := ...` declaration, not LLVM verification failure. Then +run `zig build`, `zig build test`, and `bash tests/run_examples.sh`. diff --git a/library/modules/ui/renderer.sx b/library/modules/ui/renderer.sx index 9d3556c..1bc6043 100755 --- a/library/modules/ui/renderer.sx +++ b/library/modules/ui/renderer.sx @@ -198,17 +198,17 @@ UIRenderer :: struct { w := frame.size.width; h := frame.size.height; - u0 := uv_min.x; - v0 := uv_min.y; - u1 := uv_max.x; - v1 := uv_max.y; + u_min := uv_min.x; + v_min := uv_min.y; + u_max := uv_max.x; + v_max := uv_max.y; - self.write_vertex(x0, y0, u0, v0, r, g, b, a, radius, border_w, w, h); - self.write_vertex(x1, y0, u1, v0, r, g, b, a, radius, border_w, w, h); - self.write_vertex(x0, y1, u0, v1, r, g, b, a, radius, border_w, w, h); - self.write_vertex(x1, y0, u1, v0, r, g, b, a, radius, border_w, w, h); - self.write_vertex(x1, y1, u1, v1, r, g, b, a, radius, border_w, w, h); - self.write_vertex(x0, y1, u0, v1, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x0, y0, u_min, v_min, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x1, y0, u_max, v_min, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x0, y1, u_min, v_max, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x1, y0, u_max, v_min, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x1, y1, u_max, v_max, r, g, b, a, radius, border_w, w, h); + self.write_vertex(x0, y1, u_min, v_max, r, g, b, a, radius, border_w, w, h); } write_vertex :: (self: *UIRenderer, x: f32, y: f32, u: f32, v: f32, r: f32, g: f32, b: f32, a: f32, cr: f32, bw: f32, rw: f32, rh: f32) { @@ -377,10 +377,10 @@ UIRenderer :: struct { gx1 := gx0 + cached.width * inv_dpi; gy1 := gy0 + cached.height * inv_dpi; - u0 := cached.uv_x; - v0 := cached.uv_y; - u1 := cached.uv_x + cached.uv_w; - v1 := cached.uv_y + cached.uv_h; + u_min := cached.uv_x; + v_min := cached.uv_y; + u_max := cached.uv_x + cached.uv_w; + v_max := cached.uv_y + cached.uv_h; if self.vertex_count + 6 > MAX_UI_VERTICES { @@ -389,12 +389,12 @@ UIRenderer :: struct { // corner_radius = -1.0 signals "text mode" to the fragment shader neg1 : f32 = 0.0 - 1.0; - self.write_vertex(gx0, gy0, u0, v0, r, g, b, a, neg1, 0.0, 0.0, 0.0); - self.write_vertex(gx1, gy0, u1, v0, r, g, b, a, neg1, 0.0, 0.0, 0.0); - self.write_vertex(gx0, gy1, u0, v1, r, g, b, a, neg1, 0.0, 0.0, 0.0); - self.write_vertex(gx1, gy0, u1, v0, r, g, b, a, neg1, 0.0, 0.0, 0.0); - self.write_vertex(gx1, gy1, u1, v1, r, g, b, a, neg1, 0.0, 0.0, 0.0); - self.write_vertex(gx0, gy1, u0, v1, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx0, gy0, u_min, v_min, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx1, gy0, u_max, v_min, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx0, gy1, u_min, v_max, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx1, gy0, u_max, v_min, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx1, gy1, u_max, v_max, r, g, b, a, neg1, 0.0, 0.0, 0.0); + self.write_vertex(gx0, gy1, u_min, v_max, r, g, b, a, neg1, 0.0, 0.0, 0.0); } } i += 1; diff --git a/src/ir/semantic_diagnostics.zig b/src/ir/semantic_diagnostics.zig index 0015a11..9578c08 100644 --- a/src/ir/semantic_diagnostics.zig +++ b/src/ir/semantic_diagnostics.zig @@ -11,15 +11,20 @@ const TypeTable = types.TypeTable; const ProgramIndex = program_index_mod.ProgramIndex; const TypeResolver = type_resolver.TypeResolver; -/// Declaration-name / type-position diagnostic pass. Two checks, both over the -/// main file's decls, before lowering: +/// Declaration-name / type-position diagnostic pass. Two checks, before +/// lowering: /// /// 1. Unknown-type diagnostic (issue 0064), extracted from `Lowering` /// (architecture phase A2.4): an identifier used in a type position that /// names no declared type, primitive, or in-scope generic type parameter. -/// 2. Reserved-type-name binding (issue 0076): a value binding (local/global -/// `var` or a parameter) spelled as a reserved/builtin type name. See -/// `isReservedTypeName`. +/// Main-file decls only — imported / library modules are trusted, matching +/// `checkErrorFlow`. +/// 2. Reserved-type-name binding (issues 0076, 0077): a value binding +/// (local/global `var`, a typed-local, or a parameter) spelled as a +/// reserved/builtin type name. See `isReservedTypeName`. Runs over EVERY +/// compiled module (no main-file filter): such a binding mis-lowers the same +/// way wherever declared, so an imported module or the stdlib is no +/// exception. /// /// Without (1)'s check, `TypeResolver.resolveNamed`'s empty-struct-stub fallback silently /// fabricates a 0-field struct named after the unknown identifier — so a value @@ -42,6 +47,21 @@ pub const UnknownTypeChecker = struct { main_file: ?[]const u8, pub fn run(self: UnknownTypeChecker, decls: []const *const Node) void { + // Reserved-type-name binding diagnostic (issues 0076, 0077): rejects any + // parameter name or `var` / `:=` / typed-local binding name spelled as a + // reserved/builtin type name. Runs over EVERY compiled module — imported + // user modules and the stdlib `library/` included — because such a + // binding mis-lowers identically wherever it is declared: a loaded + // aggregate passed by value to a `ptr` param → LLVM verifier abort. No + // main-file filter (unlike the unknown-type check below) and no declared- + // type / scope context — rejection is purely on spelling. The walk + // tracks each module's source file (via the diagnostic list's + // `current_source_file`, saved/restored per node) so an imported-module + // diagnostic renders against that module's text (issue 0077). + for (decls) |decl| self.checkBindingNames(decl); + + // Unknown-type diagnostic (issue 0064): main-file decls only; imported + // and library modules are trusted, matching `checkErrorFlow`. var declared = std.StringHashMap(void).init(self.alloc); defer declared.deinit(); self.collectDeclaredTypeNames(decls, &declared); @@ -54,7 +74,6 @@ pub const UnknownTypeChecker = struct { switch (decl.data) { .fn_decl => self.checkFnSignatureTypes(&decl.data.fn_decl, &declared), .struct_decl => |sd| self.checkStructFieldTypes(&sd, &declared), - .var_decl => |vd| self.checkBindingName(vd.name, decl.span), .const_decl => |cd| switch (cd.value.data) { .fn_decl => self.checkFnSignatureTypes(&cd.value.data.fn_decl, &declared), .struct_decl => |sd| self.checkStructFieldTypes(&sd, &declared), @@ -65,6 +84,108 @@ 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. + 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) { + .var_decl => |vd| { + self.checkBindingName(vd.name, node.span); + if (vd.value) |v| self.checkBindingNames(v); + }, + .fn_decl => |fd| { + for (fd.params) |p| self.checkBindingName(p.name, p.name_span); + self.checkBindingNames(fd.body); + }, + .lambda => |lm| { + for (lm.params) |p| self.checkBindingName(p.name, p.name_span); + 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), + .if_expr => |ie| { + self.checkBindingNames(ie.condition); + self.checkBindingNames(ie.then_branch); + if (ie.else_branch) |e| self.checkBindingNames(e); + }, + .while_expr => |we| { + self.checkBindingNames(we.condition); + self.checkBindingNames(we.body); + }, + .for_expr => |fe| { + 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); + }, + .push_stmt => |ps| { + self.checkBindingNames(ps.context_expr); + self.checkBindingNames(ps.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), + .call => |c| { + self.checkBindingNames(c.callee); + for (c.args) |a| self.checkBindingNames(a); + }, + .binary_op => |b| { + self.checkBindingNames(b.lhs); + self.checkBindingNames(b.rhs); + }, + .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); + }, + .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), + .force_unwrap => |fu| self.checkBindingNames(fu.operand), + .null_coalesce => |nc| { + self.checkBindingNames(nc.lhs); + self.checkBindingNames(nc.rhs); + }, + .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), + .spread_expr => |se| self.checkBindingNames(se.operand), + else => {}, + } + } + /// Collect every top-level name that can legitimately appear in a type /// position: const-decl names (covers `T :: struct/enum/union/error/alias` /// and value consts), plus the scan-populated foreign-class / generic- @@ -233,7 +354,6 @@ pub const UnknownTypeChecker = struct { } } } - for (params) |p| self.checkBindingName(p.name, p.name_span); for (params) |p| self.checkTypeNodeForUnknown(p.type_expr, declared, in_scope.items, type_vals.items); if (return_type) |rt| self.checkTypeNodeForUnknown(rt, declared, in_scope.items, type_vals.items); self.walkBodyTypes(body, declared, in_scope, type_vals); @@ -285,7 +405,6 @@ pub const UnknownTypeChecker = struct { .multi_assign => |ma| for (ma.values) |v| self.walkBodyTypes(v, declared, in_scope, type_vals), .destructure_decl => |dd| self.walkBodyTypes(dd.value, declared, in_scope, type_vals), .var_decl => |vd| { - self.checkBindingName(vd.name, node.span); if (vd.type_annotation) |ta| self.checkTypeNodeForUnknown(ta, declared, in_scope.items, type_vals.items); if (vd.value) |v| self.walkBodyTypes(v, declared, in_scope, type_vals); },