From b41a0b4768d368d81d0d33c779f919d8f315e622 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Wed, 6 Mar 2024 21:24:38 +0000
Subject: Package.Module: deduplicate identical builtin modules

Previously, when multiple modules had builtin modules with identical
sources, two distinct `Module`s and `File`s were created pointing at the
same file path. This led to a bug later in the frontend. These modules
are now deduplicated with a simple hashmap on the builtin source.
---
 src/Package/Module.zig | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

(limited to 'src/Package/Module.zig')

diff --git a/src/Package/Module.zig b/src/Package/Module.zig
index 6fadbf1dd5..e5376ed93a 100644
--- a/src/Package/Module.zig
+++ b/src/Package/Module.zig
@@ -63,6 +63,11 @@ pub const CreateOptions = struct {
 
     builtin_mod: ?*Package.Module,
 
+    /// Allocated into the given `arena`. Should be shared across all module creations in a Compilation.
+    /// Ignored if `builtin_mod` is passed or if `!have_zcu`.
+    /// Otherwise, may be `null` only if this Compilation consists of a single module.
+    builtin_modules: ?*std.StringHashMapUnmanaged(*Module),
+
     pub const Paths = struct {
         root: Package.Path,
         /// Relative to `root`. May contain path separators.
@@ -364,11 +369,20 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module {
             .wasi_exec_model = options.global.wasi_exec_model,
         }, arena);
 
+        const new = if (options.builtin_modules) |builtins| new: {
+            const gop = try builtins.getOrPut(arena, generated_builtin_source);
+            if (gop.found_existing) break :b gop.value_ptr.*;
+            errdefer builtins.removeByPtr(gop.key_ptr);
+            const new = try arena.create(Module);
+            gop.value_ptr.* = new;
+            break :new new;
+        } else try arena.create(Module);
+        errdefer if (options.builtin_modules) |builtins| assert(builtins.remove(generated_builtin_source));
+
         const new_file = try arena.create(File);
 
         const digest = Cache.HashHelper.oneShot(generated_builtin_source);
         const builtin_sub_path = try arena.dupe(u8, "b" ++ std.fs.path.sep_str ++ digest);
-        const new = try arena.create(Module);
         new.* = .{
             .root = .{
                 .root_dir = options.global_cache_directory,
-- 
cgit v1.2.3


From 38331b1cabf86586bdf70c80ed98f74c60305160 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 7 Mar 2024 06:26:25 +0000
Subject: Package.Module: actually set File.path_digest for builtin modules

---
 src/Builtin.zig        |  2 ++
 src/Package/Module.zig | 24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

(limited to 'src/Package/Module.zig')

diff --git a/src/Builtin.zig b/src/Builtin.zig
index 5c8577f4cb..d9eb1a23ff 100644
--- a/src/Builtin.zig
+++ b/src/Builtin.zig
@@ -264,6 +264,8 @@ pub fn populateFile(comp: *Compilation, mod: *Module, file: *File) !void {
     assert(!file.zir.hasCompileErrors()); // builtin.zig must not have astgen errors
     file.zir_loaded = true;
     file.status = .success_zir;
+    // Note that whilst we set `zir_loaded` here, we populated `path_digest`
+    // all the way back in `Package.Module.create`.
 }
 
 fn writeFile(file: *File, mod: *Module) !void {
diff --git a/src/Package/Module.zig b/src/Package/Module.zig
index e5376ed93a..c6eb1e8c90 100644
--- a/src/Package/Module.zig
+++ b/src/Package/Module.zig
@@ -381,8 +381,25 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module {
 
         const new_file = try arena.create(File);
 
-        const digest = Cache.HashHelper.oneShot(generated_builtin_source);
-        const builtin_sub_path = try arena.dupe(u8, "b" ++ std.fs.path.sep_str ++ digest);
+        const bin_digest, const hex_digest = digest: {
+            var hasher: Cache.Hasher = Cache.hasher_init;
+            hasher.update(generated_builtin_source);
+
+            var bin_digest: Cache.BinDigest = undefined;
+            hasher.final(&bin_digest);
+
+            var hex_digest: Cache.HexDigest = undefined;
+            _ = std.fmt.bufPrint(
+                &hex_digest,
+                "{s}",
+                .{std.fmt.fmtSliceHexLower(&bin_digest)},
+            ) catch unreachable;
+
+            break :digest .{ bin_digest, hex_digest };
+        };
+
+        const builtin_sub_path = try arena.dupe(u8, "b" ++ std.fs.path.sep_str ++ hex_digest);
+
         new.* = .{
             .root = .{
                 .root_dir = options.global_cache_directory,
@@ -429,6 +446,9 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module {
             .status = .never_loaded,
             .mod = new,
             .root_decl = .none,
+            // We might as well use this digest for the File `path digest`, since there's a
+            // one-to-one correspondence here between distinct paths and distinct contents.
+            .path_digest = bin_digest,
         };
         break :b new;
     };
-- 
cgit v1.2.3