From 69934592d8fd5b812c8552589a7005ac9f031969 Mon Sep 17 00:00:00 2001 From: agra Date: Fri, 6 Mar 2026 10:46:28 +0200 Subject: [PATCH] c import --- examples/issue-0019/c_wrapper.sx | 10 +++++++++ examples/issue-0019/main_bad.sx | 18 ++++++++++++++++ examples/issue-0019/main_good.sx | 10 +++++++++ examples/issue-0019/other.sx | 6 ++++++ src/c_import.zig | 35 +++++++++++++++++++----------- src/core.zig | 10 +++++++++ src/ir/inst.zig | 1 + src/ir/lower.zig | 37 ++++++++++++++++++++++++++++++-- 8 files changed, 113 insertions(+), 14 deletions(-) create mode 100644 examples/issue-0019/c_wrapper.sx create mode 100644 examples/issue-0019/main_bad.sx create mode 100644 examples/issue-0019/main_good.sx create mode 100644 examples/issue-0019/other.sx diff --git a/examples/issue-0019/c_wrapper.sx b/examples/issue-0019/c_wrapper.sx new file mode 100644 index 0000000..2e9a73d --- /dev/null +++ b/examples/issue-0019/c_wrapper.sx @@ -0,0 +1,10 @@ +// This module imports C functions and provides wrappers +#import c { + #include "vendors/test_c/test.h"; + #source "vendors/test_c/test.c"; +}; + +// Wrapper function that calls the C function +wrapped_add :: (a: s32, b: s32) -> s32 { + add_numbers(a, b); +} diff --git a/examples/issue-0019/main_bad.sx b/examples/issue-0019/main_bad.sx new file mode 100644 index 0000000..ff8a70e --- /dev/null +++ b/examples/issue-0019/main_bad.sx @@ -0,0 +1,18 @@ +// Test: other.sx calls C functions without importing the C module +// main imports both c_wrapper.sx and other.sx +// other.sx should NOT have access to C functions from c_wrapper +#import "../modules/std.sx"; +#import "c_wrapper.sx"; +#import "other.sx"; + +main :: () -> s32 { + // This works: we import c_wrapper so we have transitive access + result := wrapped_add(10, 20); + print("wrapped_add(10, 20) = {}\n", result); + + // This calls other.sx's function which tries to call add_numbers + // other.sx did NOT import c_wrapper.sx, so this should fail + bad := use_c_directly(); + print("use_c_directly() = {}\n", bad); + 0; +} diff --git a/examples/issue-0019/main_good.sx b/examples/issue-0019/main_good.sx new file mode 100644 index 0000000..89837ab --- /dev/null +++ b/examples/issue-0019/main_good.sx @@ -0,0 +1,10 @@ +// Test: calling wrapper functions works (we import the module) +#import "../modules/std.sx"; +#import "c_wrapper.sx"; + +main :: () -> s32 { + // This should work: calling the sx wrapper + result := wrapped_add(10, 20); + print("wrapped_add(10, 20) = {}\n", result); + 0; +} diff --git a/examples/issue-0019/other.sx b/examples/issue-0019/other.sx new file mode 100644 index 0000000..1cdb627 --- /dev/null +++ b/examples/issue-0019/other.sx @@ -0,0 +1,6 @@ +// This file does NOT import c_wrapper.sx +// It should NOT be able to call add_numbers + +use_c_directly :: () -> s32 { + add_numbers(5, 3); +} diff --git a/src/c_import.zig b/src/c_import.zig index 24990f7..f7232b4 100644 --- a/src/c_import.zig +++ b/src/c_import.zig @@ -382,21 +382,28 @@ pub fn writeCObjectFiles( } /// Walk the resolved AST and collect CImportInfo from all c_import_decl nodes. +/// Deduplicates by source pointer identity (shared nodes from import propagation). pub fn collectCImportSources(allocator: std.mem.Allocator, root: *const Node) ![]CImportInfo { if (root.data != .root) return &.{}; var infos = std.ArrayList(CImportInfo).empty; + var seen = std.AutoHashMap([*]const []const u8, void).init(allocator); + defer seen.deinit(); for (root.data.root.decls) |decl| { switch (decl.data) { .c_import_decl => |ci| { if (ci.sources.len > 0) { - try infos.append(allocator, .{ - .sources = ci.sources, - .includes = ci.includes, - .defines = ci.defines, - .flags = ci.flags, - }); + const key = ci.sources.ptr; + if (!seen.contains(key)) { + try seen.put(key, {}); + try infos.append(allocator, .{ + .sources = ci.sources, + .includes = ci.includes, + .defines = ci.defines, + .flags = ci.flags, + }); + } } }, .namespace_decl => |ns| { @@ -404,12 +411,16 @@ pub fn collectCImportSources(allocator: std.mem.Allocator, root: *const Node) ![ if (nd.data == .c_import_decl) { const nci = nd.data.c_import_decl; if (nci.sources.len > 0) { - try infos.append(allocator, .{ - .sources = nci.sources, - .includes = nci.includes, - .defines = nci.defines, - .flags = nci.flags, - }); + const key = nci.sources.ptr; + if (!seen.contains(key)) { + try seen.put(key, {}); + try infos.append(allocator, .{ + .sources = nci.sources, + .includes = nci.includes, + .defines = nci.defines, + .flags = nci.flags, + }); + } } } } diff --git a/src/core.zig b/src/core.zig index bb706f3..841a24d 100644 --- a/src/core.zig +++ b/src/core.zig @@ -23,6 +23,7 @@ pub const Compilation = struct { root: ?*Node = null, resolved_root: ?*Node = null, import_sources: std.StringHashMap([:0]const u8), + module_scopes: std.StringHashMap(std.StringHashMap(void)), sema_result: ?sema.SemaResult = null, ir_emitter: ?ir.LLVMEmitter = null, @@ -34,6 +35,7 @@ pub const Compilation = struct { .source = source, .diagnostics = errors.DiagnosticList.init(allocator, source, file_path), .import_sources = std.StringHashMap([:0]const u8).init(allocator), + .module_scopes = std.StringHashMap(std.StringHashMap(void)).init(allocator), .target_config = target_config, }; } @@ -66,6 +68,13 @@ pub const Compilation = struct { &self.diagnostics, ) catch return error.CompileError; + // Preserve per-module visibility scopes for C import access checking + self.module_scopes.put(self.file_path, mod.scope) catch {}; + var cache_it = cache.iterator(); + while (cache_it.next()) |entry| { + self.module_scopes.put(entry.key_ptr.*, entry.value_ptr.scope) catch {}; + } + // Store main file source in import_sources so error reporting can find it self.import_sources.put(self.file_path, self.source) catch {}; @@ -143,6 +152,7 @@ pub const Compilation = struct { lowering.resolved_root = root; lowering.target_config = self.target_config; lowering.diagnostics = &self.diagnostics; + lowering.module_scopes = &self.module_scopes; lowering.lowerRoot(root); if (self.diagnostics.hasErrors()) return error.CompileError; return module; diff --git a/src/ir/inst.zig b/src/ir/inst.zig index cde75e3..6881295 100644 --- a/src/ir/inst.zig +++ b/src/ir/inst.zig @@ -411,6 +411,7 @@ pub const Function = struct { is_comptime: bool = false, linkage: Linkage = .internal, call_conv: CallingConvention = .default, + source_file: ?[]const u8 = null, pub const Param = struct { name: StringId, diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 79a9640..86b9151 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -82,6 +82,8 @@ pub const Lowering = struct { lowered_functions: std.StringHashMap(void), // tracks which functions have been fully lowered local_fn_counter: u32 = 0, // unique counter for mangling local function names import_flags: std.StringHashMap(bool), // tracks whether each function is imported + module_scopes: ?*std.StringHashMap(std.StringHashMap(void)) = null, // per-module visible names (from import resolution) + current_source_file: ?[]const u8 = null, // source file of function currently being lowered type_bindings: ?std.StringHashMap(TypeId) = null, // generic type param bindings ($T → concrete TypeId) current_match_tags: ?[]const u64 = null, // type tags for current match arm (for runtime dispatch) force_block_value: bool = false, // set by lowerBlockValue to extract if-else values @@ -254,6 +256,7 @@ pub const Lowering = struct { /// This preserves the old behavior for comptime evaluation contexts. pub fn lowerDecls(self: *Lowering, decls: []const *const Node) void { for (decls) |decl| { + self.current_source_file = decl.source_file; const is_imported = if (self.main_file) |mf| (if (decl.source_file) |sf| !std.mem.eql(u8, sf, mf) else false) else @@ -308,6 +311,7 @@ pub const Lowering = struct { /// Pass 1: Scan declarations — register ASTs and extern stubs, but don't lower bodies. fn scanDecls(self: *Lowering, decls: []const *const Node) void { for (decls) |decl| { + self.current_source_file = decl.source_file; const is_imported = if (self.main_file) |mf| (if (decl.source_file) |sf| !std.mem.eql(u8, sf, mf) else false) else @@ -548,7 +552,9 @@ pub const Lowering = struct { if (fe.c_name) |c_name| { const c_name_id = self.module.types.internString(c_name); const fid = self.builder.declareExtern(c_name_id, params.items, ret_ty); - self.module.getFunctionMut(fid).call_conv = cc; + const func = self.module.getFunctionMut(fid); + func.call_conv = cc; + func.source_file = self.current_source_file; self.foreign_name_map.put(name, c_name) catch {}; return; } @@ -556,7 +562,23 @@ pub const Lowering = struct { const name_id = self.module.types.internString(name); const fid = self.builder.declareExtern(name_id, params.items, ret_ty); - self.module.getFunctionMut(fid).call_conv = cc; + const func = self.module.getFunctionMut(fid); + func.call_conv = cc; + func.source_file = self.current_source_file; + } + + /// 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 { + const fd = self.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; + // It's a C import fn_decl — check module scope + const scopes = self.module_scopes orelse return true; + const source = self.current_source_file orelse return true; + const scope = scopes.get(source) orelse return true; + return scope.contains(fn_name); } /// Lazily lower a function body on demand. Called when lowerCall can't find @@ -590,6 +612,7 @@ pub const Lowering = struct { const saved_defer_base = self.func_defer_base; const saved_block_terminated = self.block_terminated; const saved_force_block_value = self.force_block_value; + const saved_source_file = self.current_source_file; self.func_defer_base = self.defer_stack.items.len; self.block_terminated = false; self.force_block_value = false; @@ -611,6 +634,7 @@ pub const Lowering = struct { // Function not yet declared — create it fresh via lowerFunction self.lowerFunction(fd, name, false); // Restore builder state + self.current_source_file = saved_source_file; self.scope = saved_scope; self.func_defer_base = saved_defer_base; self.force_block_value = saved_force_block_value; @@ -624,8 +648,10 @@ pub const Lowering = struct { // Re-use the existing function slot — switch builder to it self.builder.func = fid; const func = &self.module.functions.items[@intFromEnum(fid)]; + self.current_source_file = func.source_file; if (!func.is_extern) { // Already promoted (e.g., via lowerComptimeDeps) — skip + self.current_source_file = saved_source_file; self.scope = saved_scope; self.func_defer_base = saved_defer_base; self.block_terminated = saved_block_terminated; @@ -694,6 +720,7 @@ pub const Lowering = struct { } // Restore builder state + self.current_source_file = saved_source_file; self.scope = saved_scope; self.func_defer_base = saved_defer_base; self.block_terminated = saved_block_terminated; @@ -3596,6 +3623,12 @@ pub const Lowering = struct { } break :blk scoped; }; + // C-import visibility: deny calls to C fn_decls not in the caller's module scope + if (!self.isCImportVisible(eff_name)) { + if (self.diagnostics) |d| + d.addFmt(.err, c.callee.span, "C function '{s}' not visible; add #import for the module that declares it", .{eff_name}); + return Ref.none; + } if (self.fn_ast_map.get(eff_name)) |fd| { if (self.current_match_tags) |tags| { if (tags.len > 0 and self.hasCastWithRuntimeType(c)) {