diff --git a/examples/1114-diagnostics-unknown-type-nested-closure-rejected.sx b/examples/1114-diagnostics-unknown-type-nested-closure-rejected.sx new file mode 100644 index 0000000..0e6ca43 --- /dev/null +++ b/examples/1114-diagnostics-unknown-type-nested-closure-rejected.sx @@ -0,0 +1,12 @@ +// An unknown type in a NESTED closure body's local annotation is rejected, +// with the enclosing function's generic params still in scope. Previously the +// body walk stopped at closure / nested-function boundaries, so a typo'd type +// inside a closure slipped through and silently became a 0-field struct. +// Regression (issue 0064, nested scopes). Expected: error at the annotation; exit 1. +main :: () -> s32 { + f := closure(() -> s32 { + bad: Coordnate = ---; + return 0; + }); + return f(); +} diff --git a/examples/1115-diagnostics-cast-value-param-rejected.sx b/examples/1115-diagnostics-cast-value-param-rejected.sx new file mode 100644 index 0000000..574b012 --- /dev/null +++ b/examples/1115-diagnostics-cast-value-param-rejected.sx @@ -0,0 +1,12 @@ +// `cast(T)` where `T` is a value parameter declared `: Type` (without the `$` +// generic sigil) is rejected with the generic-param hint. Previously it +// silently cast to a fabricated empty struct (an unknown *literal* cast target +// already errored via value resolution, but the value-param case was silent). +// Regression (issue 0064, cast position). Expected: tailored error; exit 1. +conv :: (T: Type, x: s32) -> s32 { + return cast(T) x; +} + +main :: () -> s32 { + return conv(s32, 5); +} diff --git a/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.exit b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stderr b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stderr new file mode 100644 index 0000000..e9e56f9 --- /dev/null +++ b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stderr @@ -0,0 +1,5 @@ +error: unknown type 'Coordnate' + --> /Users/agra/projects/sx/examples/1114-diagnostics-unknown-type-nested-closure-rejected.sx:8:14 + | + 8 | bad: Coordnate = ---; + | ^^^^^^^^^ diff --git a/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stdout b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stdout new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/1114-diagnostics-unknown-type-nested-closure-rejected.stdout @@ -0,0 +1 @@ + diff --git a/examples/expected/1115-diagnostics-cast-value-param-rejected.exit b/examples/expected/1115-diagnostics-cast-value-param-rejected.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/examples/expected/1115-diagnostics-cast-value-param-rejected.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1115-diagnostics-cast-value-param-rejected.stderr b/examples/expected/1115-diagnostics-cast-value-param-rejected.stderr new file mode 100644 index 0000000..f57fd80 --- /dev/null +++ b/examples/expected/1115-diagnostics-cast-value-param-rejected.stderr @@ -0,0 +1,5 @@ +error: 'T' is a value parameter, not a type; introduce a generic type parameter with `$T: Type` + --> /Users/agra/projects/sx/examples/1115-diagnostics-cast-value-param-rejected.sx:7:17 + | + 7 | return cast(T) x; + | ^ diff --git a/examples/expected/1115-diagnostics-cast-value-param-rejected.stdout b/examples/expected/1115-diagnostics-cast-value-param-rejected.stdout new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/1115-diagnostics-cast-value-param-rejected.stdout @@ -0,0 +1 @@ + diff --git a/issues/0064-nondollar-type-param-silent-empty-struct.md b/issues/0064-nondollar-type-param-silent-empty-struct.md index 04d2591..d720717 100644 --- a/issues/0064-nondollar-type-param-silent-empty-struct.md +++ b/issues/0064-nondollar-type-param-silent-empty-struct.md @@ -26,10 +26,19 @@ > ran with the value dropped. Nested function / closure bodies are their own scope > and are not descended (safe under-coverage); explicit `cast(T)` already has its > own `unresolved` diagnostic and is left to it. -> Regression tests: `examples/1111-diagnostics-nondollar-type-param-rejected.sx` -> (tailored hint), `examples/1112-diagnostics-unknown-type-name-rejected.sx` -> (typo'd field type), and `examples/1113-diagnostics-unknown-type-local-var-rejected.sx` -> (body-level local annotation) — all exit 1. Suite: 348 passed, 0 failed. +> The walk descends into **nested closure / function bodies** too (`walkBodyTypes` +> + `checkScope`): each scope accumulates its generic params onto the parent's, so +> a closure body still sees the enclosing function's `$T`, and a type annotation in +> any nesting depth is checked. `harvestScopeDecls` collects type-decl names across +> the whole body (including nested scopes) so locals aren't false-flagged. Cast +> targets are handled too: `cast(T)` where `T` is a value-`Type` param (the +> otherwise-silent cast case) gets the tailored hint, while an unknown *literal* +> cast target is left to the existing value-resolution `unresolved` diagnostic (no +> double-report). The only remaining under-coverage is benign (annotations buried in +> AST positions the walker doesn't descend stay unchecked — never a false positive). +> Regression tests: `examples/1111` (tailored hint, signature), `1112` (typo'd field +> type), `1113` (body-level local annotation), `1114` (nested-closure annotation), +> `1115` (`cast` value param) — all exit 1. Suite: 350 passed, 0 failed. ## Symptom diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 7e59fd5..f340c8b 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -566,10 +566,10 @@ pub const Lowering = struct { switch (decl.data) { .const_decl => |cd| { out.put(cd.name, {}) catch {}; - if (cd.value.data == .fn_decl) self.collectBodyDeclNames(cd.value.data.fn_decl.body, out); + if (cd.value.data == .fn_decl) self.harvestScopeDecls(cd.value.data.fn_decl.body, out); }, .struct_decl => |sd| out.put(sd.name, {}) catch {}, - .fn_decl => |fd| self.collectBodyDeclNames(fd.body, out), + .fn_decl => |fd| self.harvestScopeDecls(fd.body, out), else => {}, } } @@ -585,73 +585,88 @@ pub const Lowering = struct { while (it_al.next()) |k| out.put(k.*, {}) catch {}; } - /// Collect names declared inside a function body — local `T :: struct/enum/ - /// union` types and named consts — so a body-level type annotation - /// referencing one isn't flagged. Recurses control-flow bodies but stops at - /// nested function / closure boundaries (those have their own scope and are - /// not body-checked). - fn collectBodyDeclNames(self: *Lowering, node: *const Node, out: *std.StringHashMap(void)) void { + /// Harvest every type-declaration name (local `T :: struct/enum/union` and + /// named consts) anywhere in a function body — including inside nested + /// closure / function bodies — into the global declared set, so a type + /// annotation in any scope that references one isn't flagged. Over-collection + /// is safe: it only ever relaxes the unknown-type check, never tightens it. + fn harvestScopeDecls(self: *Lowering, node: *const Node, out: *std.StringHashMap(void)) void { switch (node.data) { - .block => |b| for (b.stmts) |s| self.collectBodyDeclNames(s, out), + .block => |b| for (b.stmts) |s| self.harvestScopeDecls(s, out), .if_expr => |ie| { - self.collectBodyDeclNames(ie.then_branch, out); - if (ie.else_branch) |e| self.collectBodyDeclNames(e, out); + self.harvestScopeDecls(ie.condition, out); + self.harvestScopeDecls(ie.then_branch, out); + if (ie.else_branch) |e| self.harvestScopeDecls(e, out); }, - .while_expr => |we| self.collectBodyDeclNames(we.body, out), - .for_expr => |fe| self.collectBodyDeclNames(fe.body, out), - .match_expr => |me| for (me.arms) |arm| self.collectBodyDeclNames(arm.body, out), - .push_stmt => |ps| self.collectBodyDeclNames(ps.body, out), - .defer_stmt => |ds| self.collectBodyDeclNames(ds.expr, out), - .onfail_stmt => |os| self.collectBodyDeclNames(os.body, out), + .while_expr => |we| { + self.harvestScopeDecls(we.condition, out); + self.harvestScopeDecls(we.body, out); + }, + .for_expr => |fe| { + self.harvestScopeDecls(fe.iterable, out); + if (fe.range_end) |re| self.harvestScopeDecls(re, out); + self.harvestScopeDecls(fe.body, out); + }, + .match_expr => |me| { + self.harvestScopeDecls(me.subject, out); + for (me.arms) |arm| self.harvestScopeDecls(arm.body, out); + }, + .push_stmt => |ps| { + self.harvestScopeDecls(ps.context_expr, out); + self.harvestScopeDecls(ps.body, out); + }, + .defer_stmt => |ds| self.harvestScopeDecls(ds.expr, out), + .onfail_stmt => |os| self.harvestScopeDecls(os.body, out), + .return_stmt => |r| if (r.value) |v| self.harvestScopeDecls(v, out), + .raise_stmt => |rs| self.harvestScopeDecls(rs.tag, out), + .assignment => |a| { + self.harvestScopeDecls(a.value, out); + self.harvestScopeDecls(a.target, out); + }, + .multi_assign => |ma| for (ma.values) |v| self.harvestScopeDecls(v, out), + .destructure_decl => |dd| self.harvestScopeDecls(dd.value, out), + .var_decl => |vd| if (vd.value) |v| self.harvestScopeDecls(v, out), .const_decl => |cd| { out.put(cd.name, {}) catch {}; - self.collectBodyDeclNames(cd.value, out); + self.harvestScopeDecls(cd.value, out); }, - .var_decl => |vd| if (vd.value) |v| self.collectBodyDeclNames(v, out), .struct_decl => |sd| out.put(sd.name, {}) catch {}, .enum_decl => |ed| out.put(ed.name, {}) catch {}, .union_decl => |ud| out.put(ud.name, {}) catch {}, - else => {}, - } - } - - /// Walk a function body checking type annotations on local var / const - /// declarations (and body-local struct fields). `in_scope` and `type_vals` - /// carry the enclosing function's generic params / value-`Type` params. - /// Recurses control-flow and decl-value blocks (so `x := if c { a: T; a }` - /// is reached) but not nested function / closure bodies. - fn checkBodyTypes( - self: *Lowering, - node: *const Node, - declared: *std.StringHashMap(void), - in_scope: []const ast.StructTypeParam, - type_vals: []const []const u8, - ) void { - switch (node.data) { - .block => |b| for (b.stmts) |s| self.checkBodyTypes(s, declared, in_scope, type_vals), - .if_expr => |ie| { - self.checkBodyTypes(ie.then_branch, declared, in_scope, type_vals); - if (ie.else_branch) |e| self.checkBodyTypes(e, declared, in_scope, type_vals); + .call => |c| { + self.harvestScopeDecls(c.callee, out); + for (c.args) |a| self.harvestScopeDecls(a, out); }, - .while_expr => |we| self.checkBodyTypes(we.body, declared, in_scope, type_vals), - .for_expr => |fe| self.checkBodyTypes(fe.body, declared, in_scope, type_vals), - .match_expr => |me| for (me.arms) |arm| self.checkBodyTypes(arm.body, declared, in_scope, type_vals), - .push_stmt => |ps| self.checkBodyTypes(ps.body, declared, in_scope, type_vals), - .defer_stmt => |ds| self.checkBodyTypes(ds.expr, declared, in_scope, type_vals), - .onfail_stmt => |os| self.checkBodyTypes(os.body, declared, in_scope, type_vals), - .var_decl => |vd| { - if (vd.type_annotation) |ta| self.checkTypeNodeForUnknown(ta, declared, in_scope, type_vals); - if (vd.value) |v| self.checkBodyTypes(v, declared, in_scope, type_vals); + .binary_op => |b| { + self.harvestScopeDecls(b.lhs, out); + self.harvestScopeDecls(b.rhs, out); }, - .const_decl => |cd| { - if (cd.type_annotation) |ta| self.checkTypeNodeForUnknown(ta, declared, in_scope, type_vals); - self.checkBodyTypes(cd.value, declared, in_scope, type_vals); + .unary_op => |u| self.harvestScopeDecls(u.operand, out), + .field_access => |fa| self.harvestScopeDecls(fa.object, out), + .index_expr => |ix| { + self.harvestScopeDecls(ix.object, out); + self.harvestScopeDecls(ix.index, out); }, - .assignment => |a| self.checkBodyTypes(a.value, declared, in_scope, type_vals), - .return_stmt => |r| if (r.value) |v| self.checkBodyTypes(v, declared, in_scope, type_vals), - .struct_decl => |sd| if (sd.type_params.len == 0) { - for (sd.field_types) |ft| self.checkTypeNodeForUnknown(ft, declared, in_scope, type_vals); + .struct_literal => |sl| { + for (sl.field_inits) |fi| self.harvestScopeDecls(fi.value, out); + if (sl.init_block) |ib| self.harvestScopeDecls(ib, out); }, + .array_literal => |al| for (al.elements) |e| self.harvestScopeDecls(e, out), + .force_unwrap => |fu| self.harvestScopeDecls(fu.operand, out), + .null_coalesce => |nc| { + self.harvestScopeDecls(nc.lhs, out); + self.harvestScopeDecls(nc.rhs, out); + }, + .deref_expr => |de| self.harvestScopeDecls(de.operand, out), + .try_expr => |te| self.harvestScopeDecls(te.operand, out), + .catch_expr => |ce| { + self.harvestScopeDecls(ce.operand, out); + self.harvestScopeDecls(ce.body, out); + }, + .comptime_expr => |ce| self.harvestScopeDecls(ce.expr, out), + .spread_expr => |se| self.harvestScopeDecls(se.operand, out), + .lambda => |lm| self.harvestScopeDecls(lm.body, out), + .fn_decl => |fd| self.harvestScopeDecls(fd.body, out), else => {}, } } @@ -664,11 +679,36 @@ pub const Lowering = struct { } fn checkFnSignatureTypes(self: *Lowering, fd: *const ast.FnDecl, declared: *std.StringHashMap(void)) void { - // Value params declared `: Type` (no `$`) — using one in a type - // position is the issue-0064 misuse; surface a tailored hint. + var in_scope = std.ArrayList(ast.StructTypeParam).empty; + defer in_scope.deinit(self.alloc); var type_vals = std.ArrayList([]const u8).empty; defer type_vals.deinit(self.alloc); - for (fd.params) |p| { + self.checkScope(fd.type_params, fd.params, fd.return_type, fd.body, declared, &in_scope, &type_vals); + } + + /// Check one function/closure scope: its generic params (`$T`) and value- + /// `Type` params become in-scope (accumulated onto the parent's, so a nested + /// closure still sees the outer function's `$T`), its param/return + /// annotations are checked, then its body is walked. The scope additions are + /// popped on return. + fn checkScope( + self: *Lowering, + type_params: []const ast.StructTypeParam, + params: []const ast.Param, + return_type: ?*Node, + body: *const Node, + declared: *std.StringHashMap(void), + in_scope: *std.ArrayList(ast.StructTypeParam), + type_vals: *std.ArrayList([]const u8), + ) void { + const save_s = in_scope.items.len; + const save_v = type_vals.items.len; + defer in_scope.shrinkRetainingCapacity(save_s); + defer type_vals.shrinkRetainingCapacity(save_v); + for (type_params) |tp| in_scope.append(self.alloc, tp) catch {}; + // Value params declared `: Type` (no `$`) — using one in a type position + // is the issue-0064 misuse; track them for the tailored hint. + for (params) |p| { if (p.type_expr.data == .type_expr) { const cn = p.type_expr.data.type_expr.name; if (std.mem.eql(u8, cn, "Type") or std.mem.eql(u8, cn, "type")) { @@ -676,9 +716,129 @@ pub const Lowering = struct { } } } - for (fd.params) |p| self.checkTypeNodeForUnknown(p.type_expr, declared, fd.type_params, type_vals.items); - if (fd.return_type) |rt| self.checkTypeNodeForUnknown(rt, declared, fd.type_params, type_vals.items); - self.checkBodyTypes(fd.body, declared, fd.type_params, type_vals.items); + 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); + } + + /// Walk a scope body checking type annotations on local var / const + /// declarations (and body-local struct fields), descending control flow and + /// expressions. Nested closure / function literals re-enter via `checkScope` + /// with their own params added to `in_scope`. + fn walkBodyTypes( + self: *Lowering, + node: *const Node, + declared: *std.StringHashMap(void), + in_scope: *std.ArrayList(ast.StructTypeParam), + type_vals: *std.ArrayList([]const u8), + ) void { + switch (node.data) { + .block => |b| for (b.stmts) |s| self.walkBodyTypes(s, declared, in_scope, type_vals), + .if_expr => |ie| { + self.walkBodyTypes(ie.condition, declared, in_scope, type_vals); + self.walkBodyTypes(ie.then_branch, declared, in_scope, type_vals); + if (ie.else_branch) |e| self.walkBodyTypes(e, declared, in_scope, type_vals); + }, + .while_expr => |we| { + self.walkBodyTypes(we.condition, declared, in_scope, type_vals); + self.walkBodyTypes(we.body, declared, in_scope, type_vals); + }, + .for_expr => |fe| { + self.walkBodyTypes(fe.iterable, declared, in_scope, type_vals); + if (fe.range_end) |re| self.walkBodyTypes(re, declared, in_scope, type_vals); + self.walkBodyTypes(fe.body, declared, in_scope, type_vals); + }, + .match_expr => |me| { + self.walkBodyTypes(me.subject, declared, in_scope, type_vals); + for (me.arms) |arm| self.walkBodyTypes(arm.body, declared, in_scope, type_vals); + }, + .push_stmt => |ps| { + self.walkBodyTypes(ps.context_expr, declared, in_scope, type_vals); + self.walkBodyTypes(ps.body, declared, in_scope, type_vals); + }, + .defer_stmt => |ds| self.walkBodyTypes(ds.expr, declared, in_scope, type_vals), + .onfail_stmt => |os| self.walkBodyTypes(os.body, declared, in_scope, type_vals), + .return_stmt => |r| if (r.value) |v| self.walkBodyTypes(v, declared, in_scope, type_vals), + .raise_stmt => |rs| self.walkBodyTypes(rs.tag, declared, in_scope, type_vals), + .assignment => |a| { + self.walkBodyTypes(a.value, declared, in_scope, type_vals); + self.walkBodyTypes(a.target, declared, in_scope, type_vals); + }, + .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| { + 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); + }, + .const_decl => |cd| { + if (cd.type_annotation) |ta| self.checkTypeNodeForUnknown(ta, declared, in_scope.items, type_vals.items); + self.walkBodyTypes(cd.value, declared, in_scope, type_vals); + }, + .struct_decl => |sd| if (sd.type_params.len == 0) { + for (sd.field_types) |ft| self.checkTypeNodeForUnknown(ft, declared, in_scope.items, type_vals.items); + }, + .call => |c| { + // `cast(T) x` parses to a `cast` call whose first arg is the + // target type spelled as an expression. An unknown *literal* + // target already errors via value resolution; only flag the + // otherwise-silent value-`Type`-param case here. + if (c.callee.data == .identifier and std.mem.eql(u8, c.callee.data.identifier.name, "cast") and c.args.len == 2) { + self.checkCastTarget(c.args[0], in_scope.items, type_vals.items); + } + self.walkBodyTypes(c.callee, declared, in_scope, type_vals); + for (c.args) |a| self.walkBodyTypes(a, declared, in_scope, type_vals); + }, + .binary_op => |b| { + self.walkBodyTypes(b.lhs, declared, in_scope, type_vals); + self.walkBodyTypes(b.rhs, declared, in_scope, type_vals); + }, + .unary_op => |u| self.walkBodyTypes(u.operand, declared, in_scope, type_vals), + .field_access => |fa| self.walkBodyTypes(fa.object, declared, in_scope, type_vals), + .index_expr => |ix| { + self.walkBodyTypes(ix.object, declared, in_scope, type_vals); + self.walkBodyTypes(ix.index, declared, in_scope, type_vals); + }, + .struct_literal => |sl| { + for (sl.field_inits) |fi| self.walkBodyTypes(fi.value, declared, in_scope, type_vals); + if (sl.init_block) |ib| self.walkBodyTypes(ib, declared, in_scope, type_vals); + }, + .array_literal => |al| for (al.elements) |e| self.walkBodyTypes(e, declared, in_scope, type_vals), + .force_unwrap => |fu| self.walkBodyTypes(fu.operand, declared, in_scope, type_vals), + .null_coalesce => |nc| { + self.walkBodyTypes(nc.lhs, declared, in_scope, type_vals); + self.walkBodyTypes(nc.rhs, declared, in_scope, type_vals); + }, + .deref_expr => |de| self.walkBodyTypes(de.operand, declared, in_scope, type_vals), + .try_expr => |te| self.walkBodyTypes(te.operand, declared, in_scope, type_vals), + .catch_expr => |ce| { + self.walkBodyTypes(ce.operand, declared, in_scope, type_vals); + self.walkBodyTypes(ce.body, declared, in_scope, type_vals); + }, + .comptime_expr => |ce| self.walkBodyTypes(ce.expr, declared, in_scope, type_vals), + .spread_expr => |se| self.walkBodyTypes(se.operand, declared, in_scope, type_vals), + .lambda => |lm| self.checkScope(lm.type_params, lm.params, lm.return_type, lm.body, declared, in_scope, type_vals), + .fn_decl => |fd| self.checkScope(fd.type_params, fd.params, fd.return_type, fd.body, declared, in_scope, type_vals), + else => {}, + } + } + + /// A `cast(T)` target naming a value-`Type` parameter (the otherwise-silent + /// issue-0064 case in cast position) gets the tailored `$T` hint. + fn checkCastTarget(self: *Lowering, arg: *const Node, in_scope: []const ast.StructTypeParam, type_vals: []const []const u8) void { + const name = switch (arg.data) { + .identifier => |id| id.name, + .type_expr => |te| te.name, + else => return, + }; + for (in_scope) |tp| if (std.mem.eql(u8, tp.name, name)) return; + for (type_vals) |tv| { + if (std.mem.eql(u8, tv, name)) { + if (self.diagnostics) |diags| { + diags.addFmt(.err, arg.span, "'{s}' is a value parameter, not a type; introduce a generic type parameter with `${s}: Type`", .{ name, name }); + } + return; + } + } } /// Recurse a type-annotation node to its leaf names, reporting any unknown.