diff --git a/issues/0106-namespaced-import-bare-visibility-over-permissive.md b/issues/0106-namespaced-import-bare-visibility-over-permissive.md new file mode 100644 index 0000000..602c5df --- /dev/null +++ b/issues/0106-namespaced-import-bare-visibility-over-permissive.md @@ -0,0 +1,113 @@ +# 0106 — namespaced-import internal names are silently bare-visible (over-permissive `isNameVisible`) + +**Symptom.** A bare reference to a top-level name authored in a module that the +consumer imports **only namespaced** (`ns :: #import "m.sx"`) is silently +visible from the consumer. Observed: it compiles + runs. Expected: an error — +the name is reachable only as `ns.name`. Root: `Lowering.isNameVisible` walks +`program_index.import_graph`, which records BOTH flat (`#import`) and namespaced +(`ns :: #import`) edges; bare-name visibility should join only over FLAT edges +(`flat_import_graph`). This is the latent 0102-family visibility bug the Phase B +caller-mode audit (unified-resolver R5) was told to surface. + +This **directly gates flow `stdlib/B`**: that step requires migrating +`isNameVisible`/`isCImportVisible` to the resolver's `user_bare_flat`/ +`c_import_bare` modes (which walk `flat_import_graph`) **byte-identically**. +Switching the edge set drops `run_examples` from 471 → 467 (see face #2), so the +byte-identical requirement cannot hold until this bug is fixed. + +## Reproduction — face #1 (user-facing over-permissiveness) + +```sx +// m.sx +secret :: () -> s64 { 7 } +``` + +```sx +// main.sx +m :: #import "m.sx"; +main :: () -> s32 { + x := secret(); // bare; `secret` is only namespaced-imported as `m.secret` + 0 +} +``` + +- **Observed** (current master): compiles, runs, exit `0` — bare `secret` + wrongly resolves to `m`'s author. +- **Expected**: `error: 'secret' is not visible; #import the module that + declares it` (it is reachable only as `m.secret`). + +(With the Phase-B edge-set change applied — `isNameVisible` over +`flat_import_graph` — this repro correctly errors, confirming the diagnosis.) + +## Reproduction — face #2 (library comptime entanglement: why a naive fix breaks std) + +```sx +// main.sx +std :: #import "modules/std.sx"; +main :: () -> s32 { + std.print("hello\n"); // legit qualified call + 0 +} +``` + +`print` in `std.sx` is a comptime metaprogram: + +```sx +print :: ($fmt: string, ..$args) { + #insert build_format(fmt); // comptime call to a std-internal fn + #insert "out(result);"; // inserts a bare call to a std-internal fn +} +``` + +The comptime call to `build_format` and the inserted `out(result)` are bare +names **authored in std.sx**, but they are visibility-checked in the +**consumer's** `current_source_file` context (comptime / `#insert` expansion +happens at the call site). Today they pass only because `import_graph[consumer]` +contains the namespaced `std` edge. Tightening bare visibility to +`flat_import_graph` makes them error +(`'build_format' / 'out' is not visible`). The same shape breaks `log` (`emit`). +Affected examples when the edge set is switched to flat: +`0015-basic-demo`, `0700-modules-import`, `0718-modules-cli-exit-json`, +`1030-errors-log-and-comptime` (467/471). + +## Root cause (suspected area) + +- `Lowering.isNameVisible` / `isCImportVisible` — `src/ir/lower.zig` (~1768-1840 + after the Phase-B refactor; `visibleOverEdges` / `nameVisibleOverEdges`). The + cross-module join uses `import_graph` (flat **and** namespaced edges) where it + should use `flat_import_graph` for a bare name. +- Comptime / `#insert` expansion context: the inserted/comptime-evaluated bare + calls of a namespaced module's function are policed against the **consumer's** + imports, not the **defining module's** own scope. The existing visibility + check already exempts UFCS-alias rewrites and mangled local names as "compiler + indirections" (`lower.zig` call site ~7284, identifier site ~3237); inserted / + comptime-generated bare calls are the same kind of indirection and are not + yet exempt — or, equivalently, the expansion should restore the defining + module's `current_source_file`. + +## Investigation prompt (paste into a fresh session) + +> Fix issue 0106: `isNameVisible` over-permits bare references to a +> namespaced-only import's internal names. Two coupled changes, in this order: +> +> 1. **Library-internal context.** Ensure a namespaced/comptime-expanded +> function's body — including `#insert`ed statements and comptime calls like +> `build_format` inside `std.print` — is visibility-checked in its **defining +> module's** context, OR exempt compiler-generated / `#insert`ed bare calls +> from the visibility check (mirror the UFCS-alias / mangled-name exemptions +> at `src/ir/lower.zig` ~7284 and ~3237). Verify with the face-#2 repro and +> examples `0015 / 0700 / 0718 / 1030`. +> 2. **Tighten bare visibility to flat.** Change `isNameVisible` (and the +> `c_import_bare` fall-through of `isCImportVisible`) to join over +> `flat_import_graph` instead of `import_graph` — i.e. route them through the +> resolver's `user_bare_flat` / `c_import_bare` modes (this is exactly Phase B +> of the unified-resolver R5 plan; `src/ir/resolver.zig` already defines the +> modes and `Lowering.visibleOverEdges` already takes a `.flat` / `.all` +> selector). Verify the face-#1 repro now errors. +> +> Acceptance: the face-#1 repro errors ("not visible"); `bash tests/run_examples.sh` +> is back to 471 `ok` with bare visibility on the flat edge set; add the face-#1 +> repro as a pinned regression (`issues/0106-…` with an `expected/` marker, or +> promote to `examples/07xx-modules-…`). Suspected files: `src/ir/lower.zig` +> (visibility + comptime/#insert expansion context), `src/ir/resolver.zig` +> (`user_bare_flat` / `c_import_bare`). diff --git a/src/imports.zig b/src/imports.zig index 98bca89..79fe4d7 100644 --- a/src/imports.zig +++ b/src/imports.zig @@ -509,8 +509,9 @@ pub const NamespaceEdges = std.StringHashMap(std.StringHashMap(NamespaceTarget)) /// The `RawDeclRef` a top-level node carries, or null when the node is not a /// selectable named declaration (e.g. `impl_block`, `var_decl`, `ufcs_alias`, -/// a flat `c_import_decl`). -fn rawDeclRefOf(decl: *const Node) ?RawDeclRef { +/// a flat `c_import_decl`). Public so the unified resolver's namespace collector +/// can classify a `NamespaceTarget.own_decls` node without re-deriving the map. +pub fn rawDeclRefOf(decl: *const Node) ?RawDeclRef { return switch (decl.data) { .fn_decl => |*d| .{ .fn_decl = d }, .const_decl => |*d| .{ .const_decl = d }, diff --git a/src/ir/ir.zig b/src/ir/ir.zig index ceb0000..fd39d88 100644 --- a/src/ir/ir.zig +++ b/src/ir/ir.zig @@ -5,6 +5,7 @@ pub const print = @import("print.zig"); pub const interp = @import("interp.zig"); pub const lower = @import("lower.zig"); pub const program_index = @import("program_index.zig"); +pub const resolver = @import("resolver.zig"); pub const type_resolver = @import("type_resolver.zig"); pub const packs = @import("packs.zig"); pub const expr_typer = @import("expr_typer.zig"); @@ -75,6 +76,7 @@ pub const print_tests = @import("print.test.zig"); pub const interp_tests = @import("interp.test.zig"); pub const lower_tests = @import("lower.test.zig"); pub const program_index_tests = @import("program_index.test.zig"); +pub const resolver_tests = @import("resolver.test.zig"); pub const type_resolver_tests = @import("type_resolver.test.zig"); pub const packs_tests = @import("packs.test.zig"); pub const expr_typer_tests = @import("expr_typer.test.zig"); diff --git a/src/ir/lower.zig b/src/ir/lower.zig index ed02032..28c1c6c 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -12,6 +12,7 @@ const interp_mod = @import("interp.zig"); const errors = @import("../errors.zig"); const jni_descriptor = @import("jni_descriptor.zig"); const program_index_mod = @import("program_index.zig"); +const resolver_mod = @import("resolver.zig"); const ProgramIndex = program_index_mod.ProgramIndex; const GlobalInfo = program_index_mod.GlobalInfo; const StructTemplate = program_index_mod.StructTemplate; @@ -110,6 +111,30 @@ const CleanupEntry = struct { binding: ?[]const u8 = null, }; +/// Pure non-transitive visibility walk: `name` is visible from `source` when +/// it's in `source`'s own scope or in any module reachable over one `graph` +/// edge. The core of the lowering visibility predicate, exposed so a unit test +/// can exercise the edge-walk without standing up a whole `Lowering`. Falls open +/// (true) when `scopes`/`graph` are null (scoping infra unwired). +pub fn nameVisibleOverEdges( + scopes: ?*std.StringHashMap(std.StringHashMap(void)), + graph: ?*std.StringHashMap(std.StringHashMap(void)), + source: []const u8, + name: []const u8, +) bool { + const sc = scopes orelse return true; + const own_scope = sc.get(source) orelse return true; + if (own_scope.contains(name)) return true; + const g = graph orelse return true; + const direct = g.get(source) orelse return true; + var it = direct.iterator(); + while (it.next()) |kv| { + const dep = sc.get(kv.key_ptr.*) orelse continue; + if (dep.contains(name)) return true; + } + return false; +} + // ── Lowering ──────────────────────────────────────────────────────────── pub const Lowering = struct { @@ -1765,45 +1790,71 @@ pub const Lowering = struct { // null-FuncId path (`lowerFunction`), which runs after all types resolve. } + /// The unified non-transitive `#import` visibility predicate, parameterized + /// by `VisibilityMode`. `isNameVisible` / `isCImportVisible` are thin + /// adapters over it. + /// + /// This is the lowering-side GATE: it walks `module_scopes` (the per-file + /// name set) joined over the edge set the mode selects. It is distinct from + /// `resolver.collectVisibleAuthors`, which collects raw AUTHORS over + /// `module_decls` — the single graph-walk that lives in `resolver.zig`. The + /// two read different facts (name set vs author refs) for different jobs, so + /// the gate's own iterator stays here, not in the resolver. + /// + /// `module_scopes[F]` holds ONLY the names authored in F (plus its namespace + /// aliases); cross-module visibility is joined here at query time. Doing the + /// join at lookup (instead of pre-merging in `resolveImports`) lets cyclic + /// imports like std.sx ↔ allocators.sx still resolve, since the cycle's + /// skipped edge is still recorded in the graph and the partner's scope is + /// filled in by the time lowering queries it. + fn isVisible(self: *Lowering, name: []const u8, vis: resolver_mod.VisibilityMode) bool { + switch (vis) { + // Registration / lazy lowering paths don't police user visibility. + .lowering_internal => return true, + // Transitive visibility is ProtocolResolver.findVisibleImpls' job; + // this predicate is single-hop only. + .impl_transitive => @panic("isVisible: transitive visibility is owned by findVisibleImpls"), + .c_import_bare => { + // Foreign-C gate: only C-import fn_decls without a library_ref + // are policed; a non-foreign body or a library-bound foreign + // decl is unconditionally visible. + const fd = self.program_index.fn_ast_map.get(name) orelse return true; + if (fd.body.data != .foreign_expr) return true; + if (fd.body.data.foreign_expr.library_ref != null) return true; + return self.visibleOverEdges(name, .flat); + }, + .user_bare_flat => return self.visibleOverEdges(name, .flat), + .legacy_direct_any => return self.visibleOverEdges(name, .all), + } + } + + const VisEdgeSet = enum { flat, all }; + + /// Resolve the mode's edge set and run the per-file visibility walk. Falls + /// open (visible) when the scoping infrastructure isn't wired (comptime + /// callers, directory imports without main_file, etc.). The caller is + /// responsible for restricting the check to names that ARE known top-level + /// decls; otherwise every local variable would be policed. + fn visibleOverEdges(self: *Lowering, name: []const u8, edges: VisEdgeSet) bool { + const source = self.current_source_file orelse return true; + const graph = switch (edges) { + .flat => self.program_index.flat_import_graph, + .all => self.program_index.import_graph, + }; + return nameVisibleOverEdges(self.program_index.module_scopes, graph, source, name); + } + /// 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. + /// Returns true for non-C functions (always visible) or if no scoping info + /// available. Byte-identical adapter over `isVisible`. fn isCImportVisible(self: *Lowering, fn_name: []const u8) bool { - const fd = self.program_index.fn_ast_map.get(fn_name) orelse return true; - // Only restrict C import fn_decls: foreign_expr with no library_ref - if (fd.body.data != .foreign_expr) return true; - if (fd.body.data.foreign_expr.library_ref != null) return true; - return self.isNameVisible(fn_name); + return self.isVisible(fn_name, .c_import_bare); } /// Non-transitive `#import` visibility check for top-level decls. - /// - /// `module_scopes[F]` holds ONLY the names authored in file F (plus its - /// namespace aliases). Cross-module visibility is joined here at query - /// time by walking each direct flat-import edge in `import_graph` — a - /// name is visible from F when it's authored in F or in any module F - /// directly `#import`s. Doing the join here (instead of pre-merging in - /// `resolveImports`) lets cyclic imports like std.sx ↔ allocators.sx - /// still resolve, since the cycle's skipped edge is still recorded in - /// `import_graph` and the partner's scope is filled in by the time - /// lowering queries it. - /// - /// Falls open when the scoping infrastructure isn't wired (comptime - /// callers, directory imports without main_file, etc.). The caller is - /// responsible for restricting the call to names that ARE known - /// top-level decls; otherwise every local variable would be policed. + /// Byte-identical adapter over `isVisible`. fn isNameVisible(self: *Lowering, name: []const u8) bool { - const scopes = self.program_index.module_scopes orelse return true; - const source = self.current_source_file orelse return true; - const own_scope = scopes.get(source) orelse return true; - if (own_scope.contains(name)) return true; - const graph = self.program_index.import_graph orelse return true; - const direct = graph.get(source) orelse return true; - var it = direct.iterator(); - while (it.next()) |kv| { - const dep = scopes.get(kv.key_ptr.*) orelse continue; - if (dep.contains(name)) return true; - } - return false; + return self.isVisible(name, .user_bare_flat); } /// Lazily lower a function body on demand. Called when lowerCall can't find diff --git a/src/ir/resolver.test.zig b/src/ir/resolver.test.zig new file mode 100644 index 0000000..e3618cd --- /dev/null +++ b/src/ir/resolver.test.zig @@ -0,0 +1,288 @@ +// Tests for resolver.zig — the shared author-collection layer (Phase B). +// +// collectVisibleAuthors is exercised over REAL Phase A facts (parse → +// resolveImports → buildImportFacts, the exact path core.zig drives) plus one +// synthetic diamond fixture for pointer-identity dedup. The visibility-adapter +// tests pin the nameVisibleOverEdges edge-walk that isNameVisible / +// isCImportVisible run on top of — including the user_bare_flat vs the +// over-permissive legacy_direct_any distinction. + +const std = @import("std"); +const ast = @import("../ast.zig"); +const parser = @import("../parser.zig"); +const imports = @import("../imports.zig"); +const errors = @import("../errors.zig"); +const resolver = @import("resolver.zig"); +const lower = @import("lower.zig"); +const pi = @import("program_index.zig"); +const ProgramIndex = pi.ProgramIndex; + +var g_test_threaded: ?std.Io.Threaded = null; +fn testIo() std.Io { + if (g_test_threaded == null) { + g_test_threaded = std.Io.Threaded.init(std.heap.page_allocator, .{}); + } + return g_test_threaded.?.io(); +} + +const Graph = std.StringHashMap(std.StringHashMap(void)); + +/// Parse `main_path`, resolve its imports, build the raw facts, and ALSO keep +/// the import / flat-import graphs (the collectors need them). `alloc` must be +/// an arena that outlives the returned views. +const Facts = struct { + decls: imports.ModuleDecls, + ns_edges: imports.NamespaceEdges, + import_graph: Graph, + flat_import_graph: Graph, +}; + +fn buildFacts(alloc: std.mem.Allocator, io: std.Io, absdir: []const u8, main_path: []const u8) !Facts { + 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 diags = errors.DiagnosticList.init(alloc, main_source, main_path); + var chain = std.StringHashMap(void).init(alloc); + var cache = imports.ModuleCache.init(alloc); + var import_graph = Graph.init(alloc); + var flat_import_graph = Graph.init(alloc); + const stdlib_paths = [_][]const u8{}; + + const mod = try imports.resolveImports( + alloc, + io, + root, + absdir, + main_path, + &chain, + &cache, + null, + &diags, + &stdlib_paths, + &import_graph, + &flat_import_graph, + .{}, + ); + + const facts = try imports.buildImportFacts(alloc, main_path, mod, &cache); + return .{ + .decls = facts.decls, + .ns_edges = facts.ns_edges, + .import_graph = import_graph, + .flat_import_graph = flat_import_graph, + }; +} + +fn tag(ref: resolver.RawDeclRef) std.meta.Tag(resolver.RawDeclRef) { + return std.meta.activeTag(ref); +} + +// ── collectVisibleAuthors ──────────────────────────────────────────────── + +// own author present; two distinct flat authors both returned RAW; and the +// user_bare_flat edge set EXCLUDES a namespaced-only import that the quarantined +// legacy_direct_any set still reaches. +test "resolver: collectVisibleAuthors — own author, two distinct flat authors, namespaced edge excluded" { + 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 = "dup :: () -> s64 { 1 }\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = "b.sx", .data = "dup :: () -> s64 { 2 }\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = "p.sx", .data = "secret :: () -> s64 { 9 }\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = "#import \"a.sx\";\n#import \"b.sx\";\ng :: #import \"p.sx\";\nselfauthored :: () -> s64 { 0 }\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); + + var idx = ProgramIndex.init(alloc); + defer idx.deinit(); + idx.module_decls = &facts.decls; + idx.flat_import_graph = &facts.flat_import_graph; + idx.import_graph = &facts.import_graph; + + var r = resolver.Resolver.init(&idx, alloc); + + // Own author (declared in main itself). + const own_set = r.collectVisibleAuthors("selfauthored", main_path, .user_bare_flat); + try std.testing.expect(own_set.own != null); + try std.testing.expectEqualStrings(main_path, own_set.own.?.source); + try std.testing.expectEqual(@as(usize, 0), own_set.flat.len); + try std.testing.expectEqual(@as(usize, 1), own_set.distinctCount()); + + // Two distinct flat authors of `dup` (a.sx and b.sx), returned raw. + const dup_set = r.collectVisibleAuthors("dup", main_path, .user_bare_flat); + try std.testing.expect(dup_set.own == null); + try std.testing.expectEqual(@as(usize, 2), dup_set.flat.len); + try std.testing.expectEqual(@as(usize, 2), dup_set.distinctCount()); + try std.testing.expectEqual(std.meta.Tag(resolver.RawDeclRef).fn_decl, tag(dup_set.flat[0].raw)); + try std.testing.expectEqual(std.meta.Tag(resolver.RawDeclRef).fn_decl, tag(dup_set.flat[1].raw)); + try std.testing.expect(dup_set.flat[0].raw.fn_decl != dup_set.flat[1].raw.fn_decl); + + // `secret` is authored only in p.sx, imported NAMESPACED (`g :: #import`). + // user_bare_flat must NOT see it (p.sx is not a flat edge)... + const flat_secret = r.collectVisibleAuthors("secret", main_path, .user_bare_flat); + try std.testing.expect(flat_secret.own == null); + try std.testing.expectEqual(@as(usize, 0), flat_secret.flat.len); + + // ...but the quarantined legacy_direct_any set (import_graph) still reaches + // it — the exact over-permissiveness user_bare_flat tightens away. + const any_secret = r.collectVisibleAuthors("secret", main_path, .legacy_direct_any); + try std.testing.expect(any_secret.own == null); + try std.testing.expectEqual(@as(usize, 1), any_secret.flat.len); +} + +// Diamond: the SAME author node is reachable over two flat edges. It must +// collapse to a single entry (dedup by author identity), not appear twice. +test "resolver: collectVisibleAuthors — diamond imports of one author dedup to one" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + // One real fn_decl node, shared between two module indices. + var body = ast.Node{ .span = .{ .start = 0, .end = 0 }, .data = .builtin_expr }; + var shared = ast.Node{ + .span = .{ .start = 0, .end = 0 }, + .data = .{ .fn_decl = .{ .name = "shared", .params = &.{}, .return_type = null, .body = &body } }, + }; + const ref = imports.rawDeclRefOf(&shared).?; + + var decls = imports.ModuleDecls.init(alloc); + inline for (.{ "p1", "p2" }) |path| { + var names = std.StringHashMap(resolver.RawDeclRef).init(alloc); + try names.put("shared", ref); + try decls.put(path, .{ .source = path, .names = names }); + } + + var flat = Graph.init(alloc); + var from_edges = std.StringHashMap(void).init(alloc); + try from_edges.put("p1", {}); + try from_edges.put("p2", {}); + try flat.put("from", from_edges); + + var idx = ProgramIndex.init(alloc); + defer idx.deinit(); + idx.module_decls = &decls; + idx.flat_import_graph = ♭ + + var r = resolver.Resolver.init(&idx, alloc); + const set = r.collectVisibleAuthors("shared", "from", .user_bare_flat); + try std.testing.expect(set.own == null); + try std.testing.expectEqual(@as(usize, 1), set.flat.len); + try std.testing.expectEqual(@as(usize, 1), set.distinctCount()); + try std.testing.expectEqual(@intFromPtr(&shared.data.fn_decl), @intFromPtr(set.flat[0].raw.fn_decl)); +} + +// ── collectNamespaceAuthors ────────────────────────────────────────────── + +// Returns a namespace target's members and touches NO graph: the Resolver here +// has no graphs (or module_decls) wired at all, yet the member is found. +test "resolver: collectNamespaceAuthors — returns target members, walks no graph" { + 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 = "point.sx", .data = "Point :: struct { x: s64 }\nhelper :: () -> s64 { 0 }\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = "g :: #import \"point.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}); + const point_path = try std.fmt.allocPrint(alloc, "{s}/point.sx", .{absdir}); + + var facts = try buildFacts(alloc, io, absdir, main_path); + + const aliases = facts.ns_edges.get(main_path) orelse return error.MissingNsEdges; + const target = aliases.get("g") orelse return error.MissingAlias; + try std.testing.expectEqualStrings(point_path, target.target_module_path); + + // A Resolver over an EMPTY index — no module_decls, no graphs. If + // collectNamespaceAuthors touched a graph it would crash / miss; it doesn't. + var idx = ProgramIndex.init(alloc); + defer idx.deinit(); + try std.testing.expect(idx.flat_import_graph == null); + try std.testing.expect(idx.import_graph == null); + var r = resolver.Resolver.init(&idx, alloc); + + const pt = r.collectNamespaceAuthors(target, "Point"); + try std.testing.expect(pt.own != null); + try std.testing.expectEqual(std.meta.Tag(resolver.RawDeclRef).struct_decl, tag(pt.own.?.raw)); + try std.testing.expectEqualStrings(point_path, pt.own.?.source); + try std.testing.expectEqual(@as(usize, 0), pt.flat.len); + + const hp = r.collectNamespaceAuthors(target, "helper"); + try std.testing.expect(hp.own != null); + try std.testing.expectEqual(std.meta.Tag(resolver.RawDeclRef).fn_decl, tag(hp.own.?.raw)); + + const miss = r.collectNamespaceAuthors(target, "Missing"); + try std.testing.expect(miss.own == null); + try std.testing.expectEqual(@as(usize, 0), miss.distinctCount()); +} + +// ── visibility predicate (the isNameVisible / isCImportVisible core) ────── + +// nameVisibleOverEdges is what isVisible(.user_bare_flat) (=> .flat graph) and +// the quarantined legacy_direct_any (=> import_graph) run on. They agree on own +// + flat names and differ ONLY on a namespaced-only name — the byte-identical +// behavior the adapters preserve vs the over-permissive set they avoid. +test "resolver: visibility edge-walk — own + flat visible; namespaced-only only under import_graph" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var scopes = Graph.init(alloc); + inline for (.{ + .{ "main", &[_][]const u8{ "selfauthored", "g" } }, + .{ "a", &[_][]const u8{"dup"} }, + .{ "p", &[_][]const u8{"secret"} }, + }) |entry| { + var s = std.StringHashMap(void).init(alloc); + for (entry[1]) |n| try s.put(n, {}); + try scopes.put(entry[0], s); + } + + // Flat graph: main flat-imports a only. Import graph: main reaches a + p. + var flat = Graph.init(alloc); + var flat_edges = std.StringHashMap(void).init(alloc); + try flat_edges.put("a", {}); + try flat.put("main", flat_edges); + + var all = Graph.init(alloc); + var all_edges = std.StringHashMap(void).init(alloc); + try all_edges.put("a", {}); + try all_edges.put("p", {}); + try all.put("main", all_edges); + + // Own-scope name: visible regardless of edge set. + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, &flat, "main", "selfauthored")); + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, &all, "main", "selfauthored")); + + // Flat-imported name: visible under both (the flat edge is in both graphs). + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, &flat, "main", "dup")); + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, &all, "main", "dup")); + + // Namespaced-only name: NOT visible under the flat set (user_bare_flat), + // but visible under the import_graph set (legacy_direct_any). + try std.testing.expect(!lower.nameVisibleOverEdges(&scopes, &flat, "main", "secret")); + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, &all, "main", "secret")); + + // Unknown name: not visible. + try std.testing.expect(!lower.nameVisibleOverEdges(&scopes, &flat, "main", "nope")); + + // Falls open when scoping infra is unwired (null scopes/graph). + try std.testing.expect(lower.nameVisibleOverEdges(null, &flat, "main", "secret")); + try std.testing.expect(lower.nameVisibleOverEdges(&scopes, null, "main", "secret")); +} diff --git a/src/ir/resolver.zig b/src/ir/resolver.zig new file mode 100644 index 0000000..d7c0c7e --- /dev/null +++ b/src/ir/resolver.zig @@ -0,0 +1,178 @@ +//! The unified sx name/type resolver — the shared author-collection layer. +//! +//! A read-only facade over the borrowed Phase A import facts on a +//! `*ProgramIndex` (`module_decls` / `namespace_edges`) and the existing +//! `import_graph` / `flat_import_graph` views. It OWNS nothing import-derived; +//! those maps live in `imports.zig`/`core.zig` and are borrowed here, exactly +//! like `module_fns`. +//! +//! Two collectors sit on top of these facts (R5 §1 #1): +//! - `collectVisibleAuthors` — own author ∪ the flat-import edge walk. THE one +//! graph-walk; the permanent flat-import F-series root. +//! - `collectNamespaceAuthors` — a single already-selected namespace target's +//! members. NO graph walk. +//! +//! Both are RAW and verdict-free: they return who authors a name, not which +//! author wins. Per-domain selectors (Phase C+) decide eligibility. Nothing +//! routes resolution through these collectors yet. +//! +//! Falsifiable invariant (R5 §1 #1): there is EXACTLY ONE iterator over +//! `flat_import_graph`/`import_graph` in this file — inside +//! `collectVisibleAuthors`. `collectNamespaceAuthors` iterates one +//! `NamespaceTarget.own_decls` slice and touches no graph. This is what keeps +//! 0102 (callable) and 0105 (type) the SAME cross-module edge-walk. + +const std = @import("std"); +const ast = @import("../ast.zig"); +const imports = @import("../imports.zig"); +const program_index = @import("program_index.zig"); +const ProgramIndex = program_index.ProgramIndex; + +// ── Raw-fact aliases (defined in imports.zig by buildImportFacts, Phase A) ── +pub const RawDeclRef = imports.RawDeclRef; +pub const RawAuthor = imports.RawAuthor; +pub const NamespaceTarget = imports.NamespaceTarget; + +/// Author multiplicity for ONE name as seen from ONE querying module: the +/// own-module author (tier-2) plus the distinct flat-import authors (tier-3), +/// diamond-deduped by author identity. RAW — no verdict, no domain, no pick. +pub const AuthorSet = struct { + /// The author declared in the querying module itself, if any. + own: ?RawAuthor, + /// Distinct flat-import authors. Diamond imports of the SAME author (same + /// AST node reached over two edges, e.g. a directory aggregate and one of + /// its member files) collapse to a single entry. Always disjoint from `own`. + flat: []const RawAuthor, + + /// own + flat, counted by author identity. `flat` is already deduped and + /// disjoint from `own`, so this is a plain sum. + pub fn distinctCount(self: AuthorSet) usize { + return (if (self.own != null) @as(usize, 1) else 0) + self.flat.len; + } +}; + +/// How a name's cross-module visibility is computed. The author collector and +/// the lowering-side visibility predicate (`Lowering.isVisible`) both switch on +/// this single vocabulary. +pub const VisibilityMode = enum { + /// own scope ∪ `flat_import_graph`. The PERMANENT core for bare-name lookup + /// under flat imports (Agra constraint) — never a transitional path. + user_bare_flat, + /// `user_bare_flat` plus the foreign-C gate (today's `isCImportVisible`): + /// only C-import `fn_decl`s without a `library_ref` are policed; everything + /// else is unconditionally visible. + c_import_bare, + /// own scope ∪ the TRANSITIVE import relation (specs.md:793-801). Owned by + /// `ProtocolResolver.findVisibleImpls`; the single-hop author collector + /// never serves it. + impl_transitive, + /// Registration / lazy lowering: falls open (visible), emits no user + /// diagnostic, performs no graph walk. + lowering_internal, + /// own scope ∪ `import_graph` (flat AND namespaced edges) — an + /// over-permissive set. QUARANTINE: reserved for sites PROVEN to be internal + /// scans, never a user-facing lookup. Deleted in Phase K. + legacy_direct_any, +}; + +/// Read-only facade over the borrowed import facts. `alloc` backs the +/// `AuthorSet.flat` slices the collectors return (the caller owns + frees them). +pub const Resolver = struct { + index: *ProgramIndex, + alloc: std.mem.Allocator, + + pub fn init(index: *ProgramIndex, alloc: std.mem.Allocator) Resolver { + return .{ .index = index, .alloc = alloc }; + } + + /// THE single graph-walk in this file (falsifiable invariant, R5 §1 #1): + /// the own author declared in `from` ∪ the flat-import authors reachable + /// over the edge set `vis` chooses. RAW — selectors decide eligibility, not + /// this. `from` is the querying module's source path. + /// + /// Edge set by mode: `flat_import_graph` for `user_bare_flat`/ + /// `c_import_bare`; `import_graph` for the quarantined `legacy_direct_any`. + /// `impl_transitive` (a transitive closure owned by `findVisibleImpls`) and + /// `lowering_internal` (no graph walk) are not single-hop author walks — + /// reaching them here is a wiring bug, so we trip loudly. + pub fn collectVisibleAuthors( + self: *Resolver, + name: []const u8, + from: []const u8, + vis: VisibilityMode, + ) AuthorSet { + const decls = self.index.module_decls orelse return .{ .own = null, .flat = &.{} }; + + const own: ?RawAuthor = blk: { + const mod = decls.get(from) orelse break :blk null; + const ref = mod.names.get(name) orelse break :blk null; + break :blk .{ .raw = ref, .source = mod.source }; + }; + + const graph = (switch (vis) { + .user_bare_flat, .c_import_bare => self.index.flat_import_graph, + .legacy_direct_any => self.index.import_graph, + // findVisibleImpls owns transitive visibility; lowering_internal + // performs no graph walk. Neither selects a single-hop edge set. + .impl_transitive, .lowering_internal => @panic( + "collectVisibleAuthors: vis mode performs no single-hop author walk", + ), + }) orelse return .{ .own = own, .flat = &.{} }; + + const direct = graph.get(from) orelse return .{ .own = own, .flat = &.{} }; + + var flat = std.ArrayList(RawAuthor).empty; + var it = direct.iterator(); // ← the one graph iterator (invariant) + while (it.next()) |kv| { + const dep = decls.get(kv.key_ptr.*) orelse continue; + const ref = dep.names.get(name) orelse continue; + const cand = RawAuthor{ .raw = ref, .source = dep.source }; + if (sameAuthor(own, cand)) continue; // keep flat disjoint from own + if (containsAuthor(flat.items, cand)) continue; // diamond dedup + flat.append(self.alloc, cand) catch @panic("collectVisibleAuthors: OOM"); + } + return .{ + .own = own, + .flat = flat.toOwnedSlice(self.alloc) catch @panic("collectVisibleAuthors: OOM"), + }; + } + + /// Container collector for ONE already-selected namespace target. Iterates + /// the target's `own_decls` and touches NO import graph (R5 §1 #1). A + /// namespace's `own_decls` is name-deduped, so a name has at most one author + /// here — returned as `own`, sourced to the target's module path. + pub fn collectNamespaceAuthors( + self: *Resolver, + target: NamespaceTarget, + name: []const u8, + ) AuthorSet { + _ = self; + for (target.own_decls) |decl| { + const dn = decl.data.declName() orelse continue; + if (!std.mem.eql(u8, dn, name)) continue; + const ref = imports.rawDeclRefOf(decl) orelse continue; + return .{ .own = .{ .raw = ref, .source = target.target_module_path }, .flat = &.{} }; + } + return .{ .own = null, .flat = &.{} }; + } +}; + +/// Author identity is the AST node pointer the `RawDeclRef` wraps; every variant +/// holds a pointer, so a single `inline else` extracts it. +fn authorNodePtr(ref: RawDeclRef) usize { + return switch (ref) { + inline else => |p| @intFromPtr(p), + }; +} + +fn sameAuthor(a: ?RawAuthor, b: RawAuthor) bool { + const aa = a orelse return false; + return authorNodePtr(aa.raw) == authorNodePtr(b.raw); +} + +fn containsAuthor(list: []const RawAuthor, b: RawAuthor) bool { + for (list) |x| { + if (authorNodePtr(x.raw) == authorNodePtr(b.raw)) return true; + } + return false; +}