diff --git a/examples/0719-modules-cli-and-json.sx b/examples/0719-modules-cli-and-json.sx new file mode 100644 index 0000000..a640d87 --- /dev/null +++ b/examples/0719-modules-cli-and-json.sx @@ -0,0 +1,56 @@ +// Two modules that each export a top-level `parse` — `std.cli.parse` +// (3-param subcommand dispatch) and `std.json.parse` (2-param document +// reader) — imported into ONE program under DISTINCT namespaces, with +// BOTH `parse`s actually called. +// +// Regression (issue 0100): same-name cross-module functions collided in +// the bare-name function table during IR lowering. Lowering re-resolved a +// call by SHORT name, so importing both modules and calling one bound the +// wrong-arity same-named function and tripped `lazyLowerFunction`'s +// param-count assert (panic). The fix resolves each `pkg.parse(...)` to a +// UNIQUE module-qualified FuncId, so `cli.parse` and `json.parse` are +// independent identities. + +#import "modules/std.sx"; +cli :: #import "modules/std/cli.sx"; +json :: #import "modules/std/json.sx"; + +report :: (label: string, ok: bool) { + if ok { print("{}: ok\n", label); } else { print("{}: FAIL\n", label); } +} + +main :: () -> ! { + gpa := GPA.init(); + arena := Arena.init(xx gpa, 8192); + defer arena.deinit(); + + // ── cli.parse: dispatch + a value flag ───────── + publish_flags : []FlagSpec = .[ + FlagSpec.{ name = "out", takes_value = true, required = true }, + ]; + cmds : []Command = .[ + Command.{ group = "ci", command = "publish", flags = publish_flags }, + Command.{ group = "ci", command = "status", flags = .[] }, + ]; + argv : []string = .["ci", "publish", "--out", "dist"]; + d : Diag = .{}; + p := try cli.parse(argv, cmds, @d); + report("cli-group", p.group == "ci"); + report("cli-command", p.command == "publish"); + report("cli-index", p.cmd_index == 0); + report("cli-flag", p.value_of("out") == "dist"); + + // ── json.parse: read a small document into the value model ─────── + doc := "{\"name\":\"sx\",\"xs\":[1,2,3]}"; + root := try json.parse(doc, xx arena); + o := root.object; + report("json-members", o.len == 2); + report("json-key0", o.items[0].key == "name"); + report("json-str", o.items[0].val.str == "sx"); + xs := o.items[1].val.array; + report("json-arr-len", xs.len == 3); + report("json-arr-2", xs.items[2].int_ == 3); + + print("=== DONE ===\n"); + return; +} diff --git a/examples/expected/0719-modules-cli-and-json.exit b/examples/expected/0719-modules-cli-and-json.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/examples/expected/0719-modules-cli-and-json.exit @@ -0,0 +1 @@ +0 diff --git a/examples/expected/0719-modules-cli-and-json.stderr b/examples/expected/0719-modules-cli-and-json.stderr new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/0719-modules-cli-and-json.stderr @@ -0,0 +1 @@ + diff --git a/examples/expected/0719-modules-cli-and-json.stdout b/examples/expected/0719-modules-cli-and-json.stdout new file mode 100644 index 0000000..0c15f20 --- /dev/null +++ b/examples/expected/0719-modules-cli-and-json.stdout @@ -0,0 +1,10 @@ +cli-group: ok +cli-command: ok +cli-index: ok +cli-flag: ok +json-members: ok +json-key0: ok +json-str: ok +json-arr-len: ok +json-arr-2: ok +=== DONE === diff --git a/issues/0100-cross-module-same-name-fn-lowering-collision.md b/issues/0100-cross-module-same-name-fn-lowering-collision.md new file mode 100644 index 0000000..af3f531 --- /dev/null +++ b/issues/0100-cross-module-same-name-fn-lowering-collision.md @@ -0,0 +1,93 @@ +# 0100 — cross-module same-name function lowering collision + +**RESOLVED.** Two modules each exporting a top-level function with the same +short name (`std.cli.parse`, 3 params; `std.json.parse`, 2 params) collided +in IR lowering's bare-name function table. `fn_ast_map` (short name → AST) +was **last-wins**, while `module.functions` / `resolveFuncByName` are +**first-wins**, so importing both modules and calling one bound the AST of +one function against the FuncId of the other and tripped +`lazyLowerFunction`'s param-count assert (`src/ir/lower.zig:1606`, +`func.params.len == fd.params.len + ctx_slots`) — `panic: reached +unreachable code`. Qualified imports (`j :: #import`) did not help: lowering +keyed everything by short name, so `j.parse` and a bare `parse` resolved to +the same colliding entry. + +**Fix** (`src/ir/lower.zig`, `src/ast.zig`, `src/imports.zig`): + +1. **Module-qualified identity.** A namespaced import's OWN plain functions + are now registered under their qualified name (`ns.fn`) in `fn_ast_map`, + giving `cli.parse` / `json.parse` independent identities. The qualified + resolution paths in `CallResolver.plan` and `lowerCall` already prefer + `ns.fn` — they just had nothing to find. `NamespaceDecl` carries the + module's `own_decls` (populated in `imports.addNamespace`) so the + registration covers authored decls, not transitive flat imports. Generic + / comptime / pack / foreign functions are excluded — they dispatch by + monomorphization off the bare template name, not the plain + `resolveFuncByName` path, so a qualified alias would strand their + per-call type bindings. The qualified function is declared + lowered on + demand by `lazyLowerFunction`'s null-FuncId path (no eager `declareFunction`, + which would resolve types before the forward-alias fixpoint). +2. **First-wins bare registration.** `scanDecls` no longer lets a later + namespace recursion clobber an existing bare `fn_ast_map` entry, aligning + it with `mergeFlat` / `resolveFuncByName`. A bare `parse` with one module + flat-imported now consistently resolves to the first (unqualified-scope) + function instead of splitting AST/FuncId across modules. + +Regression: `examples/0719-modules-cli-and-json.sx` imports BOTH `std.cli` +and `std.json` under distinct namespaces and calls both `cli.parse` +(dispatch) and `json.parse` (document read), asserting correct results. +Panics on pre-fix code; passes after. + +## Symptom + +- **Observed:** a program that imports two modules each exporting a + same-named top-level function AND calls one crashes IR lowering: + `panic: reached unreachable code` at `src/ir/lower.zig:1606` + (`lazyLowerFunction`) via `lowerCall`. +- **Expected:** each `pkg.fn(...)` resolves to its own module's function; + the program compiles and runs. + +## Reproduction + +```sx +#import "modules/std.sx"; +cli :: #import "modules/std/cli.sx"; +json :: #import "modules/std/json.sx"; + +main :: () -> s32 { + gpa := GPA.init(); + arena := Arena.init(xx gpa, 8192); + defer arena.deinit(); + + cmds : []Command = .[ Command.{ group = "ci", command = "publish", flags = .[] } ]; + argv : []string = .["ci", "publish"]; + d : Diag = .{}; + p, e := cli.parse(argv, cmds, @d); // 3-param cli.parse + if e { return 64; } + + v, je := json.parse("[1,2,3]", xx arena); // 2-param json.parse + if je { return 65; } + return 0; +} +``` + +Pre-fix: `panic: reached unreachable code` at `src/ir/lower.zig:1606`. +Post-fix: compiles and runs (exit 0). + +## Root cause + +`fn_ast_map`, `module.functions` (matched by interned name), and +`lowered_functions` were all keyed by a function's SHORT name. Two functions +sharing a short name across modules occupied the same key; the `put`-order +mismatch (AST last-wins vs FuncId first-wins) drove `lazyLowerFunction` to +lower one signature against the other's body. The qualified-call resolution +machinery already existed but was never fed module-qualified entries. + +## Fix verification + +- `zig build` → 0 +- `zig build test` → 0 (incl. LSP corpus sweep, 471 examples) +- `bash tests/run_examples.sh` → 454 passed, 0 failed +- `examples/0719-modules-cli-and-json.sx`: panics pre-fix, passes post-fix. + +Regression test: `examples/0719-modules-cli-and-json.sx`. diff --git a/src/ast.zig b/src/ast.zig index cd2948c..a0733a4 100644 --- a/src/ast.zig +++ b/src/ast.zig @@ -674,6 +674,12 @@ pub const SpreadExpr = struct { pub const NamespaceDecl = struct { name: []const u8, decls: []const *Node, + /// Decls AUTHORED in the namespaced module itself (its `own_decls`), a + /// subset of `decls` (which also carries the module's transitive flat + /// imports). Lowering registers these under their module-qualified name + /// (`ns.fn`) so `pkg.fn(...)` resolves to a unique FuncId distinct from a + /// same-named function in another module (issue 0100). + own_decls: []const *Node = &.{}, /// True when the namespace NAME was a backtick raw identifier — exempt /// from the reserved-type-name decl check (issue 0089). is_raw: bool = false, diff --git a/src/imports.zig b/src/imports.zig index 6afe18d..8517b4f 100644 --- a/src/imports.zig +++ b/src/imports.zig @@ -362,6 +362,10 @@ pub const ResolvedModule = struct { .data = .{ .namespace_decl = .{ .name = name, .decls = other.decls, + // The module's OWN authored decls — what `ns.fn` should bind + // to (issue 0100). `decls` stays the full transitive list so + // the lowering pass can still resolve transitive callees. + .own_decls = other.own_decls, // Carry the backtick raw escape from the `name :: #import …` // form so a reserved-name namespace is exempt from the decl // check, symmetric to every other decl site (issue 0089). @@ -486,12 +490,16 @@ pub fn resolveImports( // Keep c_import_decl inside namespace so codegen can find sources try ns_decls.append(allocator, decl); + const ns_slice = try ns_decls.toOwnedSlice(allocator); const ns_node = try allocator.create(Node); ns_node.* = .{ .span = decl.span, .data = .{ .namespace_decl = .{ .name = ns_name, - .decls = try ns_decls.toOwnedSlice(allocator), + .decls = ns_slice, + // A C-import namespace authors exactly the wrapped fn + // decls — they ARE its own decls (issue 0100). + .own_decls = ns_slice, .is_raw = ci.is_raw, } }, }; diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 640f2ae..502583c 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -594,6 +594,7 @@ pub const Lowering = struct { .namespace_decl => |ns| { self.registerNamespacedForeignClasses(ns); if (self.main_file != null) { + self.registerNamespaceQualifiedFns(ns.name, ns.own_decls); self.lowerDecls(ns.decls); } }, @@ -691,15 +692,26 @@ pub const Lowering = struct { false; switch (decl.data) { .fn_decl => |fd| { - self.program_index.fn_ast_map.put(fd.name, &decl.data.fn_decl) catch {}; - self.program_index.import_flags.put(fd.name, is_imported) catch {}; + // First-wins on a bare-name collision, matching `mergeFlat` + // and `resolveFuncByName`. A later namespace recursion that + // re-introduces a same-named function (e.g. a second module + // also exporting `parse`) must NOT clobber the AST while the + // function table keeps the first — that split lowers one + // signature against the other's body (issue 0100). The + // shadowed function stays reachable via its qualified name. + if (!self.program_index.fn_ast_map.contains(fd.name)) { + self.program_index.fn_ast_map.put(fd.name, &decl.data.fn_decl) catch {}; + self.program_index.import_flags.put(fd.name, is_imported) catch {}; + } // Declare extern stub for all functions (bodies lowered lazily) self.declareFunction(&fd, fd.name); }, .const_decl => |cd| { if (cd.value.data == .fn_decl) { - self.program_index.fn_ast_map.put(cd.name, &cd.value.data.fn_decl) catch {}; - self.program_index.import_flags.put(cd.name, is_imported) catch {}; + if (!self.program_index.fn_ast_map.contains(cd.name)) { + self.program_index.fn_ast_map.put(cd.name, &cd.value.data.fn_decl) catch {}; + self.program_index.import_flags.put(cd.name, is_imported) catch {}; + } self.declareFunction(&cd.value.data.fn_decl, cd.name); } else if (cd.value.data == .struct_decl) { self.registerStructDecl(&cd.value.data.struct_decl); @@ -884,6 +896,7 @@ pub const Lowering = struct { self.registerNamespacedForeignClasses(ns); if (self.main_file != null) { self.scanDecls(ns.decls); + self.registerNamespaceQualifiedFns(ns.name, ns.own_decls); } }, .ufcs_alias => |ua| { @@ -1421,6 +1434,57 @@ pub const Lowering = struct { func.has_implicit_ctx = wants_ctx; } + /// Register a namespaced import's OWN functions under their module-qualified + /// name (`ns.fn`), giving each a UNIQUE FuncId in the function table. Two + /// modules each exporting a top-level `parse` otherwise collide in the + /// bare-name `fn_ast_map` / function table (last-wins) while `resolveFuncByName` + /// picks the first declared, so `lazyLowerFunction` lowers one signature + /// against the other's body and trips its param-count assert (issue 0100). + /// The bare recursion in `scanDecls` still registers intra-module bare calls; + /// this adds the qualified identity the `pkg.fn(...)` resolution paths in + /// `CallResolver.plan` / `lowerCall` already prefer. + fn registerNamespaceQualifiedFns(self: *Lowering, ns_name: []const u8, own_decls: []const *Node) void { + const saved_source = self.current_source_file; + defer self.setCurrentSourceFile(saved_source); + for (own_decls) |decl| { + self.setCurrentSourceFile(decl.source_file); + switch (decl.data) { + .fn_decl => self.registerQualifiedFn(ns_name, &decl.data.fn_decl, decl.data.fn_decl.name), + .const_decl => |cd| { + if (cd.value.data == .fn_decl) { + self.registerQualifiedFn(ns_name, &cd.value.data.fn_decl, cd.name); + } + }, + else => {}, + } + } + } + + fn registerQualifiedFn(self: *Lowering, ns_name: []const u8, fd: *const ast.FnDecl, short: []const u8) void { + // Only PLAIN free functions need a qualified identity. Generic / + // comptime / pack functions (`Vector`, `print`, `any_to_string`) are + // dispatched by monomorphization off their BARE template name, not the + // plain `resolveFuncByName` / `lazyLowerFunction` path that trips the + // collision assert (issue 0100); registering a qualified alias for them + // would divert that machinery and strand a per-call type binding. + if (fd.type_params.len > 0 or hasComptimeParams(fd) or isPackFn(fd)) return; + // Foreign / builtin / #compiler bodies keep their literal name; a + // qualified alias has no distinct symbol to resolve to. + switch (fd.body.data) { + .foreign_expr, .builtin_expr, .compiler_expr => return, + else => {}, + } + const qualified = std.fmt.allocPrint(self.alloc, "{s}.{s}", .{ ns_name, short }) catch return; + if (self.program_index.fn_ast_map.contains(qualified)) return; + self.program_index.fn_ast_map.put(qualified, fd) catch {}; + self.program_index.import_flags.put(qualified, true) catch {}; + // No eager `declareFunction` here: the extern stub's param/return types + // would be resolved now, before the forward-alias fixpoint, caching an + // `.unresolved` for any type declared later in the module. The qualified + // function is declared + lowered on demand by `lazyLowerFunction`'s + // null-FuncId path (`lowerFunction`), which runs after all types resolve. + } + /// Check if a C-imported function is visible from the current source file. /// Returns true for non-C functions (always visible) or if no scoping info available. fn isCImportVisible(self: *Lowering, fn_name: []const u8) bool {