Commit b73ef34289

Jakub Konka <kubkon@jakubkonka.com>
2023-08-21 08:07:42
frontend: directly pass resolved frameworks container to the linker
We can infer the framework name from the included resolved framework path. Fix hash implementation, and bump linker hash value from 9 to 10.
1 parent 4793daf
src/link/Coff/lld.zig
@@ -63,7 +63,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod
         man = comp.cache_parent.obtain();
         self.base.releaseLock();
 
-        comptime assert(Compilation.link_hash_implementation_version == 9);
+        comptime assert(Compilation.link_hash_implementation_version == 10);
 
         for (self.base.options.objects) |obj| {
             _ = try man.addFile(obj.path, null);
src/link/MachO/zld.zig
@@ -3396,7 +3396,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
         // We are about to obtain this lock, so here we give other processes a chance first.
         macho_file.base.releaseLock();
 
-        comptime assert(Compilation.link_hash_implementation_version == 9);
+        comptime assert(Compilation.link_hash_implementation_version == 10);
 
         for (options.objects) |obj| {
             _ = try man.addFile(obj.path, null);
@@ -3417,7 +3417,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
         man.hash.add(options.strip);
         man.hash.addListOfBytes(options.lib_dirs);
         man.hash.addListOfBytes(options.framework_dirs);
-        link.hashAddFrameworks(&man.hash, options.frameworks);
+        try link.hashAddFrameworks(&man, options.frameworks);
         man.hash.addListOfBytes(options.rpath_list);
         if (is_dyn_lib) {
             man.hash.addOptionalBytes(options.install_name);
@@ -3555,9 +3555,8 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
         }
 
         {
-            const vals = options.frameworks.values();
-            try libs.ensureUnusedCapacity(vals.len);
-            for (vals) |v| libs.putAssumeCapacity(v.path, .{
+            try libs.ensureUnusedCapacity(options.frameworks.len);
+            for (options.frameworks) |v| libs.putAssumeCapacity(v.path, .{
                 .needed = v.needed,
                 .weak = v.weak,
                 .path = v.path,
@@ -3664,14 +3663,14 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
                 try argv.append(try std.fmt.allocPrint(arena, "-L{s}", .{lib_dir}));
             }
 
-            for (options.frameworks.keys()) |framework| {
-                const info = options.frameworks.get(framework).?;
-                const arg = if (info.needed)
-                    try std.fmt.allocPrint(arena, "-needed_framework {s}", .{framework})
-                else if (info.weak)
-                    try std.fmt.allocPrint(arena, "-weak_framework {s}", .{framework})
+            for (options.frameworks) |framework| {
+                const name = std.fs.path.stem(framework.path);
+                const arg = if (framework.needed)
+                    try std.fmt.allocPrint(arena, "-needed_framework {s}", .{name})
+                else if (framework.weak)
+                    try std.fmt.allocPrint(arena, "-weak_framework {s}", .{name})
                 else
-                    try std.fmt.allocPrint(arena, "-framework {s}", .{framework});
+                    try std.fmt.allocPrint(arena, "-framework {s}", .{name});
                 try argv.append(arg);
             }
 
src/link/Elf.zig
@@ -1367,7 +1367,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
         // We are about to obtain this lock, so here we give other processes a chance first.
         self.base.releaseLock();
 
-        comptime assert(Compilation.link_hash_implementation_version == 9);
+        comptime assert(Compilation.link_hash_implementation_version == 10);
 
         try man.addOptionalFile(self.base.options.linker_script);
         try man.addOptionalFile(self.base.options.version_script);
src/link/Wasm.zig
@@ -3193,7 +3193,7 @@ fn linkWithZld(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) l
         // We are about to obtain this lock, so here we give other processes a chance first.
         wasm.base.releaseLock();
 
-        comptime assert(Compilation.link_hash_implementation_version == 9);
+        comptime assert(Compilation.link_hash_implementation_version == 10);
 
         for (options.objects) |obj| {
             _ = try man.addFile(obj.path, null);
@@ -4254,7 +4254,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
         // We are about to obtain this lock, so here we give other processes a chance first.
         wasm.base.releaseLock();
 
-        comptime assert(Compilation.link_hash_implementation_version == 9);
+        comptime assert(Compilation.link_hash_implementation_version == 10);
 
         for (wasm.base.options.objects) |obj| {
             _ = try man.addFile(obj.path, null);
src/Compilation.zig
@@ -507,8 +507,7 @@ pub const InitOptions = struct {
     c_source_files: []const CSourceFile = &[0]CSourceFile{},
     link_objects: []LinkObject = &[0]LinkObject{},
     framework_dirs: []const []const u8 = &[0][]const u8{},
-    framework_names: []const []const u8 = &.{},
-    framework_infos: []const Framework = &.{},
+    frameworks: []const Framework = &.{},
     system_lib_names: []const []const u8 = &.{},
     system_lib_infos: []const SystemLib = &.{},
     /// These correspond to the WASI libc emulated subcomponents including:
@@ -831,7 +830,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             // Our linker can't handle objects or most advanced options yet.
             if (options.link_objects.len != 0 or
                 options.c_source_files.len != 0 or
-                options.framework_names.len != 0 or
+                options.frameworks.len != 0 or
                 options.system_lib_names.len != 0 or
                 options.link_libc or options.link_libcpp or
                 link_eh_frame_hdr or
@@ -1447,13 +1446,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             system_libs.putAssumeCapacity(lib_name, options.system_lib_infos[i]);
         }
 
-        var frameworks: std.StringArrayHashMapUnmanaged(Framework) = .{};
-        errdefer frameworks.deinit(gpa);
-        try frameworks.ensureTotalCapacity(gpa, options.framework_names.len);
-        for (options.framework_names, options.framework_infos) |framework_name, info| {
-            frameworks.putAssumeCapacity(framework_name, info);
-        }
-
         const bin_file = try link.File.openPath(gpa, .{
             .emit = bin_file_emit,
             .implib_emit = implib_emit,
@@ -1473,7 +1465,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             .link_libcpp = link_libcpp,
             .link_libunwind = link_libunwind,
             .objects = options.link_objects,
-            .frameworks = frameworks,
+            .frameworks = options.frameworks,
             .framework_dirs = options.framework_dirs,
             .system_libs = system_libs,
             .wasi_emulated_libs = options.wasi_emulated_libs,
@@ -2275,7 +2267,7 @@ fn prepareWholeEmitSubPath(arena: Allocator, opt_emit: ?EmitLoc) error{OutOfMemo
 /// to remind the programmer to update multiple related pieces of code that
 /// are in different locations. Bump this number when adding or deleting
 /// anything from the link cache manifest.
-pub const link_hash_implementation_version = 9;
+pub const link_hash_implementation_version = 10;
 
 fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifest) !void {
     const gpa = comp.gpa;
@@ -2285,7 +2277,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
     defer arena_allocator.deinit();
     const arena = arena_allocator.allocator();
 
-    comptime assert(link_hash_implementation_version == 9);
+    comptime assert(link_hash_implementation_version == 10);
 
     if (comp.bin_file.options.module) |mod| {
         const main_zig_file = try mod.main_pkg.root_src_directory.join(arena, &[_][]const u8{
@@ -2394,7 +2386,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
 
     // Mach-O specific stuff
     man.hash.addListOfBytes(comp.bin_file.options.framework_dirs);
-    link.hashAddFrameworks(&man.hash, comp.bin_file.options.frameworks);
+    try link.hashAddFrameworks(man, comp.bin_file.options.frameworks);
     try man.addOptionalFile(comp.bin_file.options.entitlements);
     man.hash.addOptional(comp.bin_file.options.pagezero_size);
     man.hash.addOptional(comp.bin_file.options.headerpad_size);
src/link.zig
@@ -57,15 +57,11 @@ pub fn hashAddSystemLibs(
     }
 }
 
-pub fn hashAddFrameworks(
-    hh: *Cache.HashHelper,
-    hm: std.StringArrayHashMapUnmanaged(Framework),
-) void {
-    const keys = hm.keys();
-    hh.addListOfBytes(keys);
-    for (hm.values()) |value| {
-        hh.add(value.needed);
-        hh.add(value.weak);
+pub fn hashAddFrameworks(man: *Cache.Manifest, hm: []const Framework) !void {
+    for (hm) |value| {
+        man.hash.add(value.needed);
+        man.hash.add(value.weak);
+        _ = try man.addFile(value.path, null);
     }
 }
 
@@ -209,7 +205,7 @@ pub const Options = struct {
 
     objects: []Compilation.LinkObject,
     framework_dirs: []const []const u8,
-    frameworks: std.StringArrayHashMapUnmanaged(Framework),
+    frameworks: []const Framework,
     /// These are *always* dynamically linked. Static libraries will be
     /// provided as positional arguments.
     system_libs: std.StringArrayHashMapUnmanaged(SystemLib),
@@ -277,7 +273,6 @@ pub const Options = struct {
 
     pub fn move(self: *Options) Options {
         const copied_state = self.*;
-        self.frameworks = .{};
         self.system_libs = .{};
         self.force_undefined_symbols = .{};
         return copied_state;
@@ -643,7 +638,6 @@ pub const File = struct {
         base.releaseLock();
         if (base.file) |f| f.close();
         if (base.intermediary_basename) |sub_path| base.allocator.free(sub_path);
-        base.options.frameworks.deinit(base.allocator);
         base.options.system_libs.deinit(base.allocator);
         base.options.force_undefined_symbols.deinit(base.allocator);
         switch (base.tag) {
src/main.zig
@@ -2876,10 +2876,7 @@ fn buildOutputType(
     // After this point, resolved_system_libs is used instead of external_system_libs.
 
     // We now repeat part of the process for frameworks.
-    var resolved_frameworks: std.MultiArrayList(struct {
-        name: []const u8,
-        framework: Compilation.Framework,
-    }) = .{};
+    var resolved_frameworks = std.ArrayList(Compilation.Framework).init(arena);
 
     if (frameworks.keys().len > 0) {
         var test_path = std.ArrayList(u8).init(gpa);
@@ -2904,13 +2901,10 @@ fn buildOutputType(
                     framework_name,
                 )) {
                     const path = try arena.dupe(u8, test_path.items);
-                    try resolved_frameworks.append(arena, .{
-                        .name = framework_name,
-                        .framework = .{
-                            .needed = info.needed,
-                            .weak = info.weak,
-                            .path = path,
-                        },
+                    try resolved_frameworks.append(.{
+                        .needed = info.needed,
+                        .weak = info.weak,
+                        .path = path,
                     });
                     continue :framework;
                 }
@@ -3327,8 +3321,7 @@ fn buildOutputType(
         .c_source_files = c_source_files.items,
         .link_objects = link_objects.items,
         .framework_dirs = framework_dirs.items,
-        .framework_names = resolved_frameworks.items(.name),
-        .framework_infos = resolved_frameworks.items(.framework),
+        .frameworks = resolved_frameworks.items,
         .system_lib_names = resolved_system_libs.items(.name),
         .system_lib_infos = resolved_system_libs.items(.lib),
         .wasi_emulated_libs = wasi_emulated_libs.items,