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

fix(imports): diagnose namespace-alias dup + propagate buildImportFacts errors [stdlib A attempt-2]

Browse Source 
Two defects from the Phase A attempt-1 review.

F1 — duplicate-name diagnostic missed NAMESPACE ALIASES (silent first-win).
`addNamespace` unconditionally put the alias into scope/own_decls, so a
same-module collision between an authored decl and a `dup :: #import "…"`
alias compiled clean in the fn-then-alias order (the scalar
ModuleRawDeclIndex silently first-won). Now `addNamespace` returns a bool
and refuses a same-module duplicate (mirroring addOwnDecl); the call site
surfaces it via the new `reportDuplicateName` (the import_decl node has no
declName, so the alias name is passed explicitly). The C-import namespace
site gets the same guard. Both orders now emit "duplicate top-level
declaration 'X'" and exit nonzero (alias-then-fn was already caught by
addOwnDecl seeing the alias in scope).

F2 — buildImportFacts errors were swallowed by `else |_| {}` in core.zig
(REJECTED-PATTERN catch-all leaving the borrowed store silently empty).
`resolveImports` returns !void, so the call is now a plain `try` and a
build failure propagates instead of producing a stale/empty store.

Tests: extend the dup-name regression with fn-vs-namespace-alias
collisions in both orders. No resolution behavior change (no lower.zig
edits; run_examples 471 byte-identical); m3te ios-sim builds via the
worktree binary.
This commit is contained in:
agra
2026-06-06 23:54:51 +03:00
parent b5ec121645
commit 5f06b6504f
3 changed files with 95 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/core.zig
View File

@@ -136,11 +136,12 @@ pub const Compilation = struct {
// Raw import facts (the unified-resolver store): scalar per-module
// raw-decl index + namespace edges, built from the SAME modules. Nothing
// consumes these yet — they are borrowed by `ProgramIndex` for later
// phases (and the LSP). Built without IR lowering.
if (imports.buildImportFacts(self.allocator, self.file_path, mod, &cache)) |facts| {
self.module_decls = facts.decls;
self.namespace_edges = facts.ns_edges;
} else |_| {}
// phases (and the LSP). Built without IR lowering. A build failure here
// (allocation) is the Phase A deliverable failing — propagate it rather
// than leaving the borrowed views silently empty/stale.
const facts = try imports.buildImportFacts(self.allocator, self.file_path, mod, &cache);
self.module_decls = facts.decls;
self.namespace_edges = facts.ns_edges;
// Store main file source in import_sources so error reporting can find it
self.import_sources.put(self.file_path, self.source) catch {};

54
src/imports.test.zig
View File

@@ -449,3 +449,57 @@ test "buildImportFacts: same-module duplicate top-level name is diagnosed" {
const m_idx = facts.decls.get(main_path) orelse return error.MissingMainIndex;
try expectTag(m_idx.names.get("foo") orelse return error.MissingFoo, .fn_decl);
}
// F1: the duplicate-name invariant must also cover NAMESPACE ALIASES. A
// `dup :: #import "…"` alias colliding with a same-module authored name is a
// duplicate in EITHER order — `addNamespace` (alias second) and `addOwnDecl`
// (alias first) each refuse the second author and the site diagnoses it. Before
// the fix the fn-then-alias order compiled clean (silent first-win in the scalar
// index). Surviving author is whichever came FIRST.
test "buildImportFacts: fn-then-namespace-alias same-module collision is diagnosed" {
var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
const io = testIo();
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.writeFile(io, .{ .sub_path = "lib.sx", .data = "helper :: () -> s64 { 9 }\n" });
try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = "dup :: () -> s64 { 1 }\ndup :: #import \"lib.sx\";\nmain :: () -> s32 { 0 }\n" });
var dirbuf: [4096]u8 = undefined;
const absdir = dirbuf[0..try tmp.dir.realPath(io, &dirbuf)];
const main_path = try std.fmt.allocPrint(alloc, "{s}/main.sx", .{absdir});
var facts = try buildFacts(alloc, io, absdir, main_path);
try std.testing.expect(hasErr(&facts.diags, "duplicate top-level declaration 'dup'"));
// The fn came first, so it survives in the scalar index; the alias dropped.
const m_idx = facts.decls.get(main_path) orelse return error.MissingMainIndex;
try expectTag(m_idx.names.get("dup") orelse return error.MissingDup, .fn_decl);
}
test "buildImportFacts: namespace-alias-then-fn same-module collision is diagnosed" {
var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
const io = testIo();
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.writeFile(io, .{ .sub_path = "lib.sx", .data = "helper :: () -> s64 { 9 }\n" });
try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = "dup :: #import \"lib.sx\";\ndup :: () -> s64 { 1 }\nmain :: () -> s32 { 0 }\n" });
var dirbuf: [4096]u8 = undefined;
const absdir = dirbuf[0..try tmp.dir.realPath(io, &dirbuf)];
const main_path = try std.fmt.allocPrint(alloc, "{s}/main.sx", .{absdir});
var facts = try buildFacts(alloc, io, absdir, main_path);
try std.testing.expect(hasErr(&facts.diags, "duplicate top-level declaration 'dup'"));
// The alias came first, so the namespace_decl survives; the fn dropped.
const m_idx = facts.decls.get(main_path) orelse return error.MissingMainIndex;
try expectTag(m_idx.names.get("dup") orelse return error.MissingDup, .namespace_decl);
}

