Commit d7afd797ccdeeab74946f047c3e755f33b5ea9b9 | zig

author	Matthew Lugg <mlugg@mlugg.co.uk> 2025-06-11 14:27:21+01:00
committer	Matthew Lugg <mlugg@mlugg.co.uk> 2025-06-12 17:51:30+01:00
log	d7afd797ccdeeab74946f047c3e755f33b5ea9b9
tree	5c203e55c9c4ffaa18da83cf74f3bb19f79fac5d
parent	4d2b216121e5fbdb019ab8e727df654e7b08e984
signature	Commit is signed but in an unrecognized format.

Zcu: handle unreferenced `test_functions` correctly

Previously, `PerThread.populateTestFunctions` was analyzing the
`test_functions` declaration if it hadn't already been analyzed, so that
it could then populate it. However, the logic for doing this wasn't
actually correct, because it didn't trigger the necessary type
resolution. I could have tried to fix this, but there's actually a
simpler solution! If the `test_functions` declaration isn't referenced
or has a compile error, then we simply don't need to update it; either
it's unreferenced so its value doesn't matter, or we're going to get a
compile error anyway. Either way, we can just give up early. This avoids
doing semantic analysis after `performAllTheWork` finishes.

Also, get rid of the "Code Generation" progress node while updating the
test decl: this is a linking task.

2 files changed, 27 insertions(+), 33 deletions(-)

src
- Compilation.zig
- Zcu
  - PerThread.zig

src/Compilation.zig+1-1

...	@@ -2817,7 +2817,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {	...	@@ -2817,7 +2817,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
2817	// The `test_functions` decl has been intentionally postponed until now,	2817	// The `test_functions` decl has been intentionally postponed until now,
2818	// at which point we must populate it with the list of test functions that	2818	// at which point we must populate it with the list of test functions that
2819	// have been discovered and not filtered out.	2819	// have been discovered and not filtered out.
2820	try pt.populateTestFunctions(main_progress_node);	2820	try pt.populateTestFunctions();
2821	}	2821	}
2822		2822
2823	try pt.processExports();	2823	try pt.processExports();

src/Zcu/PerThread.zig+26-32

 @@ -3229,39 +3229,42 @@ fn processExportsInner(
     }
 }
-pub fn populateTestFunctions(
+pub fn populateTestFunctions(pt: Zcu.PerThread) Allocator.Error!void {
-    pt: Zcu.PerThread,
-    main_progress_node: std.Progress.Node,
-) Allocator.Error!void {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
     const ip = &zcu.intern_pool;
+    // Our job is to correctly set the value of the `test_functions` declaration if it has been
+    // analyzed and sent to codegen, It usually will have been, because the test runner will
+    // reference it, and `std.builtin` shouldn't have type errors. However, if it hasn't been
+    // analyzed, we will just terminate early, since clearly the test runner hasn't referenced
+    // `test_functions` so there's no point populating it. More to the the point, we potentially
+    // *can't* populate it without doing some type resolution, and... let's try to leave Sema in
+    // the past here.
     const builtin_mod = zcu.builtin_modules.get(zcu.root_mod.getBuiltinOptions(zcu.comp.config).hash()).?;
     const builtin_file_index = zcu.module_roots.get(builtin_mod).?.unwrap().?;
-    pt.ensureFileAnalyzed(builtin_file_index) catch |err| switch (err) {
+    const builtin_root_type = zcu.fileRootType(builtin_file_index);
-        error.AnalysisFail => unreachable, // builtin module is generated so cannot be corrupt
+    if (builtin_root_type == .none) return; // `@import("builtin")` never analyzed
-        error.OutOfMemory => |e| return e,
+    const builtin_namespace = Type.fromInterned(builtin_root_type).getNamespace(zcu).unwrap().?;
-    };
+    // We know that the namespace has a `test_functions`...
-    const builtin_root_type = Type.fromInterned(zcu.fileRootType(builtin_file_index));
-    const builtin_namespace = builtin_root_type.getNamespace(zcu).unwrap().?;
     const nav_index = zcu.namespacePtr(builtin_namespace).pub_decls.getKeyAdapted(
         try ip.getOrPutString(gpa, pt.tid, "test_functions", .no_embedded_nulls),
         Zcu.Namespace.NameAdapter{ .zcu = zcu },
     ).?;
+    // ...but it might not be populated, so let's check that!
+    if (zcu.failed_analysis.contains(.wrap(.{ .nav_val = nav_index })) or
+        zcu.transitive_failed_analysis.contains(.wrap(.{ .nav_val = nav_index })) or
+        ip.getNav(nav_index).status != .fully_resolved)
     {
-        // We have to call `ensureNavValUpToDate` here in case `builtin.test_functions`
+        // The value of `builtin.test_functions` was either never referenced, or failed analysis.
-        // was not referenced by start code.
+        // Either way, we don't need to do anything.
-        zcu.sema_prog_node = main_progress_node.start("Semantic Analysis", 0);
+        return;
-        defer {
-            zcu.sema_prog_node.end();
-            zcu.sema_prog_node = std.Progress.Node.none;
-        }
-        pt.ensureNavValUpToDate(nav_index) catch |err| switch (err) {
-            error.AnalysisFail => return,
-            error.OutOfMemory => return error.OutOfMemory,
-        };
     }
+    // Okay, `builtin.test_functions` is (potentially) referenced and valid. Our job now is to swap
+    // its placeholder `&.{}` value for the actual list of all test functions.
     const test_fns_val = zcu.navValue(nav_index);
     const test_fn_ty = test_fns_val.typeOf(zcu).slicePtrFieldType(zcu).childType(zcu);
 @@ -3363,17 +3366,8 @@ pub fn populateTestFunctions(
         } });
         ip.mutateVarInit(test_fns_val.toIntern(), new_init);
     }
-    {
+    // The linker thread is not running, so we actually need to dispatch this task directly.
-        assert(zcu.codegen_prog_node.index == .none);
+    @import("../link.zig").linkTestFunctionsNav(pt, nav_index);
-        zcu.codegen_prog_node = main_progress_node.start("Code Generation", 0);
-        defer {
-            zcu.codegen_prog_node.end();
-            zcu.codegen_prog_node = std.Progress.Node.none;
-        }
-        // The linker thread is not running, so we actually need to dispatch this task directly.
-        @import("../link.zig").linkTestFunctionsNav(pt, nav_index);
-    }
 }
 /// Stores an error in `pt.zcu.failed_files` for this file, and sets the file