From c6a70b031e77f9fcb08d0fada780a9c4418f47cb Mon Sep 17 00:00:00 2001 From: Techatrix Date: Thu, 9 Jan 2025 20:10:02 +0100 Subject: [PATCH] build_runner: collect c macro definitions from build.zig fixes #2110 --- src/DocumentStore.zig | 44 ++++++++++++++++++- src/build_runner/master.zig | 39 +++++++++------- src/build_runner/shared.zig | 1 + src/translate_c.zig | 5 ++- tests/build_runner_cases/add_module.json | 3 +- tests/build_runner_cases/define_c_macro.json | 18 ++++++++ tests/build_runner_cases/define_c_macro.zig | 8 ++++ tests/build_runner_cases/empty.json | 3 +- .../module_self_import.json | 3 +- .../multiple_module_import_names.json | 3 +- tests/language_features/cimport.zig | 8 +++- 11 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 tests/build_runner_cases/define_c_macro.json create mode 100644 tests/build_runner_cases/define_c_macro.zig diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig index 498c0c6f8..6487cf45a 100644 --- a/src/DocumentStore.zig +++ b/src/DocumentStore.zig @@ -1489,6 +1489,34 @@ pub fn collectIncludeDirs( return collected_all; } +/// returns `true` if all c macro definitions could be collected +/// may return `false` because macros from a build.zig may not have been resolved already +/// **Thread safe** takes a shared lock +pub fn collectCMacros( + store: *DocumentStore, + allocator: std.mem.Allocator, + handle: *Handle, + c_macros: *std.ArrayListUnmanaged([]const u8), +) !bool { + const collected_all = switch (try handle.getAssociatedBuildFileUri2(store)) { + .none => true, + .unresolved => false, + .resolved => |build_file_uri| blk: { + const build_file = store.getBuildFile(build_file_uri).?; + const build_config = build_file.tryLockConfig() orelse break :blk false; + defer build_file.unlockConfig(); + + try c_macros.ensureUnusedCapacity(allocator, build_config.c_macros.len); + for (build_config.c_macros) |c_macro| { + c_macros.appendAssumeCapacity(try allocator.dupe(u8, c_macro)); + } + break :blk true; + }, + }; + + return collected_all; +} + /// returns the document behind `@cImport()` where `node` is the `cImport` node /// if a cImport can't be translated e.g. requires computing a /// comptime value `resolveCImport` will return null @@ -1533,10 +1561,24 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde return null; }; + var c_macros: std.ArrayListUnmanaged([]const u8) = .empty; + defer { + for (c_macros.items) |c_macro| { + self.allocator.free(c_macro); + } + c_macros.deinit(self.allocator); + } + + const collected_all_c_macros = self.collectCMacros(self.allocator, handle, &c_macros) catch |err| { + log.err("failed to resolve include paths: {}", .{err}); + return null; + }; + const maybe_result = translate_c.translate( self.allocator, self.config, include_dirs.items, + c_macros.items, source, ) catch |err| switch (err) { error.OutOfMemory => |e| return e, @@ -1547,7 +1589,7 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde }; var result = maybe_result orelse return null; - if (result == .failure and !collected_all_include_dirs) { + if (result == .failure and (!collected_all_include_dirs or !collected_all_c_macros)) { result.deinit(self.allocator); return null; } diff --git a/src/build_runner/master.zig b/src/build_runner/master.zig index 930571b93..fc70c7794 100644 --- a/src/build_runner/master.zig +++ b/src/build_runner/master.zig @@ -1026,13 +1026,19 @@ fn extractBuildInformation( name: []const u8, packages: *Packages, include_dirs: *std.StringArrayHashMapUnmanaged(void), + c_macros: *std.StringArrayHashMapUnmanaged(void), ) !void { if (module.root_source_file) |root_source_file| { _ = try packages.addPackage(name, root_source_file.getPath(module.owner)); } if (compile) |exe| { - try processPkgConfig(allocator, include_dirs, exe); + try processPkgConfig(allocator, include_dirs, c_macros, exe); + } + + try c_macros.ensureUnusedCapacity(allocator, module.c_macros.items.len); + for (module.c_macros.items) |c_macro| { + c_macros.putAssumeCapacity(c_macro, {}); } for (module.include_dirs.items) |include_dir| { @@ -1119,6 +1125,9 @@ fn extractBuildInformation( var include_dirs: std.StringArrayHashMapUnmanaged(void) = .{}; defer include_dirs.deinit(gpa); + var c_macros: std.StringArrayHashMapUnmanaged(void) = .{}; + defer c_macros.deinit(gpa); + var packages: Packages = .{ .allocator = gpa }; defer packages.deinit(); @@ -1127,20 +1136,20 @@ fn extractBuildInformation( for (steps.keys()) |step| { const compile = step.cast(Step.Compile) orelse continue; const graph = compile.root_module.getGraph(); - try helper.processItem(gpa, compile.root_module, compile, "root", &packages, &include_dirs); + try helper.processItem(gpa, compile.root_module, compile, "root", &packages, &include_dirs, &c_macros); for (graph.modules) |module| { for (module.import_table.keys(), module.import_table.values()) |name, import| { - try helper.processItem(gpa, import, null, name, &packages, &include_dirs); + try helper.processItem(gpa, import, null, name, &packages, &include_dirs, &c_macros); } } } for (b.modules.values()) |root_module| { const graph = root_module.getGraph(); - try helper.processItem(gpa, root_module, null, "root", &packages, &include_dirs); + try helper.processItem(gpa, root_module, null, "root", &packages, &include_dirs, &c_macros); for (graph.modules) |module| { for (module.import_table.keys(), module.import_table.values()) |name, import| { - try helper.processItem(gpa, import, null, name, &packages, &include_dirs); + try helper.processItem(gpa, import, null, name, &packages, &include_dirs, &c_macros); } } } @@ -1190,6 +1199,7 @@ fn extractBuildInformation( .include_dirs = include_dirs.keys(), .top_level_steps = b.top_level_steps.keys(), .available_options = available_options, + .c_macros = c_macros.keys(), }, .{ .whitespace = .indent_2, @@ -1201,6 +1211,7 @@ fn extractBuildInformation( fn processPkgConfig( allocator: std.mem.Allocator, include_dirs: *std.StringArrayHashMapUnmanaged(void), + c_macros: *std.StringArrayHashMapUnmanaged(void), exe: *Step.Compile, ) !void { for (exe.root_module.link_objects.items) |link_object| { @@ -1209,7 +1220,7 @@ fn processPkgConfig( if (system_lib.use_pkg_config == .no) continue; - getPkgConfigIncludes(allocator, include_dirs, exe, system_lib.name) catch |err| switch (err) { + const args = copied_from_zig.runPkgConfig(exe, system_lib.name) catch |err| switch (err) { error.PkgConfigInvalidOutput, error.PkgConfigCrashed, error.PkgConfigFailed, @@ -1218,31 +1229,25 @@ fn processPkgConfig( => switch (system_lib.use_pkg_config) { .yes => { // pkg-config failed, so zig will not add any include paths + continue; }, .force => { std.log.warn("pkg-config failed for library {s}", .{system_lib.name}); + continue; }, .no => unreachable, }, else => |e| return e, }; - } -} - -fn getPkgConfigIncludes( - allocator: std.mem.Allocator, - include_dirs: *std.StringArrayHashMapUnmanaged(void), - exe: *Step.Compile, - name: []const u8, -) !void { - if (copied_from_zig.runPkgConfig(exe, name)) |args| { for (args) |arg| { if (std.mem.startsWith(u8, arg, "-I")) { const candidate = arg[2..]; try include_dirs.put(allocator, candidate, {}); + } else if (std.mem.startsWith(u8, arg, "-D")) { + try c_macros.put(allocator, arg, {}); } } - } else |err| return err; + } } // TODO: Having a copy of this is not very nice diff --git a/src/build_runner/shared.zig b/src/build_runner/shared.zig index 18b9d64e4..792500e09 100644 --- a/src/build_runner/shared.zig +++ b/src/build_runner/shared.zig @@ -11,6 +11,7 @@ pub const BuildConfig = struct { include_dirs: []const []const u8, top_level_steps: []const []const u8, available_options: std.json.ArrayHashMap(AvailableOption), + c_macros: []const []const u8 = &.{}, pub const DepsBuildRoots = Package; pub const Package = struct { diff --git a/src/translate_c.zig b/src/translate_c.zig index 318302c90..4f16137cb 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -118,6 +118,7 @@ pub fn translate( allocator: std.mem.Allocator, config: Config, include_dirs: []const []const u8, + c_macros: []const []const u8, source: []const u8, ) !?Result { const tracy_zone = tracy.trace(@src()); @@ -166,7 +167,7 @@ pub fn translate( "--listen=-", }; - const argc = base_args.len + 2 * include_dirs.len + 1; + const argc = base_args.len + 2 * include_dirs.len + c_macros.len + 1; var argv: std.ArrayListUnmanaged([]const u8) = try .initCapacity(allocator, argc); defer argv.deinit(allocator); @@ -177,6 +178,8 @@ pub fn translate( argv.appendAssumeCapacity(include_dir); } + argv.appendSliceAssumeCapacity(c_macros); + argv.appendAssumeCapacity(file_path); var process: std.process.Child = .init(argv.items, allocator); diff --git a/tests/build_runner_cases/add_module.json b/tests/build_runner_cases/add_module.json index fa62c8df9..a51926a19 100644 --- a/tests/build_runner_cases/add_module.json +++ b/tests/build_runner_cases/add_module.json @@ -11,5 +11,6 @@ "install", "uninstall" ], - "available_options": {} + "available_options": {}, + "c_macros": [] } \ No newline at end of file diff --git a/tests/build_runner_cases/define_c_macro.json b/tests/build_runner_cases/define_c_macro.json new file mode 100644 index 000000000..b61f3eff0 --- /dev/null +++ b/tests/build_runner_cases/define_c_macro.json @@ -0,0 +1,18 @@ +{ + "deps_build_roots": [], + "packages": [ + { + "name": "root", + "path": "root.zig" + } + ], + "include_dirs": [], + "top_level_steps": [ + "install", + "uninstall" + ], + "available_options": {}, + "c_macros": [ + "-Dkey=value" + ] +} \ No newline at end of file diff --git a/tests/build_runner_cases/define_c_macro.zig b/tests/build_runner_cases/define_c_macro.zig new file mode 100644 index 000000000..c1cc03fdd --- /dev/null +++ b/tests/build_runner_cases/define_c_macro.zig @@ -0,0 +1,8 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const foo = b.addModule("foo", .{ + .root_source_file = b.path("root.zig"), + }); + foo.addCMacro("key", "value"); +} diff --git a/tests/build_runner_cases/empty.json b/tests/build_runner_cases/empty.json index 8b55ee76a..bc46f1e1c 100644 --- a/tests/build_runner_cases/empty.json +++ b/tests/build_runner_cases/empty.json @@ -6,5 +6,6 @@ "install", "uninstall" ], - "available_options": {} + "available_options": {}, + "c_macros": [] } \ No newline at end of file diff --git a/tests/build_runner_cases/module_self_import.json b/tests/build_runner_cases/module_self_import.json index b6149690a..2c9d4df8c 100644 --- a/tests/build_runner_cases/module_self_import.json +++ b/tests/build_runner_cases/module_self_import.json @@ -15,5 +15,6 @@ "install", "uninstall" ], - "available_options": {} + "available_options": {}, + "c_macros": [] } \ No newline at end of file diff --git a/tests/build_runner_cases/multiple_module_import_names.json b/tests/build_runner_cases/multiple_module_import_names.json index 0efa251a3..35d25ffb0 100644 --- a/tests/build_runner_cases/multiple_module_import_names.json +++ b/tests/build_runner_cases/multiple_module_import_names.json @@ -35,5 +35,6 @@ "install", "uninstall" ], - "available_options": {} + "available_options": {}, + "c_macros": [] } \ No newline at end of file diff --git a/tests/language_features/cimport.zig b/tests/language_features/cimport.zig index b74d28afa..1e97c96b0 100644 --- a/tests/language_features/cimport.zig +++ b/tests/language_features/cimport.zig @@ -109,7 +109,13 @@ fn testTranslate(c_source: []const u8) !translate_c.Result { var ctx: Context = try .init(); defer ctx.deinit(); - var result = (try translate_c.translate(allocator, zls.DocumentStore.Config.fromMainConfig(ctx.server.config), &.{}, c_source)).?; + var result = (try translate_c.translate( + allocator, + zls.DocumentStore.Config.fromMainConfig(ctx.server.config), + &.{}, + &.{}, + c_source, + )).?; errdefer result.deinit(allocator); switch (result) {