47
src/imports.zig
View File

@@ -357,6 +357,13 @@ pub const ResolvedModule = struct {
/// Add another module as a namespaced import. The alias `name` becomes
/// part of this module's own decls (so a flat-importer of this module
/// sees the alias one hop out — matching authored names).
///
/// Returns `false` (and adds nothing) when `name` already names a decl
/// authored in THIS module — symmetric to `addOwnDecl`, so a namespace
/// alias colliding with a prior same-module name is dropped rather than
/// shadowing it. The caller surfaces the drop via `reportDuplicateName`;
/// the reverse order (alias first, then a same-name authored decl) is
/// caught by `addOwnDecl` seeing the alias already in `scope`.
pub fn addNamespace(
self: *ResolvedModule,
allocator: std.mem.Allocator,
@@ -367,7 +374,8 @@ pub const ResolvedModule = struct {
other: ResolvedModule,
span: ast.Span,
is_raw: bool,
) !void {
) !bool {
if (self.scope.contains(name)) return false;
const ns_node = try allocator.create(Node);
ns_node.* = .{
.span = span,
@@ -391,6 +399,7 @@ pub const ResolvedModule = struct {
try seen_list.put(name, {});
try list.append(allocator, ns_node);
try own_list.append(allocator, ns_node);
return true;
}
pub fn finalize(
@@ -570,15 +579,24 @@ pub fn buildImportFacts(
return .{ .decls = decls, .ns_edges = ns_edges };
}
/// Surface a same-module duplicate top-level declaration as a hard error.
/// `addOwnDecl` returns `false` when the name is already in this module's scope
/// and drops the second author; without this the drop is silent, and the scalar
/// `ModuleRawDeclIndex` would lose an authored name with no diagnostic.
fn reportDuplicateDecl(diagnostics: ?*errors.DiagnosticList, added: bool, decl: *const Node) void {
/// Surface a same-module duplicate top-level declaration as a hard error at an
/// explicit name + span. `addOwnDecl` / `addNamespace` return `false` when the
/// name is already in this module's scope and drop the second author; without
/// this the drop is silent, and the scalar `ModuleRawDeclIndex` would lose an
/// authored name with no diagnostic.
fn reportDuplicateName(diagnostics: ?*errors.DiagnosticList, added: bool, name: []const u8, span: ast.Span) void {
if (added) return;
const diags = diagnostics orelse return;
diags.addFmt(.err, span, "duplicate top-level declaration '{s}'", .{name});
}
/// `reportDuplicateName` keyed off a node whose `declName()` carries the name
/// (the regular authored-decl sites; an `import_decl` has no `declName`, so a
/// namespace alias must use `reportDuplicateName` with the alias directly).
fn reportDuplicateDecl(diagnostics: ?*errors.DiagnosticList, added: bool, decl: *const Node) void {
if (added) return;
const name = decl.data.declName() orelse return;
diags.addFmt(.err, decl.span, "duplicate top-level declaration '{s}'", .{name});
reportDuplicateName(diagnostics, added, name, decl.span);
}
pub fn resolveImports(
@@ -706,10 +724,14 @@ pub fn resolveImports(
} },
};
ns_node.source_file = file_path;
try mod.scope.put(ns_name, {});
try seen_in_list.put(ns_name, {});
try decl_list.append(allocator, ns_node);
try own_decl_list.append(allocator, ns_node);
if (mod.scope.contains(ns_name)) {
reportDuplicateName(diagnostics, false, ns_name, decl.span);
} else {
try mod.scope.put(ns_name, {});
try seen_in_list.put(ns_name, {});
try decl_list.append(allocator, ns_node);
try own_decl_list.append(allocator, ns_node);
}
} else {
// Flat: add fn_decls directly + keep c_import_decl
for (result.fn_decls) |fd| {
@@ -796,7 +818,8 @@ pub fn resolveImports(
};
if (imp.name) |ns_name| {
try mod.addNamespace(allocator, &decl_list, &own_decl_list, &seen_in_list, ns_name, imported_mod, decl.span, imp.is_raw);
const added = try mod.addNamespace(allocator, &decl_list, &own_decl_list, &seen_in_list, ns_name, imported_mod, decl.span, imp.is_raw);
reportDuplicateName(diagnostics, added, ns_name, decl.span);
} else {
try mod.mergeFlat(allocator, &decl_list, &seen_in_list, &seen_nodes, imported_mod);
}