build: detect LazyPath misuse

.github/workflows/ci.yaml

@@ -5,6 +5,7 @@ on:
     branches:
       - master
       - llvm19
+  workflow_dispatch:
 concurrency:
   # Cancels pending runs when a PR gets updated.
   group: ${{ github.head_ref || github.run_id }}-${{ github.actor }}

lib/std/Build.zig

@@ -23,6 +23,11 @@ pub const Module = @import("Build/Module.zig");
 pub const Watch = @import("Build/Watch.zig");
 pub const Fuzz = @import("Build/Fuzz.zig");
 
+const ThreadLocalData = struct {
+    current_step: ?*Step = null,
+};
+pub threadlocal var thread_local: ThreadLocalData = .{};
+
 /// Shared state among all Build instances.
 graph: *Graph,
 install_tls: TopLevelStep,
@@ -2252,6 +2257,38 @@ pub const LazyPath = union(enum) {
         sub_path: []const u8,
     },
 
+    // todo: instead of an eql, we might want a hashing function for fast lookup
+    pub fn eql(lazy_path: LazyPath, other: LazyPath) bool {
+        return switch (lazy_path) {
+            .src_path => |sp| switch (other) {
+                .src_path => |sp_other| (
+                    sp.owner == sp_other.owner and
+                    std.mem.eql(u8, sp.sub_path, sp_other.sub_path)
+                ),
+                else => false,
+            },
+            .generated => |g| switch (other) {
+                .generated => |g_other| (
+                    g.file == g_other.file and
+                    g.up == g_other.up and
+                    std.mem.eql(u8, g.sub_path, g_other.sub_path)
+                ),
+                else => false,
+            },
+            .cwd_relative => |c| switch (other) {
+                .cwd_relative => |c_other| std.mem.eql(u8, c, c_other),
+                else => false,
+            },
+            .dependency => |d| switch (other) {
+                .dependency => |d_other| (
+                    d.dependency == d_other.dependency and
+                    std.mem.eql(u8, d.sub_path, d_other.sub_path)
+                ),
+                else => false,
+            },
+        };
+    }
+
     /// Returns a lazy path referring to the directory containing this path.
     ///
     /// The dirname is not allowed to escape the logical root for underlying path.
@@ -2349,12 +2386,37 @@ pub const LazyPath = union(enum) {
         };
     }
 
