fix(imports): keep merged scope first-wins; index dups in module_fns only [0102a]

Attempt-1 retained a same-name cross-module FUNCTION author in the merged
decl list (mergeFlat + the directory merge), which is the list the existing
first-wins resolver consumes. That changed the data feeding resolution
(`mod.decls` carried two `greet`), violating this step's purpose: additive
indexes with ZERO resolution change.

Revert both merge sites to byte-for-byte first-wins, exactly as on
wt-fix-0102-base. The dropped same-name author is still retained — but only
in the SEPARATE `module_fns` index, which is built from each module's
`own_decls` (un-deduped, per-path) and which nothing reads yet. The
`flat_import_graph` side data is likewise untouched. Both are foundation
for fix-0102c's bare-name disambiguation; current resolution is unchanged.

Drop the now-unused `declAuthorsFn` helper (its only callers were the two
merge sites). `fnDeclOf` stays — it feeds the index.

Tests: the existing unit test now asserts the merged scope stays first-wins
(one `greet`, a.sx's author) while `module_fns` still retains BOTH authors
and `flat_import_graph` excludes the namespaced edge. Add a mixed non-fn/fn
collision test asserting the merged scope keeps a.sx's struct (first-wins),
unchanged by the function author.
This commit is contained in:
agra
2026-06-06 11:53:16 +03:00
parent ff9cb50079
commit 3dbc6f8434
2 changed files with 93 additions and 38 deletions

View File

@@ -14,8 +14,10 @@ fn testIo() std.Io {
}
// Two flat-imported modules each author `greet`; a third is namespaced. The
// step retains BOTH `greet` authors under their own paths in `module_fns`, and
// records the namespaced import in `import_graph` but NOT in `flat_import_graph`.
// step retains BOTH `greet` authors under their own paths in `module_fns` and
// records the namespaced import in `import_graph` but NOT in `flat_import_graph`
// — WITHOUT touching the merged scope: `mod.decls` stays byte-for-byte
// first-wins (one `greet`, a.sx's), exactly as on `wt-fix-0102-base`.
test "imports: module_fns retains same-name cross-module fns; flat_import_graph excludes namespaced edge" {
var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
defer arena.deinit();
@@ -76,22 +78,29 @@ test "imports: module_fns retains same-name cross-module fns; flat_import_graph
var module_fns = imports.ModuleFns.init(alloc);
try imports.buildModuleFns(alloc, main_path, mod, &cache, &module_fns);
// mergeFlat no longer first-wins-drops the second `greet`: the global flat
// decl list the lowering pass walks now carries BOTH authors.
// The MERGED scope the first-wins resolver consumes is unchanged: mergeFlat
// still drops the second `greet`, so `mod.decls` carries exactly ONE — and
// it is a.sx's author (the first flat import), not b.sx's.
var greet_count: usize = 0;
var merged_greet: ?*const ast.FnDecl = null;
for (mod.decls) |decl| {
const name = decl.data.declName() orelse continue;
if (std.mem.eql(u8, name, "greet")) greet_count += 1;
if (!std.mem.eql(u8, name, "greet")) continue;
greet_count += 1;
if (decl.data == .fn_decl) merged_greet = &decl.data.fn_decl;
}
try std.testing.expectEqual(@as(usize, 2), greet_count);
try std.testing.expectEqual(@as(usize, 1), greet_count);
// module_fns retains BOTH authors of `greet`, keyed by their own paths.
// module_fns retains BOTH authors of `greet`, keyed by their own paths
// the dropped author is recorded here (side index), not in the merged scope.
const a_fns = module_fns.get(a_path) orelse return error.MissingAFns;
const b_fns = module_fns.get(b_path) orelse return error.MissingBFns;
const a_greet = a_fns.get("greet") orelse return error.MissingAGreet;
const b_greet = b_fns.get("greet") orelse return error.MissingBGreet;
// Distinct authoring decls — not the same node deduped down to one.
try std.testing.expect(a_greet != b_greet);
// First-wins: the surviving merged-scope `greet` is a.sx's author.
try std.testing.expect(merged_greet == a_greet);
// flat_import_graph carries the two bare `#import` edges, NOT the
// namespaced `ns :: #import` edge.
@@ -107,3 +116,73 @@ test "imports: module_fns retains same-name cross-module fns; flat_import_graph
try std.testing.expect(full.contains(b_path));
try std.testing.expect(full.contains(ns_path));
}
// Mixed collision: a.sx authors `Widget` as a STRUCT (non-fn), b.sx authors it
// as a FUNCTION. fix-0102a must NOT let the function-author retention shift the
// merged scope — first-wins keeps a.sx's struct and drops b.sx's function,
// exactly as on `wt-fix-0102-base`. (The fn author may still be indexed in
// module_fns; resolution is what must be untouched.)
test "imports: mixed non-fn/fn same-name collision stays first-wins in merged scope" {
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 = "a.sx", .data = "Widget :: struct { x: s64 }\n" });
try tmp.dir.writeFile(io, .{ .sub_path = "b.sx", .data = "Widget :: () -> s64 { 7 }\n" });
const main_src =
\\#import "a.sx";
\\#import "b.sx";
\\main :: () -> s32 { 0 }
\\
;
try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = main_src });
var dirbuf: [4096]u8 = undefined;
const dirlen = try tmp.dir.realPath(io, &dirbuf);
const absdir = dirbuf[0..dirlen];
const main_path = try std.fmt.allocPrint(alloc, "{s}/main.sx", .{absdir});
const main_bytes = try std.Io.Dir.readFileAlloc(.cwd(), io, main_path, alloc, .limited(1 << 20));
const main_source = try alloc.dupeZ(u8, main_bytes);
var p = parser.Parser.init(alloc, main_source);
const root = p.parse() catch return error.ParseFailed;
var chain = std.StringHashMap(void).init(alloc);
var cache = imports.ModuleCache.init(alloc);
var import_graph = std.StringHashMap(std.StringHashMap(void)).init(alloc);
var flat_import_graph = std.StringHashMap(std.StringHashMap(void)).init(alloc);
const stdlib_paths = [_][]const u8{};
const mod = try imports.resolveImports(
alloc,
io,
root,
absdir,
main_path,
&chain,
&cache,
null,
null,
&stdlib_paths,
&import_graph,
&flat_import_graph,
.{},
);
// Exactly ONE `Widget` survives the merged scope, and it is a.sx's STRUCT —
// the function author did not displace or duplicate it.
var widget_count: usize = 0;
var merged_is_struct = false;
for (mod.decls) |decl| {
const name = decl.data.declName() orelse continue;
if (!std.mem.eql(u8, name, "Widget")) continue;
widget_count += 1;
merged_is_struct = decl.data == .struct_decl;
}
try std.testing.expectEqual(@as(usize, 1), widget_count);
try std.testing.expect(merged_is_struct);
}

