lab/sx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(lower): resolve cross-module same-name functions by identity [0100]

Browse Source 
Two modules each exporting a top-level function with the same short name
(std.cli.parse 3-param, std.json.parse 2-param) collided in IR lowering's
bare-name function table. fn_ast_map (name -> AST) was last-wins while
module.functions / resolveFuncByName are first-wins, so importing both and
calling one bound one function's AST against the other's FuncId and tripped
lazyLowerFunction's param-count assert (lower.zig:1606) — reached
unreachable code.

Fix:
- Register a namespaced import's OWN plain functions under their qualified
  name (ns.fn) in fn_ast_map, giving cli.parse / json.parse independent
  identities. The qualified resolution paths in CallResolver.plan /
  lowerCall already prefer ns.fn. NamespaceDecl now carries own_decls
  (populated in imports.addNamespace). Generic/comptime/pack/foreign
  functions are excluded (they dispatch by monomorphization off the bare
  template name); no eager declareFunction (it would resolve types before
  the forward-alias fixpoint).
- Make scanDecls' bare fn_ast_map registration first-wins so a later
  namespace recursion cannot clobber an earlier (flat) entry, aligning it
  with mergeFlat / resolveFuncByName.

Regression: examples/0719-modules-cli-and-json.sx imports both std.cli and
std.json under distinct namespaces and calls both parses; panics pre-fix,
passes after. issues/0100 marked RESOLVED.
This commit is contained in:
agra
2026-06-06 02:30:19 +03:00
parent fb3fdaf454
commit 3edc67521b
8 changed files with 244 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
examples/0719-modules-cli-and-json.sx Normal file
View File

@@ -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 <group> <command> + 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;
}

1
examples/expected/0719-modules-cli-and-json.exit Normal file
View File

@@ -0,0 +1 @@
0

1
examples/expected/0719-modules-cli-and-json.stderr Normal file
View File

@@ -0,0 +1 @@

10
examples/expected/0719-modules-cli-and-json.stdout Normal file
View File

@@ -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 ===

93
issues/0100-cross-module-same-name-fn-lowering-collision.md Normal file
View File

@@ -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`.

6
src/ast.zig
View File

@@ -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,

10
src/imports.zig
View File

@@ -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,
} },
};

72
src/ir/lower.zig
View File

@@ -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 {