From 41eff5723dd61b19be88830f16d76514583e3339 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 19 Jan 2024 22:18:04 -0700 Subject: [PATCH 1/3] Compilation: avoid caching root source file twice The deleted lines here are redundant because they happen first thing inside the function call below. Additionally, skip hashing the root source file if it is an empty string. I explored making this field along with `root` optional but found this to be less messy actually. --- src/Compilation.zig | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index 8b6c7abbad..a36bf65227 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -1170,7 +1170,7 @@ fn addModuleTableToCacheHash( hash.addOptionalBytes(mod.root.root_dir.path); hash.addBytes(mod.root.sub_path); }, - .files => |man| { + .files => |man| if (mod.root_src_path.len != 0) { const pkg_zig_file = try mod.root.joinString(arena, mod.root_src_path); _ = try man.addFile(pkg_zig_file, null); }, @@ -2467,8 +2467,6 @@ fn addNonIncrementalStuffToCacheManifest( comptime assert(link_hash_implementation_version == 11); if (comp.module) |mod| { - const main_zig_file = try mod.main_mod.root.joinString(arena, mod.main_mod.root_src_path); - _ = try man.addFile(main_zig_file, null); try addModuleTableToCacheHash(gpa, arena, &man.hash, mod.root_mod, mod.main_mod, .{ .files = man }); // Synchronize with other matching comments: ZigOnlyHashStuff From 29cce62a259384e7daffe8fa68a0edf34377c6b3 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 19 Jan 2024 22:19:50 -0700 Subject: [PATCH 2/3] CLI: introduce -M; deprecate --mod I changed my mind on how the CLI for Zig modules should work. I don't like that `--mod` takes 2 parameters. Instead let's swing all the way in the other direction: `-M[name][=src]` This is shorter (Zig CLI invocations are long enough already), avoids the double parameter edge case, and supports the concept of omitting the source file part of the argument, which was already wanted for `-Mstd`. The legacy way to encode that was `--mod std ''` - awkward! Undocumented support for `--mod` remains so that this branch does not need a zig1.wasm update. The next time that file is updated, support for `--mod` can be dropped. Importantly, this commit also adds support for modules that do not have a root zig source file. In such case, it sets root to cwd and root_src_path to empty string, and only sets have_zcu to true if a module is provided with a root zig source file. --- src/main.zig | 160 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 107 insertions(+), 53 deletions(-) diff --git a/src/main.zig b/src/main.zig index 77e380aee8..75b7e5dde1 100644 --- a/src/main.zig +++ b/src/main.zig @@ -409,10 +409,10 @@ const usage_build_generic = \\ --libc [file] Provide a file which specifies libc paths \\ -x language Treat subsequent input files as having type \\ --dep [[import=]name] Add an entry to the next module's import table - \\ --mod [name] [src] Create a module based on the current per-module settings. + \\ -M[name][=src] Create a module based on the current per-module settings. \\ The first module is the main module. - \\ "std" can be configured by leaving src blank. - \\ After a --mod argument, per-module settings are reset. + \\ "std" can be configured by omitting src + \\ After a -M argument, per-module settings are reset. \\ --error-limit [num] Set the maximum amount of distinct error values \\ -fllvm Force using LLVM as the codegen backend \\ -fno-llvm Prevent using LLVM as the codegen backend @@ -1040,56 +1040,39 @@ fn buildOutputType( .value = value, }); } else if (mem.eql(u8, arg, "--mod")) { - const mod_name = args_iter.nextOrFatal(); - const root_src_orig = args_iter.nextOrFatal(); - - const gop = try create_module.modules.getOrPut(arena, mod_name); - - if (gop.found_existing) { - fatal("unable to add module '{s}': already exists as '{s}'", .{ - mod_name, gop.value_ptr.paths.root_src_path, - }); - } - - // See duplicate logic: ModCreationGlobalFlags - create_module.opts.have_zcu = true; - if (mod_opts.single_threaded == false) - create_module.opts.any_non_single_threaded = true; - if (mod_opts.sanitize_thread == true) - create_module.opts.any_sanitize_thread = true; - if (mod_opts.unwind_tables == true) - create_module.opts.any_unwind_tables = true; - if (mod_opts.strip == false) - create_module.opts.any_non_stripped = true; - if (mod_opts.error_tracing == true) - create_module.opts.any_error_tracing = true; - - const root_src = try introspect.resolvePath(arena, root_src_orig); - gop.value_ptr.* = .{ - .paths = .{ - .root = .{ - .root_dir = Cache.Directory.cwd(), - .sub_path = fs.path.dirname(root_src) orelse "", - }, - .root_src_path = fs.path.basename(root_src), - }, - .cc_argv = try cc_argv.toOwnedSlice(arena), - .inherited = mod_opts, - .target_arch_os_abi = target_arch_os_abi, - .target_mcpu = target_mcpu, - .deps = try deps.toOwnedSlice(arena), - .resolved = null, - .c_source_files_start = c_source_files_owner_index, - .c_source_files_end = create_module.c_source_files.items.len, - .rc_source_files_start = rc_source_files_owner_index, - .rc_source_files_end = create_module.rc_source_files.items.len, - }; - cssan.reset(); - mod_opts = .{}; - target_arch_os_abi = null; - target_mcpu = null; - c_source_files_owner_index = create_module.c_source_files.items.len; - rc_source_files_owner_index = create_module.rc_source_files.items.len; + // deprecated, kept around until the next zig1.wasm update + try handleModArg( + arena, + args_iter.nextOrFatal(), + args_iter.nextOrFatal(), + &create_module, + &mod_opts, + &cc_argv, + &target_arch_os_abi, + &target_mcpu, + &deps, + &c_source_files_owner_index, + &rc_source_files_owner_index, + &cssan, + ); + } else if (mem.startsWith(u8, arg, "-M")) { + var it = mem.splitScalar(u8, arg["-M".len..], '='); + const mod_name = it.next().?; + const root_src_orig = it.next(); + try handleModArg( + arena, + mod_name, + root_src_orig, + &create_module, + &mod_opts, + &cc_argv, + &target_arch_os_abi, + &target_mcpu, + &deps, + &c_source_files_owner_index, + &rc_source_files_owner_index, + &cssan, + ); } else if (mem.eql(u8, arg, "--error-limit")) { const next_arg = args_iter.nextOrFatal(); error_limit = std.fmt.parseUnsigned(Module.ErrorInt, next_arg, 0) catch |err| { @@ -7797,3 +7780,74 @@ fn parseImageBase(s: []const u8) u64 { return std.fmt.parseUnsigned(u64, s, 0) catch |err| fatal("unable to parse image base '{s}': {s}", .{ s, @errorName(err) }); } + +fn handleModArg( + arena: Allocator, + mod_name: []const u8, + opt_root_src_orig: ?[]const u8, + create_module: *CreateModule, + mod_opts: *Package.Module.CreateOptions.Inherited, + cc_argv: *std.ArrayListUnmanaged([]const u8), + target_arch_os_abi: *?[]const u8, + target_mcpu: *?[]const u8, + deps: *std.ArrayListUnmanaged(CliModule.Dep), + c_source_files_owner_index: *usize, + rc_source_files_owner_index: *usize, + cssan: *ClangSearchSanitizer, +) !void { + const gop = try create_module.modules.getOrPut(arena, mod_name); + + if (gop.found_existing) { + fatal("unable to add module '{s}': already exists as '{s}'", .{ + mod_name, gop.value_ptr.paths.root_src_path, + }); + } + + // See duplicate logic: ModCreationGlobalFlags + if (mod_opts.single_threaded == false) + create_module.opts.any_non_single_threaded = true; + if (mod_opts.sanitize_thread == true) + create_module.opts.any_sanitize_thread = true; + if (mod_opts.unwind_tables == true) + create_module.opts.any_unwind_tables = true; + if (mod_opts.strip == false) + create_module.opts.any_non_stripped = true; + if (mod_opts.error_tracing == true) + create_module.opts.any_error_tracing = true; + + gop.value_ptr.* = .{ + .paths = p: { + if (opt_root_src_orig) |root_src_orig| { + create_module.opts.have_zcu = true; + const root_src = try introspect.resolvePath(arena, root_src_orig); + break :p .{ + .root = .{ + .root_dir = Cache.Directory.cwd(), + .sub_path = fs.path.dirname(root_src) orelse "", + }, + .root_src_path = fs.path.basename(root_src), + }; + } + break :p .{ + .root = .{ .root_dir = Cache.Directory.cwd() }, + .root_src_path = "", + }; + }, + .cc_argv = try cc_argv.toOwnedSlice(arena), + .inherited = mod_opts.*, + .target_arch_os_abi = target_arch_os_abi.*, + .target_mcpu = target_mcpu.*, + .deps = try deps.toOwnedSlice(arena), + .resolved = null, + .c_source_files_start = c_source_files_owner_index.*, + .c_source_files_end = create_module.c_source_files.items.len, + .rc_source_files_start = rc_source_files_owner_index.*, + .rc_source_files_end = create_module.rc_source_files.items.len, + }; + cssan.reset(); + mod_opts.* = .{}; + target_arch_os_abi.* = null; + target_mcpu.* = null; + c_source_files_owner_index.* = create_module.c_source_files.items.len; + rc_source_files_owner_index.* = create_module.rc_source_files.items.len; +} From 2dea37545046e1a7e472e52c8c110ee6f32842d7 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 19 Jan 2024 22:28:07 -0700 Subject: [PATCH 3/3] std.Build.Compile: handle modules sans root source files Uses the new `-M[name][=src]` CLI syntax to omit the source when the module does not have a zig root source file. Only some kinds of link objects imply that this should happen. --- lib/std/Build/Step/Compile.zig | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index efa5129d97..c66fe80478 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -1220,15 +1220,18 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { } } - // The CLI assumes if it sees a --mod argument that it is a zig - // compilation unit. If there is no root source file, then this - // is not a zig compilation unit - it is perhaps a set of - // linker objects, or C source files instead. - // In such case, there will be only one module, so we can leave - // off the naming here. + // When the CLI sees a -M argument, it determines whether it + // implies the existence of a Zig compilation unit based on + // whether there is a root source file. If there is no root + // source file, then this is not a zig compilation unit - it is + // perhaps a set of linker objects, or C source files instead. + // Linker objects are added to the CLI globally, while C source + // files must have a module parent. if (module.root_source_file) |lp| { const src = lp.getPath2(module.owner, step); - try zig_args.appendSlice(&.{ "--mod", module_cli_name, src }); + try zig_args.append(b.fmt("-M{s}={s}", .{ module_cli_name, src })); + } else if (moduleNeedsCliArg(module)) { + try zig_args.append(b.fmt("-M{s}", .{module_cli_name})); } } } @@ -1850,3 +1853,10 @@ pub fn rootModuleTarget(c: *Compile) std.Target { // The root module is always given a target, so we know this to be non-null. return c.root_module.resolved_target.?.result; } + +fn moduleNeedsCliArg(mod: *const Module) bool { + return for (mod.link_objects.items) |o| switch (o) { + .c_source_file, .c_source_files, .assembly_file, .win32_resource_file => break true, + else => continue, + } else false; +}