View File

@@ -5,24 +5,10 @@ const errors = @import("errors.zig");
const c_import = @import("c_import.zig");
const Node = ast.Node;
/// True iff `decl` authors a top-level FUNCTION — either a bare `fn_decl`
/// (`f :: (…) -> T { … }`) or a `const_decl` whose value is a function
/// (`f :: (…) { … }` parsed as a const-bound fn). Used by the flat / directory
/// merge to retain a same-name function authored by a DIFFERENT module instead
/// of first-wins dropping it (fix-0102a): a flat importer that pulls two modules
/// each authoring `f` needs BOTH decls reachable downstream. Non-function decls
/// keep first-wins dedup.
fn declAuthorsFn(decl: *const Node) bool {
return switch (decl.data) {
.fn_decl => true,
.const_decl => |cd| cd.value.data == .fn_decl,
else => false,
};
}
/// The `*const ast.FnDecl` a function-authoring decl carries, or null when the
/// decl is not a function (mirrors `declAuthorsFn`). Drives the per-module
/// `module_fns` identity index (fix-0102a).
/// decl is not a function — either a bare `fn_decl` (`f :: (…) -> T { … }`) or a
/// `const_decl` whose value is a function. Drives the per-module `module_fns`
/// identity index (fix-0102a).
fn fnDeclOf(decl: *const Node) ?*const ast.FnDecl {
return switch (decl.data) {
.fn_decl => &decl.data.fn_decl,
@@ -360,13 +346,8 @@ pub const ResolvedModule = struct {
for (other.decls) |decl| {
if (seen_nodes.contains(decl)) continue;
if (decl.data.declName()) |name| {
if (seen_list.contains(name)) {
// First-wins dedup for non-functions; retain a same-name
// FUNCTION authored by a different module (fix-0102a).
if (!declAuthorsFn(decl)) continue;
} else {
try seen_list.put(name, {});
}
if (seen_list.contains(name)) continue;
try seen_list.put(name, {});
}
try seen_nodes.put(decl, {});
try list.append(allocator, decl);
@@ -787,13 +768,8 @@ fn resolveDirectoryImport(
for (file_mod.decls) |decl| {
if (seen_nodes.contains(decl)) continue;
if (decl.data.declName()) |name| {
if (seen_in_list.contains(name)) {
// First-wins dedup for non-functions; retain a same-name
// FUNCTION authored by a different file (fix-0102a).
if (!declAuthorsFn(decl)) continue;
} else {
try seen_in_list.put(name, {});
}
if (seen_in_list.contains(name)) continue;
try seen_in_list.put(name, {});
}
try seen_nodes.put(decl, {});
try decl_list.append(allocator, decl);