Skip to content

Commit b7cb42f

Browse files
committed
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.
1 parent 01287a3 commit b7cb42f

File tree

2 files changed

+27
-33
lines changed

2 files changed

+27
-33
lines changed

src/Compilation.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2817,7 +2817,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
28172817
// The `test_functions` decl has been intentionally postponed until now,
28182818
// at which point we must populate it with the list of test functions that
28192819
// have been discovered and not filtered out.
2820-
try pt.populateTestFunctions(main_progress_node);
2820+
try pt.populateTestFunctions();
28212821
}
28222822

28232823
try pt.processExports();

src/Zcu/PerThread.zig

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3229,39 +3229,42 @@ fn processExportsInner(
32293229
}
32303230
}
32313231

3232-
pub fn populateTestFunctions(
3233-
pt: Zcu.PerThread,
3234-
main_progress_node: std.Progress.Node,
3235-
) Allocator.Error!void {
3232+
pub fn populateTestFunctions(pt: Zcu.PerThread) Allocator.Error!void {
32363233
const zcu = pt.zcu;
32373234
const gpa = zcu.gpa;
32383235
const ip = &zcu.intern_pool;
3236+
3237+
// Our job is to correctly set the value of the `test_functions` declaration if it has been
3238+
// analyzed and sent to codegen, It usually will have been, because the test runner will
3239+
// reference it, and `std.builtin` shouldn't have type errors. However, if it hasn't been
3240+
// analyzed, we will just terminate early, since clearly the test runner hasn't referenced
3241+
// `test_functions` so there's no point populating it. More to the the point, we potentially
3242+
// *can't* populate it without doing some type resolution, and... let's try to leave Sema in
3243+
// the past here.
3244+
32393245
const builtin_mod = zcu.builtin_modules.get(zcu.root_mod.getBuiltinOptions(zcu.comp.config).hash()).?;
32403246
const builtin_file_index = zcu.module_roots.get(builtin_mod).?.unwrap().?;
3241-
pt.ensureFileAnalyzed(builtin_file_index) catch |err| switch (err) {
3242-
error.AnalysisFail => unreachable, // builtin module is generated so cannot be corrupt
3243-
error.OutOfMemory => |e| return e,
3244-
};
3245-
const builtin_root_type = Type.fromInterned(zcu.fileRootType(builtin_file_index));
3246-
const builtin_namespace = builtin_root_type.getNamespace(zcu).unwrap().?;
3247+
const builtin_root_type = zcu.fileRootType(builtin_file_index);
3248+
if (builtin_root_type == .none) return; // `@import("builtin")` never analyzed
3249+
const builtin_namespace = Type.fromInterned(builtin_root_type).getNamespace(zcu).unwrap().?;
3250+
// We know that the namespace has a `test_functions`...
32473251
const nav_index = zcu.namespacePtr(builtin_namespace).pub_decls.getKeyAdapted(
32483252
try ip.getOrPutString(gpa, pt.tid, "test_functions", .no_embedded_nulls),
32493253
Zcu.Namespace.NameAdapter{ .zcu = zcu },
32503254
).?;
3255+
// ...but it might not be populated, so let's check that!
3256+
if (zcu.failed_analysis.contains(.wrap(.{ .nav_val = nav_index })) or
3257+
zcu.transitive_failed_analysis.contains(.wrap(.{ .nav_val = nav_index })) or
3258+
ip.getNav(nav_index).status != .fully_resolved)
32513259
{
3252-
// We have to call `ensureNavValUpToDate` here in case `builtin.test_functions`
3253-
// was not referenced by start code.
3254-
zcu.sema_prog_node = main_progress_node.start("Semantic Analysis", 0);
3255-
defer {
3256-
zcu.sema_prog_node.end();
3257-
zcu.sema_prog_node = std.Progress.Node.none;
3258-
}
3259-
pt.ensureNavValUpToDate(nav_index) catch |err| switch (err) {
3260-
error.AnalysisFail => return,
3261-
error.OutOfMemory => return error.OutOfMemory,
3262-
};
3260+
// The value of `builtin.test_functions` was either never referenced, or failed analysis.
3261+
// Either way, we don't need to do anything.
3262+
return;
32633263
}
32643264

3265+
// Okay, `builtin.test_functions` is (potentially) referenced and valid. Our job now is to swap
3266+
// its placeholder `&.{}` value for the actual list of all test functions.
3267+
32653268
const test_fns_val = zcu.navValue(nav_index);
32663269
const test_fn_ty = test_fns_val.typeOf(zcu).slicePtrFieldType(zcu).childType(zcu);
32673270

@@ -3363,17 +3366,8 @@ pub fn populateTestFunctions(
33633366
} });
33643367
ip.mutateVarInit(test_fns_val.toIntern(), new_init);
33653368
}
3366-
{
3367-
assert(zcu.codegen_prog_node.index == .none);
3368-
zcu.codegen_prog_node = main_progress_node.start("Code Generation", 0);
3369-
defer {
3370-
zcu.codegen_prog_node.end();
3371-
zcu.codegen_prog_node = std.Progress.Node.none;
3372-
}
3373-
3374-
// The linker thread is not running, so we actually need to dispatch this task directly.
3375-
@import("../link.zig").linkTestFunctionsNav(pt, nav_index);
3376-
}
3369+
// The linker thread is not running, so we actually need to dispatch this task directly.
3370+
@import("../link.zig").linkTestFunctionsNav(pt, nav_index);
33773371
}
33783372

33793373
/// Stores an error in `pt.zcu.failed_files` for this file, and sets the file

0 commit comments

Comments
 (0)