-    /// Adds dependencies this file source implies to the given step.
-    pub fn addStepDependencies(lazy_path: LazyPath, other_step: *Step) void {
-        switch (lazy_path) {
-            .src_path, .cwd_relative, .dependency => {},
-            .generated => |gen| other_step.dependOn(gen.file.step),
-        }
+    pub fn getPathInGenerateStep(lazy_path: LazyPath, src_builder: *Build) Cache.Path {
+        const current_step = thread_local.current_step orelse @panic(
+            "getPathInGenerateStep called on LazyPath outside of any step's make function"
+        );
+        const generated = switch (lazy_path) {
+            .generated => |generated| generated,
+            else => std.debug.panic(
+                "called getPathInGenerateStep on a non generate lazy path '{s}'",
+                .{ lazy_path.getDisplayName() },
+            ),
+        };
+        if (current_step != generated.file.step) std.debug.panic(
+            "getPathInGenerateStep called for lazy path '{s}' in step '{s}' but it is generated in step '{s}'",
+            .{ lazy_path.getDisplayName(), current_step.name, generated.file.step.name },
+        );
+        return lazy_path.getPathNoChecks(src_builder, null);
+    }
+
+    /// Call during the make phase.  Resolves the given lazy path, given that the
+    /// lazy path is a dependency of the given step, and, the step is a dependecy
+    /// of the current step being built.
+    pub fn getPathFromDependency(lazy_path: LazyPath, dep_step: *Step) Cache.Path {
+        const current_step = thread_local.current_step orelse @panic(
+            "getPath called on LazyPath outside of any step's make function"
+        );
+        std.debug.assert(current_step != dep_step);
+        if (!dep_step.generates(lazy_path) and !dep_step.dependsOnLazyPathShallow(lazy_path)) std.debug.panic(
+            "step '{s}' dependency '{s}' does not depend on nor generate lazy path '{s}'",
+            .{ current_step.name, dep_step.name, lazy_path.getDisplayName() },
+        );
+        return lazy_path.getPathNoChecks(dep_step.owner, current_step);
     }
 
     /// Deprecated, see `getPath3`.
@@ -2373,6 +2435,22 @@ pub const LazyPath = union(enum) {
     /// `asking_step` is only used for debugging purposes; it's the step being
     /// run that is asking for the path.
     pub fn getPath3(lazy_path: LazyPath, src_builder: *Build, asking_step: ?*Step) Cache.Path {
+        const current_step = thread_local.current_step orelse @panic(
+            "getPath called on LazyPath outside of any step's make function"
+        );
+        if (asking_step) |a| {
+            if (current_step != a) @panic(
+                "getPath called on LazyPath outside of the asking step's make function"
+            );
+        }
+        if (!current_step.dependsOnLazyPathShallow(lazy_path)) std.debug.panic(
+            "step '{s}' is missing a dependency on lazy path '{s}'",
+            .{ current_step.name, lazy_path.getDisplayName() },
+        );
+        return lazy_path.getPathNoChecks(src_builder, asking_step);
+    }
+
+    fn getPathNoChecks(lazy_path: LazyPath, src_builder: *Build, asking_step: ?*Step) Cache.Path {
         switch (lazy_path) {
             .src_path => |sp| return .{
                 .root_dir = sp.owner.build_root,

lib/std/Build/Module.zig

@@ -348,7 +348,7 @@ fn addLazyPathDependencies(m: *Module, module: *Module, lazy_path: LazyPath) voi
 
 fn addLazyPathDependenciesOnly(m: *Module, lazy_path: LazyPath) void {
     for (m.depending_steps.keys()) |compile| {
-        lazy_path.addStepDependencies(&compile.step);
+        _ = compile.step.dependOnLazyPathIfNotAlready(lazy_path);
     }
 }
 

lib/std/Build/Step.zig

@@ -17,6 +17,9 @@ dependants: std.ArrayListUnmanaged(*Step),
 /// retain previous value, or update.
 inputs: Inputs,
 
+/// Direct lazy path dependencies, added with `dependOnLazyPath`.
+lazy_paths: std.ArrayListUnmanaged(Build.LazyPath) = .empty,
+
 state: State,
 /// Set this field to declare an upper bound on the amount of bytes of memory it will
 /// take to run the step. Zero means no limit.
@@ -228,6 +231,17 @@ pub fn init(options: StepOptions) Step {
 pub fn make(s: *Step, options: MakeOptions) error{ MakeFailed, MakeSkipped }!void {
     const arena = s.owner.allocator;
 
+    const old_step = Build.thread_local.current_step;
+    defer Build.thread_local.current_step = old_step;
+
+    const panic_on_nested_makes = true;
+    if (old_step) |old| if (panic_on_nested_makes) std.debug.panic(
+        "make was called on step '{s}' while inside make for step '{s}'",
+        .{ s.name, old.name },
+    );
+
+    Build.thread_local.current_step = s;
+
     s.makeFn(s, options) catch |err| switch (err) {
         error.MakeFailed => return error.MakeFailed,
         error.MakeSkipped => return error.MakeSkipped,
@@ -254,6 +268,47 @@ pub fn dependOn(step: *Step, other: *Step) void {
     step.dependencies.append(other) catch @panic("OOM");
 }
 
+pub fn dependsOnShallow(step: *Step, other: *Step) bool {
+    for (step.dependencies) |d| {
+        if (d == other) return true;
+    }
+    return false;
+}
+
+pub fn dependOnLazyPath(step: *Step, lazy_path: Build.LazyPath) void {
+    if (!dependOnLazyPathIfNotAlready(step, lazy_path)) std.debug.panic(
+        "step '{s}' already depends on this lazy path '{s}'",
+        .{ step.name, lazy_path.getDisplayName() },
+    );
+}
+
+pub fn dependOnLazyPathIfNotAlready(step: *Step, lazy_path: Build.LazyPath) bool {
+    if (dependsOnLazyPathShallow(step, lazy_path))
+        return false;
+    step.lazy_paths.append(step.owner.allocator, lazy_path) catch @panic("OOM");
+    switch (lazy_path) {
+        .src_path, .cwd_relative, .dependency => {},
+        .generated => |gen| step.dependOn(gen.file.step),
+    }
+    return true;
+}
+
+
+pub fn dependsOnLazyPathShallow(step: *Step, lazy_path: Build.LazyPath) bool {
+    for (step.lazy_paths.items) |lp| {
+        if (lazy_path.eql(lp))
+            return true;
+    }
+    return false;
+}
+
+pub fn generates(step: *Step, lazy_path: Build.LazyPath) bool {
+    return switch (lazy_path) {
+        .generated => |g| g.file.step == step,
+        .src_path, .cwd_relative, .dependency => false,
+    };
+}
+
 pub fn getStackTrace(s: *Step) ?std.builtin.StackTrace {
     var len: usize = 0;
     while (len < s.debug_stack_trace.len and s.debug_stack_trace[len] != 0) {

lib/std/Build/Step/CheckFile.zig

@@ -38,7 +38,7 @@ pub fn create(
         .expected_matches = owner.dupeStrings(options.expected_matches),
         .expected_exact = options.expected_exact,
     };
-    check_file.source.addStepDependencies(&check_file.step);
+    check_file.step.dependOnLazyPath(check_file.source);
     return check_file;
 }
 

lib/std/Build/Step/CheckObject.zig

@@ -38,7 +38,7 @@ pub fn create(
         .checks = std.ArrayList(Check).init(gpa),
         .obj_format = obj_format,
     };
-    check_object.source.addStepDependencies(&check_object.step);
+    check_object.step.dependOnLazyPath(check_object.source);
     return check_object;
 }
 
@@ -348,6 +348,7 @@ fn checkExactInner(check_object: *CheckObject, phrase: []const u8, lazy_path: ?s
     assert(check_object.checks.items.len > 0);
     const last = &check_object.checks.items[check_object.checks.items.len - 1];
     last.exact(.{ .string = check_object.step.owner.dupe(phrase), .lazy_path = lazy_path });
+    if (lazy_path) |p| _ = check_object.step.dependOnLazyPathIfNotAlready(p);
 }
 
 /// Adds a fuzzy match phrase to the latest created Check.
@@ -369,6 +370,7 @@ fn checkContainsInner(check_object: *CheckObject, phrase: []const u8, lazy_path:
     assert(check_object.checks.items.len > 0);
     const last = &check_object.checks.items[check_object.checks.items.len - 1];
     last.contains(.{ .string = check_object.step.owner.dupe(phrase), .lazy_path = lazy_path });
+    if (lazy_path) |p| _ = check_object.step.dependOnLazyPathIfNotAlready(p);
 }
 
 /// Adds an exact match phrase with variable extractor to the latest created Check.
@@ -386,6 +388,7 @@ fn checkExtractInner(check_object: *CheckObject, phrase: []const u8, lazy_path:
     assert(check_object.checks.items.len > 0);
     const last = &check_object.checks.items[check_object.checks.items.len - 1];
     last.extract(.{ .string = check_object.step.owner.dupe(phrase), .lazy_path = lazy_path });
+    if (lazy_path) |p| _ = check_object.step.dependOnLazyPathIfNotAlready(p);
 }
 
 /// Adds another searched phrase to the latest created Check
@@ -404,6 +407,7 @@ fn checkNotPresentInner(check_object: *CheckObject, phrase: []const u8, lazy_pat
     assert(check_object.checks.items.len > 0);
     const last = &check_object.checks.items[check_object.checks.items.len - 1];
     last.notPresent(.{ .string = check_object.step.owner.dupe(phrase), .lazy_path = lazy_path });
+    if (lazy_path) |p| _ = check_object.step.dependOnLazyPathIfNotAlready(p);
 }
 
 /// Creates a new check checking in the file headers (section, program headers, etc.).

lib/std/Build/Step/Compile.zig

@@ -423,22 +423,25 @@ pub fn create(owner: *std.Build, options: Options) *Compile {
 
     compile.root_module.init(owner, options.root_module, compile);
 
+    // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+    //compile.step.dependOnLazyPath(compile.getEmittedBin());
+
     if (options.zig_lib_dir) |lp| {
         compile.zig_lib_dir = lp.dupe(compile.step.owner);
-        lp.addStepDependencies(&compile.step);
+        compile.step.dependOnLazyPath(lp);
     }
 
     if (options.test_runner) |lp| {
         compile.test_runner = lp.dupe(compile.step.owner);
-        lp.addStepDependencies(&compile.step);
+        compile.step.dependOnLazyPath(lp);
     }
 
     // Only the PE/COFF format has a Resource Table which is where the manifest
     // gets embedded, so for any other target the manifest file is just ignored.
     if (target.ofmt == .coff) {
         if (options.win32_manifest) |lp| {
             compile.win32_manifest = lp.dupe(compile.step.owner);
-            lp.addStepDependencies(&compile.step);
+            compile.step.dependOnLazyPath(lp);
         }
     }
 
@@ -485,7 +488,7 @@ pub fn installHeader(cs: *Compile, source: LazyPath, dest_rel_path: []const u8)
     } };
     cs.installed_headers.append(installation) catch @panic("OOM");
     cs.addHeaderInstallationToIncludeTree(installation);
-    installation.getSource().addStepDependencies(&cs.step);
+    cs.step.dependOnLazyPath(installation.getSource());
 }
 
 /// Marks headers from the specified directory for installation alongside this artifact.
@@ -505,7 +508,7 @@ pub fn installHeadersDirectory(
     } };
     cs.installed_headers.append(installation) catch @panic("OOM");
     cs.addHeaderInstallationToIncludeTree(installation);
-    installation.getSource().addStepDependencies(&cs.step);
+    cs.step.dependOnLazyPath(installation.getSource());
 }
 
 /// Marks the specified config header for installation alongside this artifact.
@@ -524,7 +527,7 @@ pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void {
         const installation_copy = installation.dupe(lib.step.owner);
         cs.installed_headers.append(installation_copy) catch @panic("OOM");
         cs.addHeaderInstallationToIncludeTree(installation_copy);
-        installation_copy.getSource().addStepDependencies(&cs.step);
+        cs.step.dependOnLazyPath(installation_copy.getSource());
     }
 }
 
@@ -578,13 +581,13 @@ pub const setLinkerScriptPath = setLinkerScript;
 pub fn setLinkerScript(compile: *Compile, source: LazyPath) void {
     const b = compile.step.owner;
     compile.linker_script = source.dupe(b);
-    source.addStepDependencies(&compile.step);
+    compile.step.dependOnLazyPath(source);
 }
 
 pub fn setVersionScript(compile: *Compile, source: LazyPath) void {
     const b = compile.step.owner;
     compile.version_script = source.dupe(b);
-    source.addStepDependencies(&compile.step);
+    compile.step.dependOnLazyPath(source);
 }
 
 pub fn forceUndefinedSymbol(compile: *Compile, symbol_name: []const u8) void {
@@ -1200,7 +1203,10 @@ fn getZigArgs(compile: *Compile, fuzz: bool) ![][]const u8 {
                                 const included_in_lib_or_obj = !my_responsibility and
                                     (dep.compile.?.kind == .lib or dep.compile.?.kind == .obj);
                                 if (!already_linked and !included_in_lib_or_obj) {
-                                    try zig_args.append(other.getEmittedBin().getPath2(b, step));
+                                    const bin = other.getEmittedBin().getPathFromDependency(&other.step);
+                                    try zig_args.append(other.step.owner.pathResolve(
+                                        &.{ bin.root_dir.path orelse ".", bin.sub_path}
+                                    ));
                                     total_linker_objects += 1;
                                 }
                             },
@@ -1837,9 +1843,10 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
     if (compile.kind == .lib and compile.linkage != null and compile.linkage.? == .dynamic and
         compile.version != null and std.Build.wantSharedLibSymLinks(compile.rootModuleTarget()))
     {
+        const bin_path = compile.getEmittedBin().getPathInGenerateStep(b);
         try doAtomicSymLinks(
             step,
-            compile.getEmittedBin().getPath2(b, step),
+            b.pathResolve(&.{ bin_path.root_dir.path orelse ".", bin_path.sub_path}),
             compile.major_only_filename.?,
             compile.name_only_filename.?,
         );

lib/std/Build/Step/InstallArtifact.zig

@@ -106,11 +106,19 @@ pub fn create(owner: *std.Build, artifact: *Step.Compile, options: Options) *Ins
 
     install_artifact.step.dependOn(&artifact.step);
 
-    if (install_artifact.dest_dir != null) install_artifact.emitted_bin = artifact.getEmittedBin();
-    if (install_artifact.pdb_dir != null) install_artifact.emitted_pdb = artifact.getEmittedPdb();
+    if (install_artifact.dest_dir != null) {
+        install_artifact.emitted_bin = artifact.getEmittedBin();
+        install_artifact.step.dependOnLazyPath(install_artifact.emitted_bin.?);
+    }
+    if (install_artifact.pdb_dir != null) {
+        install_artifact.emitted_pdb = artifact.getEmittedPdb();
+        install_artifact.step.dependOnLazyPath(install_artifact.emitted_pdb.?);
+    }
     // https://github.com/ziglang/zig/issues/9698
-    //if (install_artifact.h_dir != null) install_artifact.emitted_h = artifact.getEmittedH();
-    if (install_artifact.implib_dir != null) install_artifact.emitted_implib = artifact.getEmittedImplib();
+    if (install_artifact.implib_dir != null) {
+        install_artifact.emitted_implib = artifact.getEmittedImplib();
+        install_artifact.step.dependOnLazyPath(install_artifact.emitted_implib.?);
+    }
 
     return install_artifact;
 }
@@ -176,7 +184,7 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
 
         for (install_artifact.artifact.installed_headers.items) |installation| switch (installation) {
             .file => |file| {
-                const src_path = file.source.getPath3(b, step);
+                const src_path = file.source.getPathFromDependency(&install_artifact.artifact.step);
                 const full_h_path = b.getInstallPath(h_dir, file.dest_rel_path);
                 const p = fs.Dir.updateFile(src_path.root_dir.handle, src_path.sub_path, cwd, full_h_path, .{}) catch |err| {
                     return step.fail("unable to update file from '{s}' to '{s}': {s}", .{
@@ -186,7 +194,7 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
                 all_cached = all_cached and p == .fresh;
             },
             .directory => |dir| {
-                const src_dir_path = dir.source.getPath3(b, step);
+                const src_dir_path = dir.source.getPathFromDependency(&install_artifact.artifact.step);
                 const full_h_prefix = b.getInstallPath(h_dir, dir.dest_rel_path);
 
                 var src_dir = src_dir_path.root_dir.handle.openDir(src_dir_path.sub_path, .{ .iterate = true }) catch |err| {