From 4d7818a76ad951f0a16c3831b31841430b1368f7 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Tue, 12 Nov 2024 19:33:50 +0000
Subject: [PATCH 1/4] compiler: allow files with AstGen errors to undergo
 semantic analysis

This commit enhances AstGen to introduce a form of error resilience
which allows valid ZIR to be emitted even when AstGen errors occur.

When a non-fatal AstGen error (e.g. `appendErrorNode`) occurs, ZIR
generation is not affected; the error is added to `astgen.errors` and
ultimately to the errors stored in `extra`, but that doesn't stop us
getting valid ZIR. Fatal AstGen errors (e.g. `failNode`) are a bit
trickier. These errors return `error.AnalysisFail`, which is propagated
up the stack. In theory, any parent expression can catch this error and
handle it, continuing ZIR generation whilst throwing away whatever was
lost. For now, we only do this in one place: when creating declarations.
If a call to `fnDecl`, `comptimeDecl`, `globalVarDecl`, etc, returns
`error.AnalysisFail`, the `declaration` instruction is still created,
but its body simply contains the new `extended(astgen_error())`
instruction, which instructs Sema to terminate semantic analysis with a
transitive error. This means that a fatal AstGen error causes the
innermost declaration containing the error to fail, but the rest of the
file remains intact.

If a source file contains parse errors, or an `error.AnalysisFail`
happens when lowering the top-level struct (e.g. there is an error in
one of its fields, or a name has multiple declarations), then lowering
for the entire file fails. Alongside the existing `Zir.hasCompileErrors`
query, this commit introduces `Zir.loweringFailed`, which returns `true`
only in this case.

The end result here is that files with AstGen failures will almost
always still emit valid ZIR, and hence can undergo semantic analysis on
the parts of the file which are (from AstGen's perspective) valid. This
is a noteworthy improvement to UX, but the main motivation here is
actually incremental compilation. Previously, AstGen failures caused
lots of semantic analysis work to be thrown out, because all `AnalUnit`s
in the file required re-analysis so as to trigger necessary transitive
failures and remove stored compile errors which would no longer make
sense (because a fresh compilation of this code would not emit those
errors, as the units those errors applied to would fail sooner due to
referencing a failed file). Now, this case only applies when a file has
severe top-level errors, which is far less common than something like
having an unused variable.

Lastly, this commit changes a few errors in `AstGen` to become fatal
when they were previously non-fatal and vice versa. If there is still a
reasonable way to continue AstGen and lower to ZIR after an error, it is
non-fatal; otherwise, it is fatal. For instance, `comptime const`, while
redundant syntax, has a clear meaning we can lower; on the other hand,
using an undeclared identifer has no sane lowering, so must trigger a
fatal error.
---
 lib/std/multi_array_list.zig                  |   6 +
 lib/std/zig/AstGen.zig                        | 178 ++++++++++++++----
 lib/std/zig/Zir.zig                           |  24 ++-
 src/Sema.zig                                  |   1 +
 src/Zcu/PerThread.zig                         |  43 +++--
 src/main.zig                                  |   4 +-
 src/print_zir.zig                             |   1 +
 .../access_invalid_typeInfo_decl.zig          |   9 +-
 .../astgen_sema_errors_combined.zig           |  17 ++
 .../colliding_invalid_top_level_functions.zig |   9 +-
 ...de_comptime_function_has_compile_error.zig |   2 +
 .../compile_errors/invalid_compare_string.zig |  20 +-
 .../cases/compile_errors/invalid_decltest.zig |   2 +-
 ...elled_type_with_pointer_only_reference.zig |   4 -
 .../noreturn_builtins_divert_control_flow.zig |   3 +-
 test/cases/function_redeclaration.zig         |   6 -
 test/cases/unused_vars.zig                    |   2 +-
 17 files changed, 238 insertions(+), 93 deletions(-)
 create mode 100644 test/cases/compile_errors/astgen_sema_errors_combined.zig

diff --git a/lib/std/multi_array_list.zig b/lib/std/multi_array_list.zig
index 71ccb68b99..f928617211 100644
--- a/lib/std/multi_array_list.zig
+++ b/lib/std/multi_array_list.zig
@@ -74,6 +74,12 @@ pub fn MultiArrayList(comptime T: type) type {
             len: usize,
             capacity: usize,
 
+            pub const empty: Slice = .{
+                .ptrs = undefined,
+                .len = 0,
+                .capacity = 0,
+            };
+
             pub fn items(self: Slice, comptime field: Field) []FieldType(field) {
                 const F = FieldType(field);
                 if (self.capacity == 0) {
diff --git a/lib/std/zig/AstGen.zig b/lib/std/zig/AstGen.zig
index a9b518357b..51291456d1 100644
--- a/lib/std/zig/AstGen.zig
+++ b/lib/std/zig/AstGen.zig
@@ -172,9 +172,9 @@ pub fn generate(gpa: Allocator, tree: Ast) Allocator.Error!Zir {
     };
     defer gz_instructions.deinit(gpa);
 
-    // The AST -> ZIR lowering process assumes an AST that does not have any
-    // parse errors.
-    if (tree.errors.len == 0) {
+    // The AST -> ZIR lowering process assumes an AST that does not have any parse errors.
+    // Parse errors, or AstGen errors in the root struct, are considered "fatal", so we emit no ZIR.
+    const fatal = if (tree.errors.len == 0) fatal: {
         if (AstGen.structDeclInner(
             &gen_scope,
             &gen_scope.base,
@@ -184,13 +184,15 @@ pub fn generate(gpa: Allocator, tree: Ast) Allocator.Error!Zir {
             0,
         )) |struct_decl_ref| {
             assert(struct_decl_ref.toIndex().? == .main_struct_inst);
+            break :fatal false;
         } else |err| switch (err) {
             error.OutOfMemory => return error.OutOfMemory,
-            error.AnalysisFail => {}, // Handled via compile_errors below.
+            error.AnalysisFail => break :fatal true, // Handled via compile_errors below.
         }
-    } else {
+    } else fatal: {
         try lowerAstErrors(&astgen);
-    }
+        break :fatal true;
+    };
 
     const err_index = @intFromEnum(Zir.ExtraIndex.compile_errors);
     if (astgen.compile_errors.items.len == 0) {
@@ -228,8 +230,8 @@ pub fn generate(gpa: Allocator, tree: Ast) Allocator.Error!Zir {
         }
     }
 
-    return Zir{
-        .instructions = astgen.instructions.toOwnedSlice(),
+    return .{
+        .instructions = if (fatal) .empty else astgen.instructions.toOwnedSlice(),
         .string_bytes = try astgen.string_bytes.toOwnedSlice(gpa),
         .extra = try astgen.extra.toOwnedSlice(gpa),
     };
@@ -2101,7 +2103,7 @@ fn comptimeExprAst(
 ) InnerError!Zir.Inst.Ref {
     const astgen = gz.astgen;
     if (gz.is_comptime) {
-        return astgen.failNode(node, "redundant comptime keyword in already comptime scope", .{});
+        try astgen.appendErrorNode(node, "redundant comptime keyword in already comptime scope", .{});
     }
     const tree = astgen.tree;
     const node_datas = tree.nodes.items(.data);
@@ -3275,6 +3277,9 @@ fn varDecl(
                 try astgen.appendErrorTok(comptime_token, "'comptime const' is redundant; instead wrap the initialization expression with 'comptime'", .{});
             }
 
+            // `comptime const` is a non-fatal error; treat it like the init was marked `comptime`.
+            const force_comptime = var_decl.comptime_token != null;
+
             // Depending on the type of AST the initialization expression is, we may need an lvalue
             // or an rvalue as a result location. If it is an rvalue, we can use the instruction as
             // the variable, no memory location needed.
@@ -3288,7 +3293,7 @@ fn varDecl(
                 } else .{ .rl = .none, .ctx = .const_init };
                 const prev_anon_name_strategy = gz.anon_name_strategy;
                 gz.anon_name_strategy = .dbg_var;
-                const init_inst = try reachableExpr(gz, scope, result_info, var_decl.ast.init_node, node);
+                const init_inst = try reachableExprComptime(gz, scope, result_info, var_decl.ast.init_node, node, force_comptime);
                 gz.anon_name_strategy = prev_anon_name_strategy;
 
                 try gz.addDbgVar(.dbg_var_val, ident_name, init_inst);
@@ -3358,7 +3363,7 @@ fn varDecl(
             const prev_anon_name_strategy = gz.anon_name_strategy;
             gz.anon_name_strategy = .dbg_var;
             defer gz.anon_name_strategy = prev_anon_name_strategy;
-            const init_inst = try reachableExpr(gz, scope, init_result_info, var_decl.ast.init_node, node);
+            const init_inst = try reachableExprComptime(gz, scope, init_result_info, var_decl.ast.init_node, node, force_comptime);
 
             // The const init expression may have modified the error return trace, so signal
             // to Sema that it should save the new index for restoring later.
@@ -3503,7 +3508,7 @@ fn assignDestructure(gz: *GenZir, scope: *Scope, node: Ast.Node.Index) InnerErro
 
     const full = tree.assignDestructure(node);
     if (full.comptime_token != null and gz.is_comptime) {
-        return astgen.failNode(node, "redundant comptime keyword in already comptime scope", .{});
+        return astgen.appendErrorNode(node, "redundant comptime keyword in already comptime scope", .{});
     }
 
     // If this expression is marked comptime, we must wrap the whole thing in a comptime block.
@@ -3562,7 +3567,7 @@ fn assignDestructureMaybeDecls(
 
     const full = tree.assignDestructure(node);
     if (full.comptime_token != null and gz.is_comptime) {
-        return astgen.failNode(node, "redundant comptime keyword in already comptime scope", .{});
+        try astgen.appendErrorNode(node, "redundant comptime keyword in already comptime scope", .{});
     }
 
     const is_comptime = full.comptime_token != null or gz.is_comptime;
@@ -3676,6 +3681,7 @@ fn assignDestructureMaybeDecls(
 
     if (full.comptime_token != null and !any_non_const_variables) {
         try astgen.appendErrorTok(full.comptime_token.?, "'comptime const' is redundant; instead wrap the initialization expression with 'comptime'", .{});
+        // Note that this is non-fatal; we will still evaluate at comptime.
     }
 
     // If this expression is marked comptime, we must wrap it in a comptime block.
@@ -4125,8 +4131,8 @@ fn fnDecl(
     // The source slice is added towards the *end* of this function.
     astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column));
 
-    // missing function name already happened in scanContainer()
-    const fn_name_token = fn_proto.name_token orelse return error.AnalysisFail;
+    // missing function name already checked in scanContainer()
+    const fn_name_token = fn_proto.name_token.?;
 
     // We insert this at the beginning so that its instruction index marks the
     // start of the top level declaration.
@@ -5167,8 +5173,7 @@ fn structDeclInner(
 
         if (is_comptime) {
             switch (layout) {
-                .@"packed" => return astgen.failTok(member.comptime_token.?, "packed struct fields cannot be marked comptime", .{}),
-                .@"extern" => return astgen.failTok(member.comptime_token.?, "extern struct fields cannot be marked comptime", .{}),
+                .@"packed", .@"extern" => return astgen.failTok(member.comptime_token.?, "{s} struct fields cannot be marked comptime", .{@tagName(layout)}),
                 .auto => any_comptime_fields = true,
             }
         } else {
@@ -5195,7 +5200,7 @@ fn structDeclInner(
 
         if (have_align) {
             if (layout == .@"packed") {
-                try astgen.appendErrorNode(member.ast.align_expr, "unable to override alignment of packed struct fields", .{});
+                return astgen.failNode(member.ast.align_expr, "unable to override alignment of packed struct fields", .{});
             }
             any_aligned_fields = true;
             const align_ref = try expr(&block_scope, &namespace.base, coerced_align_ri, member.ast.align_expr);
@@ -5289,8 +5294,7 @@ fn tupleDecl(
 
     switch (layout) {
         .auto => {},
-        .@"extern" => return astgen.failNode(node, "extern tuples are not supported", .{}),
-        .@"packed" => return astgen.failNode(node, "packed tuples are not supported", .{}),
+        .@"extern", .@"packed" => return astgen.failNode(node, "{s} tuples are not supported", .{@tagName(layout)}),
     }
 
     if (backing_int_node != 0) {
@@ -5673,7 +5677,7 @@ fn containerDecl(
                 };
             };
             if (counts.nonexhaustive_node != 0 and container_decl.ast.arg == 0) {
-                try astgen.appendErrorNodeNotes(
+                return astgen.failNodeNotes(
                     node,
                     "non-exhaustive enum missing integer tag type",
                     .{},
@@ -5896,9 +5900,19 @@ fn containerMember(
             const full = tree.fullFnProto(&buf, member_node).?;
             const body = if (node_tags[member_node] == .fn_decl) node_datas[member_node].rhs else 0;
 
+            const prev_decl_index = wip_members.decl_index;
             astgen.fnDecl(gz, scope, wip_members, member_node, body, full) catch |err| switch (err) {
                 error.OutOfMemory => return error.OutOfMemory,
-                error.AnalysisFail => {},
+                error.AnalysisFail => {
+                    wip_members.decl_index = prev_decl_index;
+                    try addFailedDeclaration(
+                        wip_members,
+                        gz,
+                        .{ .named = full.name_token.? },
+                        full.ast.proto_node,
+                        full.visib_token != null,
+                    );
+                },
             };
         },
 
@@ -5907,28 +5921,77 @@ fn containerMember(
         .simple_var_decl,
         .aligned_var_decl,
         => {
-            astgen.globalVarDecl(gz, scope, wip_members, member_node, tree.fullVarDecl(member_node).?) catch |err| switch (err) {
+            const full = tree.fullVarDecl(member_node).?;
+            const prev_decl_index = wip_members.decl_index;
+            astgen.globalVarDecl(gz, scope, wip_members, member_node, full) catch |err| switch (err) {
                 error.OutOfMemory => return error.OutOfMemory,
-                error.AnalysisFail => {},
+                error.AnalysisFail => {
+                    wip_members.decl_index = prev_decl_index;
+                    try addFailedDeclaration(
+                        wip_members,
+                        gz,
+                        .{ .named = full.ast.mut_token + 1 },
+                        member_node,
+                        full.visib_token != null,
+                    );
+                },
             };
         },
 
         .@"comptime" => {
+            const prev_decl_index = wip_members.decl_index;
             astgen.comptimeDecl(gz, scope, wip_members, member_node) catch |err| switch (err) {
                 error.OutOfMemory => return error.OutOfMemory,
-                error.AnalysisFail => {},
+                error.AnalysisFail => {
+                    wip_members.decl_index = prev_decl_index;
+                    try addFailedDeclaration(
+                        wip_members,
+                        gz,
+                        .@"comptime",
+                        member_node,
+                        false,
+                    );
+                },
             };
         },
         .@"usingnamespace" => {
+            const prev_decl_index = wip_members.decl_index;
             astgen.usingnamespaceDecl(gz, scope, wip_members, member_node) catch |err| switch (err) {
                 error.OutOfMemory => return error.OutOfMemory,
-                error.AnalysisFail => {},
+                error.AnalysisFail => {
+                    wip_members.decl_index = prev_decl_index;
+                    try addFailedDeclaration(
+                        wip_members,
+                        gz,
+                        .@"usingnamespace",
+                        member_node,
+                        is_pub: {
+                            const main_tokens = tree.nodes.items(.main_token);
+                            const token_tags = tree.tokens.items(.tag);
+                            const main_token = main_tokens[member_node];
+                            break :is_pub main_token > 0 and token_tags[main_token - 1] == .keyword_pub;
+                        },
+                    );
+                },
             };
         },
         .test_decl => {
+            const prev_decl_index = wip_members.decl_index;
+            // We need to have *some* decl here so that the decl count matches what's expected.
+            // Since it doesn't strictly matter *what* this is, let's save ourselves the trouble
+            // of duplicating the test name logic, and just assume this is an unnamed test.
             astgen.testDecl(gz, scope, wip_members, member_node) catch |err| switch (err) {
                 error.OutOfMemory => return error.OutOfMemory,
-                error.AnalysisFail => {},
+                error.AnalysisFail => {
+                    wip_members.decl_index = prev_decl_index;
+                    try addFailedDeclaration(
+                        wip_members,
+                        gz,
+                        .unnamed_test,
+                        member_node,
+                        false,
+                    );
+                },
             };
         },
         else => unreachable,
@@ -6140,7 +6203,7 @@ fn orelseCatchExpr(
         const payload = payload_token orelse break :blk &else_scope.base;
         const err_str = tree.tokenSlice(payload);
         if (mem.eql(u8, err_str, "_")) {
-            return astgen.failTok(payload, "discard of error capture; omit it instead", .{});
+            try astgen.appendErrorTok(payload, "discard of error capture; omit it instead", .{});
         }
         const err_name = try astgen.identAsString(payload);
 
@@ -6599,7 +6662,7 @@ fn whileExpr(
 
     const is_inline = while_full.inline_token != null;
     if (parent_gz.is_comptime and is_inline) {
-        return astgen.failTok(while_full.inline_token.?, "redundant inline keyword in comptime scope", .{});
+        try astgen.appendErrorTok(while_full.inline_token.?, "redundant inline keyword in comptime scope", .{});
     }
     const loop_tag: Zir.Inst.Tag = if (is_inline) .block_inline else .loop;
     const loop_block = try parent_gz.makeBlockInst(loop_tag, node);
@@ -6889,7 +6952,7 @@ fn forExpr(
 
     const is_inline = for_full.inline_token != null;
     if (parent_gz.is_comptime and is_inline) {
-        return astgen.failTok(for_full.inline_token.?, "redundant inline keyword in comptime scope", .{});
+        try astgen.appendErrorTok(for_full.inline_token.?, "redundant inline keyword in comptime scope", .{});
     }
     const tree = astgen.tree;
     const token_tags = tree.tokens.items(.tag);
@@ -6950,7 +7013,7 @@ fn forExpr(
                     .none;
 
                 if (end_val == .none and is_discard) {
-                    return astgen.failTok(ident_tok, "discard of unbounded counter", .{});
+                    try astgen.appendErrorTok(ident_tok, "discard of unbounded counter", .{});
                 }
 
                 const start_is_zero = nodeIsTriviallyZero(tree, start_node);
@@ -7467,6 +7530,7 @@ fn switchExprErrUnion(
     const err_name = blk: {
         const err_str = tree.tokenSlice(error_payload);
         if (mem.eql(u8, err_str, "_")) {
+            // This is fatal because we already know we're switching on the captured error.
             return astgen.failTok(error_payload, "discard of error capture; omit it instead", .{});
         }
         const err_name = try astgen.identAsString(error_payload);
@@ -7521,7 +7585,7 @@ fn switchExprErrUnion(
 
             const capture_slice = tree.tokenSlice(capture_token);
             if (mem.eql(u8, capture_slice, "_")) {
-                return astgen.failTok(capture_token, "discard of error capture; omit it instead", .{});
+                try astgen.appendErrorTok(capture_token, "discard of error capture; omit it instead", .{});
             }
             const tag_name = try astgen.identAsString(capture_token);
             try astgen.detectLocalShadowing(&case_scope.base, tag_name, capture_token, capture_slice, .capture);
@@ -8018,7 +8082,7 @@ fn switchExpr(
                 break :blk payload_sub_scope;
             const tag_slice = tree.tokenSlice(tag_token);
             if (mem.eql(u8, tag_slice, "_")) {
-                return astgen.failTok(tag_token, "discard of tag capture; omit it instead", .{});
+                try astgen.appendErrorTok(tag_token, "discard of tag capture; omit it instead", .{});
             } else if (case.inline_token == null) {
                 return astgen.failTok(tag_token, "tag capture on non-inline prong", .{});
             }
@@ -13699,6 +13763,8 @@ fn scanContainer(
     const main_tokens = tree.nodes.items(.main_token);
     const token_tags = tree.tokens.items(.tag);
 
+    var any_invalid_declarations = false;
+
     // This type forms a linked list of source tokens declaring the same name.
     const NameEntry = struct {
         tok: Ast.TokenIndex,
@@ -13758,6 +13824,7 @@ fn scanContainer(
                 const ident = main_tokens[member_node] + 1;
                 if (token_tags[ident] != .identifier) {
                     try astgen.appendErrorNode(member_node, "missing function name", .{});
+                    any_invalid_declarations = true;
                     continue;
                 }
                 break :blk .{ .decl, ident };
@@ -13853,6 +13920,7 @@ fn scanContainer(
                     token_bytes,
                 }),
             });
+            any_invalid_declarations = true;
             continue;
         }
 
@@ -13870,6 +13938,7 @@ fn scanContainer(
                             .{},
                         ),
                     });
+                    any_invalid_declarations = true;
                     break;
                 }
                 s = local_val.parent;
@@ -13886,6 +13955,7 @@ fn scanContainer(
                             .{},
                         ),
                     });
+                    any_invalid_declarations = true;
                     break;
                 }
                 s = local_ptr.parent;
@@ -13897,7 +13967,10 @@ fn scanContainer(
         };
     }
 
-    if (!any_duplicates) return decl_count;
+    if (!any_duplicates) {
+        if (any_invalid_declarations) return error.AnalysisFail;
+        return decl_count;
+    }
 
     for (names.keys(), names.values()) |name, first| {
         if (first.next == null) continue;
@@ -13909,6 +13982,7 @@ fn scanContainer(
         try notes.append(astgen.arena, try astgen.errNoteNode(namespace.node, "{s} declared here", .{@tagName(container_kind)}));
         const name_duped = try astgen.arena.dupe(u8, mem.span(astgen.nullTerminatedString(name)));
         try astgen.appendErrorTokNotes(first.tok, "duplicate {s} member name '{s}'", .{ @tagName(container_kind), name_duped }, notes.items);
+        any_invalid_declarations = true;
     }
 
     for (test_names.keys(), test_names.values()) |name, first| {
@@ -13921,6 +13995,7 @@ fn scanContainer(
         try notes.append(astgen.arena, try astgen.errNoteNode(namespace.node, "{s} declared here", .{@tagName(container_kind)}));
         const name_duped = try astgen.arena.dupe(u8, mem.span(astgen.nullTerminatedString(name)));
         try astgen.appendErrorTokNotes(first.tok, "duplicate test name '{s}'", .{name_duped}, notes.items);
+        any_invalid_declarations = true;
     }
 
     for (decltest_names.keys(), decltest_names.values()) |name, first| {
@@ -13933,9 +14008,11 @@ fn scanContainer(
         try notes.append(astgen.arena, try astgen.errNoteNode(namespace.node, "{s} declared here", .{@tagName(container_kind)}));
         const name_duped = try astgen.arena.dupe(u8, mem.span(astgen.nullTerminatedString(name)));
         try astgen.appendErrorTokNotes(first.tok, "duplicate decltest '{s}'", .{name_duped}, notes.items);
+        any_invalid_declarations = true;
     }
 
-    return decl_count;
+    assert(any_invalid_declarations);
+    return error.AnalysisFail;
 }
 
 fn isInferred(astgen: *AstGen, ref: Zir.Inst.Ref) bool {
@@ -14083,6 +14160,37 @@ const DeclarationName = union(enum) {
     @"usingnamespace",
 };
 
+fn addFailedDeclaration(
+    wip_members: *WipMembers,
+    gz: *GenZir,
+    name: DeclarationName,
+    src_node: Ast.Node.Index,
+    is_pub: bool,
+) !void {
+    const decl_inst = try gz.makeDeclaration(src_node);
+    wip_members.nextDecl(decl_inst);
+    var decl_gz = gz.makeSubBlock(&gz.base); // scope doesn't matter here
+    _ = try decl_gz.add(.{
+        .tag = .extended,
+        .data = .{ .extended = .{
+            .opcode = .astgen_error,
+            .small = undefined,
+            .operand = undefined,
+        } },
+    });
+    try setDeclaration(
+        decl_inst,
+        @splat(0), // use a fixed hash to represent an AstGen failure; we don't care about source changes if AstGen still failed!
+        name,
+        gz.astgen.source_line,
+        is_pub,
+        false, // we don't care about exports since semantic analysis will fail
+        .empty,
+        &decl_gz,
+        null,
+    );
+}
+
 /// Sets all extra data for a `declaration` instruction.
 /// Unstacks `value_gz`, `align_gz`, `linksection_gz`, and `addrspace_gz`.
 fn setDeclaration(
diff --git a/lib/std/zig/Zir.zig b/lib/std/zig/Zir.zig
index f2c103f835..2844bda55a 100644
--- a/lib/std/zig/Zir.zig
+++ b/lib/std/zig/Zir.zig
@@ -120,7 +120,21 @@ pub fn bodySlice(zir: Zir, start: usize, len: usize) []Inst.Index {
 }
 
 pub fn hasCompileErrors(code: Zir) bool {
-    return code.extra[@intFromEnum(ExtraIndex.compile_errors)] != 0;
+    if (code.extra[@intFromEnum(ExtraIndex.compile_errors)] != 0) {
+        return true;
+    } else {
+        assert(code.instructions.len != 0); // i.e. lowering did not fail
+        return false;
+    }
+}
+
+pub fn loweringFailed(code: Zir) bool {
+    if (code.instructions.len == 0) {
+        assert(code.hasCompileErrors());
+        return true;
+    } else {
+        return false;
+    }
 }
 
 pub fn deinit(code: *Zir, gpa: Allocator) void {
@@ -2089,7 +2103,14 @@ pub const Inst = struct {
         /// `small` is an `Inst.InplaceOp`.
         inplace_arith_result_ty,
         /// Marks a statement that can be stepped to but produces no code.
+        /// `operand` and `small` are ignored.
         dbg_empty_stmt,
+        /// At this point, AstGen encountered a fatal error which terminated ZIR lowering for this body.
+        /// A file-level error has been reported. Sema should terminate semantic analysis.
+        /// `operand` and `small` are ignored.
+        /// This instruction is always `noreturn`, however, it is not considered as such by ZIR-level queries. This allows AstGen to assume that
+        /// any code may have gone here, avoiding false-positive "unreachable code" errors.
+        astgen_error,
 
         pub const InstData = struct {
             opcode: Extended,
@@ -4065,6 +4086,7 @@ fn findDeclsInner(
                 .inplace_arith_result_ty,
                 .tuple_decl,
                 .dbg_empty_stmt,
+                .astgen_error,
                 => return,
 
                 // `@TypeOf` has a body.
diff --git a/src/Sema.zig b/src/Sema.zig
index 0f8668c4ad..8d2354f4fe 100644
--- a/src/Sema.zig
+++ b/src/Sema.zig
@@ -1360,6 +1360,7 @@ fn analyzeBodyInner(
                         i += 1;
                         continue;
                     },
+                    .astgen_error => return error.AnalysisFail,
                 };
             },
 
diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig
index c6a8b58f41..b12790badb 100644
--- a/src/Zcu/PerThread.zig
+++ b/src/Zcu/PerThread.zig
@@ -185,11 +185,11 @@ pub fn astGenFile(
             log.debug("AstGen cached success: {s}", .{file.sub_file_path});
 
             if (file.zir.hasCompileErrors()) {
-                {
-                    comp.mutex.lock();
-                    defer comp.mutex.unlock();
-                    try zcu.failed_files.putNoClobber(gpa, file, null);
-                }
+                comp.mutex.lock();
+                defer comp.mutex.unlock();
+                try zcu.failed_files.putNoClobber(gpa, file, null);
+            }
+            if (file.zir.loweringFailed()) {
                 file.status = .astgen_failure;
                 return error.AnalysisFail;
             }
@@ -226,7 +226,7 @@ pub fn astGenFile(
     //    single-threaded context, so we need to keep both versions around
     //    until that point in the pipeline. Previous ZIR data is freed after
     //    that.
-    if (file.zir_loaded and !file.zir.hasCompileErrors()) {
+    if (file.zir_loaded and !file.zir.loweringFailed()) {
         assert(file.prev_zir == null);
         const prev_zir_ptr = try gpa.create(Zir);
         file.prev_zir = prev_zir_ptr;
@@ -321,11 +321,11 @@ pub fn astGenFile(
     };
 
     if (file.zir.hasCompileErrors()) {
-        {
-            comp.mutex.lock();
-            defer comp.mutex.unlock();
-            try zcu.failed_files.putNoClobber(gpa, file, null);
-        }
+        comp.mutex.lock();
+        defer comp.mutex.unlock();
+        try zcu.failed_files.putNoClobber(gpa, file, null);
+    }
+    if (file.zir.loweringFailed()) {
         file.status = .astgen_failure;
         return error.AnalysisFail;
     }
@@ -363,7 +363,7 @@ pub fn updateZirRefs(pt: Zcu.PerThread) Allocator.Error!void {
             .file = file,
             .inst_map = .{},
         };
-        if (!new_zir.hasCompileErrors()) {
+        if (!new_zir.loweringFailed()) {
             try Zcu.mapOldZirToNew(gpa, old_zir.*, file.zir, &gop.value_ptr.inst_map);
         }
     }
@@ -379,20 +379,19 @@ pub fn updateZirRefs(pt: Zcu.PerThread) Allocator.Error!void {
 
             const file = updated_file.file;
 
-            if (file.zir.hasCompileErrors()) {
-                // If we mark this as outdated now, users of this inst will just get a transitive analysis failure.
-                // Ultimately, they would end up throwing out potentially useful analysis results.
-                // So, do nothing. We already have the file failure -- that's sufficient for now!
-                continue;
-            }
             const old_inst = tracked_inst.inst.unwrap() orelse continue; // we can't continue tracking lost insts
             const tracked_inst_index = (InternPool.TrackedInst.Index.Unwrapped{
                 .tid = @enumFromInt(tid),
                 .index = @intCast(tracked_inst_unwrapped_index),
             }).wrap(ip);
             const new_inst = updated_file.inst_map.get(old_inst) orelse {
-                // Tracking failed for this instruction. Invalidate associated `src_hash` deps.
-                log.debug("tracking failed for %{d}", .{old_inst});
+                // Tracking failed for this instruction.
+                // This may be due to changes in the ZIR, or AstGen might have failed due to a very broken file.
+                // Either way, invalidate associated `src_hash` deps.
+                log.debug("tracking failed for %{d}{s}", .{
+                    old_inst,
+                    if (file.zir.loweringFailed()) " due to AstGen failure" else "",
+                });
                 tracked_inst.inst = .lost;
                 try zcu.markDependeeOutdated(.not_marked_po, .{ .src_hash = tracked_inst_index });
                 continue;
@@ -494,8 +493,8 @@ pub fn updateZirRefs(pt: Zcu.PerThread) Allocator.Error!void {
 
     for (updated_files.keys(), updated_files.values()) |file_index, updated_file| {
         const file = updated_file.file;
-        if (file.zir.hasCompileErrors()) {
-            // Keep `prev_zir` around: it's the last non-error ZIR.
+        if (file.zir.loweringFailed()) {
+            // Keep `prev_zir` around: it's the last usable ZIR.
             // Don't update the namespace, as we have no new data to update *to*.
         } else {
             const prev_zir = file.prev_zir.?;
diff --git a/src/main.zig b/src/main.zig
index 0d239f1f99..a6bec502be 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -6457,7 +6457,7 @@ fn cmdChangelist(
     file.zir_loaded = true;
     defer file.zir.deinit(gpa);
 
-    if (file.zir.hasCompileErrors()) {
+    if (file.zir.loweringFailed()) {
         var wip_errors: std.zig.ErrorBundle.Wip = undefined;
         try wip_errors.init(gpa);
         defer wip_errors.deinit();
@@ -6492,7 +6492,7 @@ fn cmdChangelist(
     file.zir = try AstGen.generate(gpa, new_tree);
     file.zir_loaded = true;
 
-    if (file.zir.hasCompileErrors()) {
+    if (file.zir.loweringFailed()) {
         var wip_errors: std.zig.ErrorBundle.Wip = undefined;
         try wip_errors.init(gpa);
         defer wip_errors.deinit();
diff --git a/src/print_zir.zig b/src/print_zir.zig
index 808ead0e79..5aae9a1845 100644
--- a/src/print_zir.zig
+++ b/src/print_zir.zig
@@ -623,6 +623,7 @@ const Writer = struct {
             .inplace_arith_result_ty => try self.writeInplaceArithResultTy(stream, extended),
 
             .dbg_empty_stmt => try stream.writeAll("))"),
+            .astgen_error => try stream.writeAll("))"),
         }
     }
 
diff --git a/test/cases/compile_errors/access_invalid_typeInfo_decl.zig b/test/cases/compile_errors/access_invalid_typeInfo_decl.zig
index 6fc9f2085f..05386c22e1 100644
--- a/test/cases/compile_errors/access_invalid_typeInfo_decl.zig
+++ b/test/cases/compile_errors/access_invalid_typeInfo_decl.zig
@@ -1,11 +1,8 @@
-const A = B;
-test "Crash" {
+pub const A = B;
+export fn foo() void {
     _ = @typeInfo(@This()).@"struct".decls[0];
 }
 
 // error
-// backend=stage2
-// target=native
-// is_test=true
 //
-// :1:11: error: use of undeclared identifier 'B'
+// :1:15: error: use of undeclared identifier 'B'
diff --git a/test/cases/compile_errors/astgen_sema_errors_combined.zig b/test/cases/compile_errors/astgen_sema_errors_combined.zig
new file mode 100644
index 0000000000..963cca6b7e
--- /dev/null
+++ b/test/cases/compile_errors/astgen_sema_errors_combined.zig
@@ -0,0 +1,17 @@
+const a = bogus; // astgen error (undeclared identifier)
+const b: u32 = "hi"; // sema error (type mismatch)
+
+comptime {
+    _ = b;
+    @compileError("not hit because 'b' failed");
+}
+
+comptime {
+    @compileError("this should be hit");
+}
+
+// error
+//
+// :1:11: error: use of undeclared identifier 'bogus'
+// :2:16: error: expected type 'u32', found '*const [2:0]u8'
+// :10:5: error: this should be hit
diff --git a/test/cases/compile_errors/colliding_invalid_top_level_functions.zig b/test/cases/compile_errors/colliding_invalid_top_level_functions.zig
index e63810c986..9dfe951494 100644
--- a/test/cases/compile_errors/colliding_invalid_top_level_functions.zig
+++ b/test/cases/compile_errors/colliding_invalid_top_level_functions.zig
@@ -1,13 +1,8 @@
-fn func() bogus {}
-fn func() bogus {}
-export fn entry() usize {
-    return @sizeOf(@TypeOf(func));
-}
+fn func() void {}
+fn func() void {}
 
 // error
 //
 // :1:4: error: duplicate struct member name 'func'
 // :2:4: note: duplicate name here
 // :1:1: note: struct declared here
-// :1:11: error: use of undeclared identifier 'bogus'
-// :2:11: error: use of undeclared identifier 'bogus'
diff --git a/test/cases/compile_errors/constant_inside_comptime_function_has_compile_error.zig b/test/cases/compile_errors/constant_inside_comptime_function_has_compile_error.zig
index 6efed3eb81..2afe5c4630 100644
--- a/test/cases/compile_errors/constant_inside_comptime_function_has_compile_error.zig
+++ b/test/cases/compile_errors/constant_inside_comptime_function_has_compile_error.zig
@@ -19,3 +19,5 @@ export fn entry() void {
 //
 // :4:5: error: unreachable code
 // :4:25: note: control flow is diverted here
+// :4:25: error: aoeu
+// :1:36: note: called from here
diff --git a/test/cases/compile_errors/invalid_compare_string.zig b/test/cases/compile_errors/invalid_compare_string.zig
index 95f6fa4122..7a2bebf6b0 100644
--- a/test/cases/compile_errors/invalid_compare_string.zig
+++ b/test/cases/compile_errors/invalid_compare_string.zig
@@ -1,22 +1,27 @@
 comptime {
     const a = "foo";
-    if (a == "foo") unreachable;
+    if (a != "foo") unreachable;
 }
 comptime {
     const a = "foo";
-    if (a == ("foo")) unreachable; // intentionally allow
+    if (a == "foo") {} else unreachable;
+}
+comptime {
+    const a = "foo";
+    if (a != ("foo")) {} // intentionally allow
+    if (a == ("foo")) {} // intentionally allow
 }
 comptime {
     const a = "foo";
     switch (a) {
-        "foo" => unreachable,
-        else => {},
+        "foo" => {},
+        else => unreachable,
     }
 }
 comptime {
     const a = "foo";
     switch (a) {
-        ("foo") => unreachable, // intentionally allow
+        ("foo") => {}, // intentionally allow
         else => {},
     }
 }
@@ -25,5 +30,6 @@ comptime {
 // backend=stage2
 // target=native
 //
-// :3:11: error: cannot compare strings with ==
-// :12:9: error: cannot switch on strings
+// :3:11: error: cannot compare strings with !=
+// :7:11: error: cannot compare strings with ==
+// :17:9: error: cannot switch on strings
diff --git a/test/cases/compile_errors/invalid_decltest.zig b/test/cases/compile_errors/invalid_decltest.zig
index cde984f366..e6a9122e16 100644
--- a/test/cases/compile_errors/invalid_decltest.zig
+++ b/test/cases/compile_errors/invalid_decltest.zig
@@ -1,6 +1,6 @@
 export fn foo() void {
     const a = 1;
-    struct {
+    _ = struct {
         test a {}
     };
 }
diff --git a/test/cases/compile_errors/misspelled_type_with_pointer_only_reference.zig b/test/cases/compile_errors/misspelled_type_with_pointer_only_reference.zig
index 9d22dba037..e2a2d2e635 100644
--- a/test/cases/compile_errors/misspelled_type_with_pointer_only_reference.zig
+++ b/test/cases/compile_errors/misspelled_type_with_pointer_only_reference.zig
@@ -28,10 +28,6 @@ fn foo() void {
     _ = jd;
 }
 
-export fn entry() usize {
-    return @sizeOf(@TypeOf(foo));
-}
-
 // error
 // backend=stage2
 // target=native
diff --git a/test/cases/compile_errors/noreturn_builtins_divert_control_flow.zig b/test/cases/compile_errors/noreturn_builtins_divert_control_flow.zig
index 32627f1652..755c7f6c31 100644
--- a/test/cases/compile_errors/noreturn_builtins_divert_control_flow.zig
+++ b/test/cases/compile_errors/noreturn_builtins_divert_control_flow.zig
@@ -7,7 +7,7 @@ export fn entry2() void {
     @panic("");
 }
 export fn entry3() void {
-    @compileError("");
+    @compileError("expect to hit this");
     @compileError("");
 }
 
@@ -21,3 +21,4 @@ export fn entry3() void {
 // :6:5: note: control flow is diverted here
 // :11:5: error: unreachable code
 // :10:5: note: control flow is diverted here
+// :10:5: error: expect to hit this
diff --git a/test/cases/function_redeclaration.zig b/test/cases/function_redeclaration.zig
index 2b8dc4c15d..3849a49e1f 100644
--- a/test/cases/function_redeclaration.zig
+++ b/test/cases/function_redeclaration.zig
@@ -2,14 +2,8 @@
 fn entry() void {}
 fn entry() void {}
 
-fn foo() void {
-    var foo = 1234;
-}
-
 // error
 //
 // :2:4: error: duplicate struct member name 'entry'
 // :3:4: note: duplicate name here
 // :2:1: note: struct declared here
-// :6:9: error: local variable shadows declaration of 'foo'
-// :5:1: note: declared here
diff --git a/test/cases/unused_vars.zig b/test/cases/unused_vars.zig
index 15ed691843..45a9a45cb1 100644
--- a/test/cases/unused_vars.zig
+++ b/test/cases/unused_vars.zig
@@ -1,6 +1,6 @@
 pub fn main() void {
     const x = 1;
-    const y, var z = .{ 2, 3 };
+    const y, var z: u32 = .{ 2, 3 };
 }
 
 // error

From 7f3211a101d8763ec5f0009b219f6819dba2cd35 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 5 Dec 2024 16:59:50 +0000
Subject: [PATCH 2/4] compiler: incremental compilation fixes

The previous commit exposed some bugs in incremental compilation. This
commit fixes those, and adds a little more logging for debugging
incremental compilation.

Also, allow `ast-check -t` to dump ZIR when there are non-fatal AstGen
errors.
---
 src/Compilation.zig   | 30 +++++++++++++++++++++---------
 src/InternPool.zig    | 20 ++++++++++++++++++++
 src/Zcu/PerThread.zig | 43 ++++++++++++++++++++++++-------------------
 src/main.zig          | 19 ++++++++++++++++---
 4 files changed, 81 insertions(+), 31 deletions(-)

diff --git a/src/Compilation.zig b/src/Compilation.zig
index a7d3b6b437..659e24c3f1 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -3223,17 +3223,29 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle {
         }
     }
 
-    if (comp.zcu) |zcu| {
-        if (comp.incremental and bundle.root_list.items.len == 0) {
-            const should_have_error = for (zcu.transitive_failed_analysis.keys()) |failed_unit| {
-                const refs = try zcu.resolveReferences();
-                if (refs.contains(failed_unit)) break true;
-            } else false;
-            if (should_have_error) {
-                @panic("referenced transitive analysis errors, but none actually emitted");
+    // TODO: eventually, this should be behind `std.debug.runtime_safety`. But right now, this is a
+    // very common way for incremental compilation bugs to manifest, so let's always check it.
+    if (comp.zcu) |zcu| if (comp.incremental and bundle.root_list.items.len == 0) {
+        for (zcu.transitive_failed_analysis.keys()) |failed_unit| {
+            const refs = try zcu.resolveReferences();
+            var ref = refs.get(failed_unit) orelse continue;
+            // This AU is referenced and has a transitive compile error, meaning it referenced something with a compile error.
+            // However, we haven't reported any such error.
+            // This is a compiler bug.
+            const stderr = std.io.getStdErr().writer();
+            try stderr.writeAll("referenced transitive analysis errors, but none actually emitted\n");
+            try stderr.print("{} [transitive failure]\n", .{zcu.fmtAnalUnit(failed_unit)});
+            while (ref) |r| {
+                try stderr.print("referenced by: {}{s}\n", .{
+                    zcu.fmtAnalUnit(r.referencer),
+                    if (zcu.transitive_failed_analysis.contains(r.referencer)) " [transitive failure]" else "",
+                });
+                ref = refs.get(r.referencer).?;
             }
+
+            @panic("referenced transitive analysis errors, but none actually emitted");
         }
-    }
+    };
 
     const compile_log_text = if (comp.zcu) |m| m.compile_log_text.items else "";
     return bundle.toOwnedBundle(compile_log_text);
diff --git a/src/InternPool.zig b/src/InternPool.zig
index 63cdd7cec8..c6f9278109 100644
--- a/src/InternPool.zig
+++ b/src/InternPool.zig
@@ -8614,6 +8614,16 @@ pub fn getFuncDecl(
     defer gop.deinit();
     if (gop == .existing) {
         extra.mutate.len = prev_extra_len;
+
+        const zir_body_inst_ptr = ip.funcDeclInfo(gop.existing).zirBodyInstPtr(ip);
+        if (zir_body_inst_ptr.* != key.zir_body_inst) {
+            // Since this function's `owner_nav` matches `key`, this *is* the function we're talking
+            // about. The only way it could have a different ZIR `func` instruction is if the old
+            // instruction has been lost and replaced with a new `TrackedInst.Index`.
+            assert(zir_body_inst_ptr.resolve(ip) == null);
+            zir_body_inst_ptr.* = key.zir_body_inst;
+        }
+
         return gop.existing;
     }
 
@@ -8760,6 +8770,16 @@ pub fn getFuncDeclIes(
         // An existing function type was found; undo the additions to our two arrays.
         items.mutate.len -= 4;
         extra.mutate.len = prev_extra_len;
+
+        const zir_body_inst_ptr = ip.funcDeclInfo(func_gop.existing).zirBodyInstPtr(ip);
+        if (zir_body_inst_ptr.* != key.zir_body_inst) {
+            // Since this function's `owner_nav` matches `key`, this *is* the function we're talking
+            // about. The only way it could have a different ZIR `func` instruction is if the old
+            // instruction has been lost and replaced with a new `TrackedInst.Index`.
+            assert(zir_body_inst_ptr.resolve(ip) == null);
+            zir_body_inst_ptr.* = key.zir_body_inst;
+        }
+
         return func_gop.existing;
     }
     func_gop.putTentative(func_index);
diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig
index b12790badb..18494a6836 100644
--- a/src/Zcu/PerThread.zig
+++ b/src/Zcu/PerThread.zig
@@ -538,7 +538,7 @@ pub fn ensureCauAnalyzed(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) Zcu
     const anal_unit = AnalUnit.wrap(.{ .cau = cau_index });
     const cau = ip.getCau(cau_index);
 
-    log.debug("ensureCauAnalyzed {d}", .{@intFromEnum(cau_index)});
+    log.debug("ensureCauAnalyzed {}", .{zcu.fmtAnalUnit(anal_unit)});
 
     assert(!zcu.analysis_in_progress.contains(anal_unit));
 
@@ -576,13 +576,19 @@ pub fn ensureCauAnalyzed(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) Zcu
     }
 
     const sema_result: SemaCauResult, const analysis_fail = if (pt.ensureCauAnalyzedInner(cau_index, cau_outdated)) |result|
-        .{ result, false }
+        // This `Cau` has gone from failed to success, so even if the value of the owner `Nav` didn't actually
+        // change, we need to invalidate the dependencies anyway.
+        .{ .{
+            .invalidate_decl_val = result.invalidate_decl_val or prev_failed,
+            .invalidate_decl_ref = result.invalidate_decl_ref or prev_failed,
+        }, false }
     else |err| switch (err) {
         error.AnalysisFail => res: {
             if (!zcu.failed_analysis.contains(anal_unit)) {
                 // If this `Cau` caused the error, it would have an entry in `failed_analysis`.
                 // Since it does not, this must be a transitive failure.
                 try zcu.transitive_failed_analysis.put(gpa, anal_unit, {});
+                log.debug("mark transitive analysis failure for {}", .{zcu.fmtAnalUnit(anal_unit)});
             }
             // We consider this `Cau` to be outdated if:
             // * Previous analysis succeeded; in this case, we need to re-analyze dependants to ensure
@@ -707,12 +713,12 @@ pub fn ensureFuncBodyAnalyzed(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
 
     // We only care about the uncoerced function.
     const func_index = ip.unwrapCoercedFunc(maybe_coerced_func_index);
+    const anal_unit = AnalUnit.wrap(.{ .func = func_index });
+
+    log.debug("ensureFuncBodyAnalyzed {}", .{zcu.fmtAnalUnit(anal_unit)});
 
     const func = zcu.funcInfo(maybe_coerced_func_index);
 
-    log.debug("ensureFuncBodyAnalyzed {d}", .{@intFromEnum(func_index)});
-
-    const anal_unit = AnalUnit.wrap(.{ .func = func_index });
     const func_outdated = zcu.outdated.swapRemove(anal_unit) or
         zcu.potentially_outdated.swapRemove(anal_unit);
 
@@ -740,6 +746,7 @@ pub fn ensureFuncBodyAnalyzed(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
                 // If this function caused the error, it would have an entry in `failed_analysis`.
                 // Since it does not, this must be a transitive failure.
                 try zcu.transitive_failed_analysis.put(gpa, anal_unit, {});
+                log.debug("mark transitive analysis failure for {}", .{zcu.fmtAnalUnit(anal_unit)});
             }
             // We consider the IES to be outdated if the function previously succeeded analysis; in this case,
             // we need to re-analyze dependants to ensure they hit a transitive error here, rather than reporting
@@ -751,10 +758,8 @@ pub fn ensureFuncBodyAnalyzed(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
 
     if (func_outdated) {
         if (ies_outdated) {
-            log.debug("func IES invalidated ('{d}')", .{@intFromEnum(func_index)});
             try zcu.markDependeeOutdated(.marked_po, .{ .interned = func_index });
         } else {
-            log.debug("func IES up-to-date ('{d}')", .{@intFromEnum(func_index)});
             try zcu.markPoDependeeUpToDate(.{ .interned = func_index });
         }
     }
@@ -779,6 +784,7 @@ fn ensureFuncBodyAnalyzedInner(
     // results in the worst case.
 
     if (func.generic_owner == .none) {
+        // Among another things, this ensures that the function's `zir_body_inst` is correct.
         try pt.ensureCauAnalyzed(ip.getNav(func.owner_nav).analysis_owner.unwrap().?);
         if (ip.getNav(func.owner_nav).status.resolved.val != func_index) {
             // This function is no longer referenced! There's no point in re-analyzing it.
@@ -787,6 +793,7 @@ fn ensureFuncBodyAnalyzedInner(
         }
     } else {
         const go_nav = zcu.funcInfo(func.generic_owner).owner_nav;
+        // Among another things, this ensures that the function's `zir_body_inst` is correct.
         try pt.ensureCauAnalyzed(ip.getNav(go_nav).analysis_owner.unwrap().?);
         if (ip.getNav(go_nav).status.resolved.val != func.generic_owner) {
             // The generic owner is no longer referenced, so this function is also unreferenced.
@@ -824,8 +831,8 @@ fn ensureFuncBodyAnalyzedInner(
         }
     }
 
-    log.debug("analyze and generate fn body '{d}'; reason='{s}'", .{
-        @intFromEnum(func_index),
+    log.debug("analyze and generate fn body {}; reason='{s}'", .{
+        zcu.fmtAnalUnit(anal_unit),
         if (func_outdated) "outdated" else "never analyzed",
     });
 
@@ -1164,7 +1171,7 @@ fn semaCau(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) !SemaCauResult {
         .none, .type => false,
     };
 
-    log.debug("semaCau '{d}'", .{@intFromEnum(cau_index)});
+    log.debug("semaCau {}", .{zcu.fmtAnalUnit(anal_unit)});
 
     try zcu.analysis_in_progress.put(gpa, anal_unit, {});
     errdefer _ = zcu.analysis_in_progress.swapRemove(anal_unit);
@@ -2307,16 +2314,14 @@ pub fn getErrorValueFromSlice(pt: Zcu.PerThread, name: []const u8) Allocator.Err
     return pt.getErrorValue(try pt.zcu.intern_pool.getOrPutString(pt.zcu.gpa, name));
 }
 
+/// Removes any entry from `Zcu.failed_files` associated with `file`. Acquires `Compilation.mutex` as needed.
+/// `file.zir` must be unchanged from the last update, as it is used to determine if there is such an entry.
 fn lockAndClearFileCompileError(pt: Zcu.PerThread, file: *Zcu.File) void {
-    switch (file.status) {
-        .success_zir, .retryable_failure => {},
-        .never_loaded, .parse_failure, .astgen_failure => {
-            pt.zcu.comp.mutex.lock();
-            defer pt.zcu.comp.mutex.unlock();
-            if (pt.zcu.failed_files.fetchSwapRemove(file)) |kv| {
-                if (kv.value) |msg| msg.destroy(pt.zcu.gpa); // Delete previous error message.
-            }
-        },
+    if (!file.zir_loaded or !file.zir.hasCompileErrors()) return;
+    pt.zcu.comp.mutex.lock();
+    defer pt.zcu.comp.mutex.unlock();
+    if (pt.zcu.failed_files.fetchSwapRemove(file)) |kv| {
+        if (kv.value) |msg| msg.destroy(pt.zcu.gpa); // Delete previous error message.
     }
 }
 
diff --git a/src/main.zig b/src/main.zig
index a6bec502be..628687937c 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -6096,11 +6096,18 @@ fn cmdAstCheck(
         var error_bundle = try wip_errors.toOwnedBundle("");
         defer error_bundle.deinit(gpa);
         error_bundle.renderToStdErr(color.renderOptions());
-        process.exit(1);
+
+        if (file.zir.loweringFailed()) {
+            process.exit(1);
+        }
     }
 
     if (!want_output_text) {
-        return cleanExit();
+        if (file.zir.hasCompileErrors()) {
+            process.exit(1);
+        } else {
+            return cleanExit();
+        }
     }
     if (!build_options.enable_debug_extensions) {
         fatal("-t option only available in builds of zig with debug extensions", .{});
@@ -6144,7 +6151,13 @@ fn cmdAstCheck(
         // zig fmt: on
     }
 
-    return @import("print_zir.zig").renderAsTextToFile(gpa, &file, io.getStdOut());
+    try @import("print_zir.zig").renderAsTextToFile(gpa, &file, io.getStdOut());
+
+    if (file.zir.hasCompileErrors()) {
+        process.exit(1);
+    } else {
+        return cleanExit();
+    }
 }
 
 fn cmdDetectCpu(

From 8f849684f46ad0835bd9591f420e49e212880cb2 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 5 Dec 2024 18:03:09 +0000
Subject: [PATCH 3/4] std.zig.Zir: improve instruction tracking

The main change here is to partition tracked instructions found within a
declaration. It's very unlikely that, for instance, a `struct { ... }`
type declaration was intentionally turned into a reification or an
anonymous initialization, so it makes sense to track things in a few
different arrays.

In particular, this fixes an issue where a `func` instruction could
wrongly be mapped to something else if the types of function parameters
changed. This would cause huge problems further down the pipeline; we
expect that if a `declaration` is tracked, and it previously contained a
`func`/`func_inferred`/`func_fancy`, then this instruction is either
tracked to another `func`/`func_inferred`/`func_fancy` instruction, or
is lost.

Also, this commit takes the opportunity to rename the functions actually
doing this logic. `Zir.findDecls` was a name that might have made sense
at some point, but nowadays, it's definitely not finding declarations,
and it's not *exclusively* finding type declarations. Instead, the point
is to find instructions which we want to track; hence the new name,
`Zir.findTrackable`.

Lastly, a nice side effect of partitioning the output of `findTrackable`
is that `Zir.declIterator` no longer needs to accept input instructions
which aren't type declarations (e.g. `reify`, `func`).
---
 lib/std/zig/Zir.zig | 372 +++++++++++++++++++++++---------------------
 src/Zcu.zig         |  82 +++++++---
 2 files changed, 253 insertions(+), 201 deletions(-)

diff --git a/lib/std/zig/Zir.zig b/lib/std/zig/Zir.zig
index 2844bda55a..fca6d74e21 100644
--- a/lib/std/zig/Zir.zig
+++ b/lib/std/zig/Zir.zig
@@ -3615,145 +3615,155 @@ pub const DeclIterator = struct {
 };
 
 pub fn declIterator(zir: Zir, decl_inst: Zir.Inst.Index) DeclIterator {
-    const tags = zir.instructions.items(.tag);
-    const datas = zir.instructions.items(.data);
-    switch (tags[@intFromEnum(decl_inst)]) {
-        // Functions are allowed and yield no iterations.
-        // This is because they are returned by `findDecls`.
-        .func, .func_inferred, .func_fancy => return .{
-            .extra_index = undefined,
-            .decls_remaining = 0,
-            .zir = zir,
-        },
+    const inst = zir.instructions.get(@intFromEnum(decl_inst));
+    assert(inst.tag == .extended);
+    const extended = inst.data.extended;
+    switch (extended.opcode) {
+        .struct_decl => {
+            const small: Inst.StructDecl.Small = @bitCast(extended.small);
+            var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.StructDecl).@"struct".fields.len);
+            const captures_len = if (small.has_captures_len) captures_len: {
+                const captures_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :captures_len captures_len;
+            } else 0;
+            extra_index += @intFromBool(small.has_fields_len);
+            const decls_len = if (small.has_decls_len) decls_len: {
+                const decls_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :decls_len decls_len;
+            } else 0;
 
-        .extended => {
-            const extended = datas[@intFromEnum(decl_inst)].extended;
-            switch (extended.opcode) {
-                // Reifications are allowed and yield no iterations.
-                // This is because they are returned by `findDecls`.
-                .reify => return .{
-                    .extra_index = undefined,
-                    .decls_remaining = 0,
-                    .zir = zir,
-                },
-                .struct_decl => {
-                    const small: Inst.StructDecl.Small = @bitCast(extended.small);
-                    var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.StructDecl).@"struct".fields.len);
-                    const captures_len = if (small.has_captures_len) captures_len: {
-                        const captures_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :captures_len captures_len;
-                    } else 0;
-                    extra_index += @intFromBool(small.has_fields_len);
-                    const decls_len = if (small.has_decls_len) decls_len: {
-                        const decls_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :decls_len decls_len;
-                    } else 0;
+            extra_index += captures_len;
 
-                    extra_index += captures_len;
-
-                    if (small.has_backing_int) {
-                        const backing_int_body_len = zir.extra[extra_index];
-                        extra_index += 1; // backing_int_body_len
-                        if (backing_int_body_len == 0) {
-                            extra_index += 1; // backing_int_ref
-                        } else {
-                            extra_index += backing_int_body_len; // backing_int_body_inst
-                        }
-                    }
-
-                    return .{
-                        .extra_index = extra_index,
-                        .decls_remaining = decls_len,
-                        .zir = zir,
-                    };
-                },
-                .enum_decl => {
-                    const small: Inst.EnumDecl.Small = @bitCast(extended.small);
-                    var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.EnumDecl).@"struct".fields.len);
-                    extra_index += @intFromBool(small.has_tag_type);
-                    const captures_len = if (small.has_captures_len) captures_len: {
-                        const captures_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :captures_len captures_len;
-                    } else 0;
-                    extra_index += @intFromBool(small.has_body_len);
-                    extra_index += @intFromBool(small.has_fields_len);
-                    const decls_len = if (small.has_decls_len) decls_len: {
-                        const decls_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :decls_len decls_len;
-                    } else 0;
-
-                    extra_index += captures_len;
-
-                    return .{
-                        .extra_index = extra_index,
-                        .decls_remaining = decls_len,
-                        .zir = zir,
-                    };
-                },
-                .union_decl => {
-                    const small: Inst.UnionDecl.Small = @bitCast(extended.small);
-                    var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.UnionDecl).@"struct".fields.len);
-                    extra_index += @intFromBool(small.has_tag_type);
-                    const captures_len = if (small.has_captures_len) captures_len: {
-                        const captures_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :captures_len captures_len;
-                    } else 0;
-                    extra_index += @intFromBool(small.has_body_len);
-                    extra_index += @intFromBool(small.has_fields_len);
-                    const decls_len = if (small.has_decls_len) decls_len: {
-                        const decls_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :decls_len decls_len;
-                    } else 0;
-
-                    extra_index += captures_len;
-
-                    return .{
-                        .extra_index = extra_index,
-                        .decls_remaining = decls_len,
-                        .zir = zir,
-                    };
-                },
-                .opaque_decl => {
-                    const small: Inst.OpaqueDecl.Small = @bitCast(extended.small);
-                    var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.OpaqueDecl).@"struct".fields.len);
-                    const decls_len = if (small.has_decls_len) decls_len: {
-                        const decls_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :decls_len decls_len;
-                    } else 0;
-                    const captures_len = if (small.has_captures_len) captures_len: {
-                        const captures_len = zir.extra[extra_index];
-                        extra_index += 1;
-                        break :captures_len captures_len;
-                    } else 0;
-
-                    extra_index += captures_len;
-
-                    return .{
-                        .extra_index = extra_index,
-                        .decls_remaining = decls_len,
-                        .zir = zir,
-                    };
-                },
-                else => unreachable,
+            if (small.has_backing_int) {
+                const backing_int_body_len = zir.extra[extra_index];
+                extra_index += 1; // backing_int_body_len
+                if (backing_int_body_len == 0) {
+                    extra_index += 1; // backing_int_ref
+                } else {
+                    extra_index += backing_int_body_len; // backing_int_body_inst
+                }
             }
+
+            return .{
+                .extra_index = extra_index,
+                .decls_remaining = decls_len,
+                .zir = zir,
+            };
+        },
+        .enum_decl => {
+            const small: Inst.EnumDecl.Small = @bitCast(extended.small);
+            var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.EnumDecl).@"struct".fields.len);
+            extra_index += @intFromBool(small.has_tag_type);
+            const captures_len = if (small.has_captures_len) captures_len: {
+                const captures_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :captures_len captures_len;
+            } else 0;
+            extra_index += @intFromBool(small.has_body_len);
+            extra_index += @intFromBool(small.has_fields_len);
+            const decls_len = if (small.has_decls_len) decls_len: {
+                const decls_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :decls_len decls_len;
+            } else 0;
+
+            extra_index += captures_len;
+
+            return .{
+                .extra_index = extra_index,
+                .decls_remaining = decls_len,
+                .zir = zir,
+            };
+        },
+        .union_decl => {
+            const small: Inst.UnionDecl.Small = @bitCast(extended.small);
+            var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.UnionDecl).@"struct".fields.len);
+            extra_index += @intFromBool(small.has_tag_type);
+            const captures_len = if (small.has_captures_len) captures_len: {
+                const captures_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :captures_len captures_len;
+            } else 0;
+            extra_index += @intFromBool(small.has_body_len);
+            extra_index += @intFromBool(small.has_fields_len);
+            const decls_len = if (small.has_decls_len) decls_len: {
+                const decls_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :decls_len decls_len;
+            } else 0;
+
+            extra_index += captures_len;
+
+            return .{
+                .extra_index = extra_index,
+                .decls_remaining = decls_len,
+                .zir = zir,
+            };
+        },
+        .opaque_decl => {
+            const small: Inst.OpaqueDecl.Small = @bitCast(extended.small);
+            var extra_index: u32 = @intCast(extended.operand + @typeInfo(Inst.OpaqueDecl).@"struct".fields.len);
+            const decls_len = if (small.has_decls_len) decls_len: {
+                const decls_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :decls_len decls_len;
+            } else 0;
+            const captures_len = if (small.has_captures_len) captures_len: {
+                const captures_len = zir.extra[extra_index];
+                extra_index += 1;
+                break :captures_len captures_len;
+            } else 0;
+
+            extra_index += captures_len;
+
+            return .{
+                .extra_index = extra_index,
+                .decls_remaining = decls_len,
+                .zir = zir,
+            };
         },
         else => unreachable,
     }
 }
 
-/// Find all type declarations, recursively, within a `declaration` instruction. Does not recurse through
-/// said type declarations' declarations; to find all declarations, call this function on the declarations
-/// of the discovered types recursively.
-/// The iterator would have to allocate memory anyway to iterate, so an `ArrayList` is populated as the result.
-pub fn findDecls(zir: Zir, gpa: Allocator, list: *std.ArrayListUnmanaged(Inst.Index), decl_inst: Zir.Inst.Index) !void {
-    list.clearRetainingCapacity();
+/// `DeclContents` contains all "interesting" instructions found within a declaration by `findTrackable`.
+/// These instructions are partitioned into a few different sets, since this makes ZIR instruction mapping
+/// more effective.
+pub const DeclContents = struct {
+    /// This is a simple optional because ZIR guarantees that a `func`/`func_inferred`/`func_fancy` instruction
+    /// can only occur once per `declaration`.
+    func_decl: ?Inst.Index,
+    explicit_types: std.ArrayListUnmanaged(Inst.Index),
+    other: std.ArrayListUnmanaged(Inst.Index),
+
+    pub const init: DeclContents = .{
+        .func_decl = null,
+        .explicit_types = .empty,
+        .other = .empty,
+    };
+
+    pub fn clear(contents: *DeclContents) void {
+        contents.func_decl = null;
+        contents.explicit_types.clearRetainingCapacity();
+        contents.other.clearRetainingCapacity();
+    }
+
+    pub fn deinit(contents: *DeclContents, gpa: Allocator) void {
+        contents.explicit_types.deinit(gpa);
+        contents.other.deinit(gpa);
+    }
+};
+
+/// Find all tracked ZIR instructions, recursively, within a `declaration` instruction. Does not recurse through
+/// nested declarations; to find all declarations, call this function recursively on the type declarations discovered
+/// in `contents.explicit_types`.
+///
+/// This populates an `ArrayListUnmanaged` because an iterator would need to allocate memory anyway.
+pub fn findTrackable(zir: Zir, gpa: Allocator, contents: *DeclContents, decl_inst: Zir.Inst.Index) !void {
+    contents.clear();
+
     const declaration, const extra_end = zir.getDeclaration(decl_inst);
     const bodies = declaration.getBodies(extra_end, zir);
 
@@ -3762,27 +3772,27 @@ pub fn findDecls(zir: Zir, gpa: Allocator, list: *std.ArrayListUnmanaged(Inst.In
     var found_defers: std.AutoHashMapUnmanaged(u32, void) = .empty;
     defer found_defers.deinit(gpa);
 
-    try zir.findDeclsBody(gpa, list, &found_defers, bodies.value_body);
-    if (bodies.align_body) |b| try zir.findDeclsBody(gpa, list, &found_defers, b);
-    if (bodies.linksection_body) |b| try zir.findDeclsBody(gpa, list, &found_defers, b);
-    if (bodies.addrspace_body) |b| try zir.findDeclsBody(gpa, list, &found_defers, b);
+    try zir.findTrackableBody(gpa, contents, &found_defers, bodies.value_body);
+    if (bodies.align_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b);
+    if (bodies.linksection_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b);
+    if (bodies.addrspace_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b);
 }
 
-/// Like `findDecls`, but only considers the `main_struct_inst` instruction. This may return more than
+/// Like `findTrackable`, but only considers the `main_struct_inst` instruction. This may return more than
 /// just that instruction because it will also traverse fields.
-pub fn findDeclsRoot(zir: Zir, gpa: Allocator, list: *std.ArrayListUnmanaged(Inst.Index)) !void {
-    list.clearRetainingCapacity();
+pub fn findTrackableRoot(zir: Zir, gpa: Allocator, contents: *DeclContents) !void {
+    contents.clear();
 
     var found_defers: std.AutoHashMapUnmanaged(u32, void) = .empty;
     defer found_defers.deinit(gpa);
 
-    try zir.findDeclsInner(gpa, list, &found_defers, .main_struct_inst);
+    try zir.findTrackableInner(gpa, contents, &found_defers, .main_struct_inst);
 }
 
-fn findDeclsInner(
+fn findTrackableInner(
     zir: Zir,
     gpa: Allocator,
-    list: *std.ArrayListUnmanaged(Inst.Index),
+    contents: *DeclContents,
     defers: *std.AutoHashMapUnmanaged(u32, void),
     inst: Inst.Index,
 ) Allocator.Error!void {
@@ -4026,7 +4036,7 @@ fn findDeclsInner(
         .struct_init,
         .struct_init_ref,
         .struct_init_anon,
-        => return list.append(gpa, inst),
+        => return contents.other.append(gpa, inst),
 
         .extended => {
             const extended = datas[@intFromEnum(inst)].extended;
@@ -4093,15 +4103,15 @@ fn findDeclsInner(
                 .typeof_peer => {
                     const extra = zir.extraData(Zir.Inst.TypeOfPeer, extended.operand);
                     const body = zir.bodySlice(extra.data.body_index, extra.data.body_len);
-                    try zir.findDeclsBody(gpa, list, defers, body);
+                    try zir.findTrackableBody(gpa, contents, defers, body);
                 },
 
                 // Reifications and opaque declarations need tracking, but have no body.
-                .reify, .opaque_decl => return list.append(gpa, inst),
+                .reify, .opaque_decl => return contents.other.append(gpa, inst),
 
                 // Struct declarations need tracking and have bodies.
                 .struct_decl => {
-                    try list.append(gpa, inst);
+                    try contents.explicit_types.append(gpa, inst);
 
                     const small: Zir.Inst.StructDecl.Small = @bitCast(extended.small);
                     const extra = zir.extraData(Zir.Inst.StructDecl, extended.operand);
@@ -4130,7 +4140,7 @@ fn findDeclsInner(
                         } else {
                             const body = zir.bodySlice(extra_index, backing_int_body_len);
                             extra_index += backing_int_body_len;
-                            try zir.findDeclsBody(gpa, list, defers, body);
+                            try zir.findTrackableBody(gpa, contents, defers, body);
                         }
                     }
                     extra_index += decls_len;
@@ -4186,12 +4196,12 @@ fn findDeclsInner(
 
                     // Now, `fields_extra_index` points to `bodies`. Let's treat this as one big body.
                     const merged_bodies = zir.bodySlice(fields_extra_index, total_bodies_len);
-                    try zir.findDeclsBody(gpa, list, defers, merged_bodies);
+                    try zir.findTrackableBody(gpa, contents, defers, merged_bodies);
                 },
 
                 // Union declarations need tracking and have a body.
                 .union_decl => {
-                    try list.append(gpa, inst);
+                    try contents.explicit_types.append(gpa, inst);
 
                     const small: Zir.Inst.UnionDecl.Small = @bitCast(extended.small);
                     const extra = zir.extraData(Zir.Inst.UnionDecl, extended.operand);
@@ -4216,12 +4226,12 @@ fn findDeclsInner(
                     extra_index += captures_len;
                     extra_index += decls_len;
                     const body = zir.bodySlice(extra_index, body_len);
-                    try zir.findDeclsBody(gpa, list, defers, body);
+                    try zir.findTrackableBody(gpa, contents, defers, body);
                 },
 
                 // Enum declarations need tracking and have a body.
                 .enum_decl => {
-                    try list.append(gpa, inst);
+                    try contents.explicit_types.append(gpa, inst);
 
                     const small: Zir.Inst.EnumDecl.Small = @bitCast(extended.small);
                     const extra = zir.extraData(Zir.Inst.EnumDecl, extended.operand);
@@ -4246,7 +4256,7 @@ fn findDeclsInner(
                     extra_index += captures_len;
                     extra_index += decls_len;
                     const body = zir.bodySlice(extra_index, body_len);
-                    try zir.findDeclsBody(gpa, list, defers, body);
+                    try zir.findTrackableBody(gpa, contents, defers, body);
                 },
             }
         },
@@ -4255,7 +4265,8 @@ fn findDeclsInner(
         .func,
         .func_inferred,
         => {
-            try list.append(gpa, inst);
+            assert(contents.func_decl == null);
+            contents.func_decl = inst;
 
             const inst_data = datas[@intFromEnum(inst)].pl_node;
             const extra = zir.extraData(Inst.Func, inst_data.payload_index);
@@ -4266,14 +4277,15 @@ fn findDeclsInner(
                 else => {
                     const body = zir.bodySlice(extra_index, extra.data.ret_body_len);
                     extra_index += body.len;
-                    try zir.findDeclsBody(gpa, list, defers, body);
+                    try zir.findTrackableBody(gpa, contents, defers, body);
                 },
             }
             const body = zir.bodySlice(extra_index, extra.data.body_len);
-            return zir.findDeclsBody(gpa, list, defers, body);
+            return zir.findTrackableBody(gpa, contents, defers, body);
         },
         .func_fancy => {
-            try list.append(gpa, inst);
+            assert(contents.func_decl == null);
+            contents.func_decl = inst;
 
             const inst_data = datas[@intFromEnum(inst)].pl_node;
             const extra = zir.extraData(Inst.FuncFancy, inst_data.payload_index);
@@ -4284,7 +4296,7 @@ fn findDeclsInner(
                 const body_len = zir.extra[extra_index];
                 extra_index += 1;
                 const body = zir.bodySlice(extra_index, body_len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
                 extra_index += body.len;
             } else if (extra.data.bits.has_align_ref) {
                 extra_index += 1;
@@ -4294,7 +4306,7 @@ fn findDeclsInner(
                 const body_len = zir.extra[extra_index];
                 extra_index += 1;
                 const body = zir.bodySlice(extra_index, body_len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
                 extra_index += body.len;
             } else if (extra.data.bits.has_addrspace_ref) {
                 extra_index += 1;
@@ -4304,7 +4316,7 @@ fn findDeclsInner(
                 const body_len = zir.extra[extra_index];
                 extra_index += 1;
                 const body = zir.bodySlice(extra_index, body_len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
                 extra_index += body.len;
             } else if (extra.data.bits.has_section_ref) {
                 extra_index += 1;
@@ -4314,7 +4326,7 @@ fn findDeclsInner(
                 const body_len = zir.extra[extra_index];
                 extra_index += 1;
                 const body = zir.bodySlice(extra_index, body_len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
                 extra_index += body.len;
             } else if (extra.data.bits.has_cc_ref) {
                 extra_index += 1;
@@ -4324,7 +4336,7 @@ fn findDeclsInner(
                 const body_len = zir.extra[extra_index];
                 extra_index += 1;
                 const body = zir.bodySlice(extra_index, body_len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
                 extra_index += body.len;
             } else if (extra.data.bits.has_ret_ty_ref) {
                 extra_index += 1;
@@ -4333,7 +4345,7 @@ fn findDeclsInner(
             extra_index += @intFromBool(extra.data.bits.has_any_noalias);
 
             const body = zir.bodySlice(extra_index, extra.data.body_len);
-            return zir.findDeclsBody(gpa, list, defers, body);
+            return zir.findTrackableBody(gpa, contents, defers, body);
         },
 
         // Block instructions, recurse over the bodies.
@@ -4348,24 +4360,24 @@ fn findDeclsInner(
             const inst_data = datas[@intFromEnum(inst)].pl_node;
             const extra = zir.extraData(Inst.Block, inst_data.payload_index);
             const body = zir.bodySlice(extra.end, extra.data.body_len);
-            return zir.findDeclsBody(gpa, list, defers, body);
+            return zir.findTrackableBody(gpa, contents, defers, body);
         },
         .condbr, .condbr_inline => {
             const inst_data = datas[@intFromEnum(inst)].pl_node;
             const extra = zir.extraData(Inst.CondBr, inst_data.payload_index);
             const then_body = zir.bodySlice(extra.end, extra.data.then_body_len);
             const else_body = zir.bodySlice(extra.end + then_body.len, extra.data.else_body_len);
-            try zir.findDeclsBody(gpa, list, defers, then_body);
-            try zir.findDeclsBody(gpa, list, defers, else_body);
+            try zir.findTrackableBody(gpa, contents, defers, then_body);
+            try zir.findTrackableBody(gpa, contents, defers, else_body);
         },
         .@"try", .try_ptr => {
             const inst_data = datas[@intFromEnum(inst)].pl_node;
             const extra = zir.extraData(Inst.Try, inst_data.payload_index);
             const body = zir.bodySlice(extra.end, extra.data.body_len);
-            try zir.findDeclsBody(gpa, list, defers, body);
+            try zir.findTrackableBody(gpa, contents, defers, body);
         },
-        .switch_block, .switch_block_ref => return zir.findDeclsSwitch(gpa, list, defers, inst, .normal),
-        .switch_block_err_union => return zir.findDeclsSwitch(gpa, list, defers, inst, .err_union),
+        .switch_block, .switch_block_ref => return zir.findTrackableSwitch(gpa, contents, defers, inst, .normal),
+        .switch_block_err_union => return zir.findTrackableSwitch(gpa, contents, defers, inst, .err_union),
 
         .suspend_block => @panic("TODO iterate suspend block"),
 
@@ -4373,7 +4385,7 @@ fn findDeclsInner(
             const inst_data = datas[@intFromEnum(inst)].pl_tok;
             const extra = zir.extraData(Inst.Param, inst_data.payload_index);
             const body = zir.bodySlice(extra.end, extra.data.body_len);
-            try zir.findDeclsBody(gpa, list, defers, body);
+            try zir.findTrackableBody(gpa, contents, defers, body);
         },
 
         inline .call, .field_call => |tag| {
@@ -4389,7 +4401,7 @@ fn findDeclsInner(
                 const first_arg_start_off = args_len;
                 const final_arg_end_off = zir.extra[extra.end + args_len - 1];
                 const args_body = zir.bodySlice(extra.end + first_arg_start_off, final_arg_end_off - first_arg_start_off);
-                try zir.findDeclsBody(gpa, list, defers, args_body);
+                try zir.findTrackableBody(gpa, contents, defers, args_body);
             }
         },
         .@"defer" => {
@@ -4397,7 +4409,7 @@ fn findDeclsInner(
             const gop = try defers.getOrPut(gpa, inst_data.index);
             if (!gop.found_existing) {
                 const body = zir.bodySlice(inst_data.index, inst_data.len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
             }
         },
         .defer_err_code => {
@@ -4406,16 +4418,16 @@ fn findDeclsInner(
             const gop = try defers.getOrPut(gpa, extra.index);
             if (!gop.found_existing) {
                 const body = zir.bodySlice(extra.index, extra.len);
-                try zir.findDeclsBody(gpa, list, defers, body);
+                try zir.findTrackableBody(gpa, contents, defers, body);
             }
         },
     }
 }
 
-fn findDeclsSwitch(
+fn findTrackableSwitch(
     zir: Zir,
     gpa: Allocator,
-    list: *std.ArrayListUnmanaged(Inst.Index),
+    contents: *DeclContents,
     defers: *std.AutoHashMapUnmanaged(u32, void),
     inst: Inst.Index,
     /// Distinguishes between `switch_block[_ref]` and `switch_block_err_union`.
@@ -4451,7 +4463,7 @@ fn findDeclsSwitch(
             const body = zir.bodySlice(extra_index, prong_info.body_len);
             extra_index += body.len;
 
-            try zir.findDeclsBody(gpa, list, defers, body);
+            try zir.findTrackableBody(gpa, contents, defers, body);
 
             break :has_special extra.data.bits.has_else;
         },
@@ -4463,7 +4475,7 @@ fn findDeclsSwitch(
         const body = zir.bodySlice(extra_index, prong_info.body_len);
         extra_index += body.len;
 
-        try zir.findDeclsBody(gpa, list, defers, body);
+        try zir.findTrackableBody(gpa, contents, defers, body);
     }
 
     {
@@ -4475,7 +4487,7 @@ fn findDeclsSwitch(
             const body = zir.bodySlice(extra_index, prong_info.body_len);
             extra_index += body.len;
 
-            try zir.findDeclsBody(gpa, list, defers, body);
+            try zir.findTrackableBody(gpa, contents, defers, body);
         }
     }
     {
@@ -4492,20 +4504,20 @@ fn findDeclsSwitch(
             const body = zir.bodySlice(extra_index, prong_info.body_len);
             extra_index += body.len;
 
-            try zir.findDeclsBody(gpa, list, defers, body);
+            try zir.findTrackableBody(gpa, contents, defers, body);
         }
     }
 }
 
-fn findDeclsBody(
+fn findTrackableBody(
     zir: Zir,
     gpa: Allocator,
-    list: *std.ArrayListUnmanaged(Inst.Index),
+    contents: *DeclContents,
     defers: *std.AutoHashMapUnmanaged(u32, void),
     body: []const Inst.Index,
 ) Allocator.Error!void {
     for (body) |member| {
-        try zir.findDeclsInner(gpa, list, defers, member);
+        try zir.findTrackableInner(gpa, contents, defers, member);
     }
 }
 
diff --git a/src/Zcu.zig b/src/Zcu.zig
index d03eb4cc9a..bb2dc2a8df 100644
--- a/src/Zcu.zig
+++ b/src/Zcu.zig
@@ -2593,26 +2593,44 @@ pub fn mapOldZirToNew(
     defer match_stack.deinit(gpa);
 
     // Used as temporary buffers for namespace declaration instructions
-    var old_decls: std.ArrayListUnmanaged(Zir.Inst.Index) = .empty;
-    defer old_decls.deinit(gpa);
-    var new_decls: std.ArrayListUnmanaged(Zir.Inst.Index) = .empty;
-    defer new_decls.deinit(gpa);
+    var old_contents: Zir.DeclContents = .init;
+    defer old_contents.deinit(gpa);
+    var new_contents: Zir.DeclContents = .init;
+    defer new_contents.deinit(gpa);
 
     // Map the main struct inst (and anything in its fields)
     {
-        try old_zir.findDeclsRoot(gpa, &old_decls);
-        try new_zir.findDeclsRoot(gpa, &new_decls);
+        try old_zir.findTrackableRoot(gpa, &old_contents);
+        try new_zir.findTrackableRoot(gpa, &new_contents);
 
-        assert(old_decls.items[0] == .main_struct_inst);
-        assert(new_decls.items[0] == .main_struct_inst);
+        assert(old_contents.explicit_types.items[0] == .main_struct_inst);
+        assert(new_contents.explicit_types.items[0] == .main_struct_inst);
 
-        // We don't have any smart way of matching up these type declarations, so we always
-        // correlate them based on source order.
-        const n = @min(old_decls.items.len, new_decls.items.len);
-        try match_stack.ensureUnusedCapacity(gpa, n);
-        for (old_decls.items[0..n], new_decls.items[0..n]) |old_inst, new_inst| {
+        assert(old_contents.func_decl == null);
+        assert(new_contents.func_decl == null);
+
+        // We don't have any smart way of matching up these instructions, so we correlate them based on source order
+        // in their respective arrays.
+
+        const num_explicit_types = @min(old_contents.explicit_types.items.len, new_contents.explicit_types.items.len);
+        try match_stack.ensureUnusedCapacity(gpa, @intCast(num_explicit_types));
+        for (
+            old_contents.explicit_types.items[0..num_explicit_types],
+            new_contents.explicit_types.items[0..num_explicit_types],
+        ) |old_inst, new_inst| {
+            // Here we use `match_stack`, so that we will recursively consider declarations on these types.
             match_stack.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst });
         }
+
+        const num_other = @min(old_contents.other.items.len, new_contents.other.items.len);
+        try inst_map.ensureUnusedCapacity(gpa, @intCast(num_other));
+        for (
+            old_contents.other.items[0..num_other],
+            new_contents.other.items[0..num_other],
+        ) |old_inst, new_inst| {
+            // These instructions don't have declarations, so we just modify `inst_map` directly.
+            inst_map.putAssumeCapacity(old_inst, new_inst);
+        }
     }
 
     while (match_stack.popOrNull()) |match_item| {
@@ -2700,17 +2718,39 @@ pub fn mapOldZirToNew(
             // Match the `declaration` instruction
             try inst_map.put(gpa, old_decl_inst, new_decl_inst);
 
-            // Find container type declarations within this declaration
-            try old_zir.findDecls(gpa, &old_decls, old_decl_inst);
-            try new_zir.findDecls(gpa, &new_decls, new_decl_inst);
+            // Find trackable instructions within this declaration
+            try old_zir.findTrackable(gpa, &old_contents, old_decl_inst);
+            try new_zir.findTrackable(gpa, &new_contents, new_decl_inst);
 
-            // We don't have any smart way of matching up these type declarations, so we always
-            // correlate them based on source order.
-            const n = @min(old_decls.items.len, new_decls.items.len);
-            try match_stack.ensureUnusedCapacity(gpa, n);
-            for (old_decls.items[0..n], new_decls.items[0..n]) |old_inst, new_inst| {
+            // We don't have any smart way of matching up these instructions, so we correlate them based on source order
+            // in their respective arrays.
+
+            const num_explicit_types = @min(old_contents.explicit_types.items.len, new_contents.explicit_types.items.len);
+            try match_stack.ensureUnusedCapacity(gpa, @intCast(num_explicit_types));
+            for (
+                old_contents.explicit_types.items[0..num_explicit_types],
+                new_contents.explicit_types.items[0..num_explicit_types],
+            ) |old_inst, new_inst| {
+                // Here we use `match_stack`, so that we will recursively consider declarations on these types.
                 match_stack.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst });
             }
+
+            const num_other = @min(old_contents.other.items.len, new_contents.other.items.len);
+            try inst_map.ensureUnusedCapacity(gpa, @intCast(num_other));
+            for (
+                old_contents.other.items[0..num_other],
+                new_contents.other.items[0..num_other],
+            ) |old_inst, new_inst| {
+                // These instructions don't have declarations, so we just modify `inst_map` directly.
+                inst_map.putAssumeCapacity(old_inst, new_inst);
+            }
+
+            if (old_contents.func_decl) |old_func_inst| {
+                if (new_contents.func_decl) |new_func_inst| {
+                    // There are no declarations on a function either, so again, we just directly add it to `inst_map`.
+                    try inst_map.put(gpa, old_func_inst, new_func_inst);
+                }
+            }
         }
     }
 }

From 9f086f84f53de4eb23d96fe611c071f27405a660 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Fri, 6 Dec 2024 06:22:32 +0000
Subject: [PATCH 4/4] Zcu: allow test declarations to be failed

The introduction of the `extended(astgen_error())` instruction allows a
`test` declaration to be unresolved, i.e. the declaration doesn't even
contain a `func`. I could modify AstGen to not do this, but it makes
more sense to just handle this case when collecting test functions.

Note that tests under incremental compilation are currently broken if
you ever remove all references to a test; this is tracked as a subtask
of #21165.
---
 src/Zcu/PerThread.zig | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig
index 18494a6836..aeda9ea1b1 100644
--- a/src/Zcu/PerThread.zig
+++ b/src/Zcu/PerThread.zig
@@ -2511,6 +2511,19 @@ pub fn populateTestFunctions(
 
         for (test_fn_vals, zcu.test_functions.keys()) |*test_fn_val, test_nav_index| {
             const test_nav = ip.getNav(test_nav_index);
+
+            {
+                // The test declaration might have failed; if that's the case, just return, as we'll
+                // be emitting a compile error anyway.
+                const cau = test_nav.analysis_owner.unwrap().?;
+                const anal_unit: AnalUnit = .wrap(.{ .cau = cau });
+                if (zcu.failed_analysis.contains(anal_unit) or
+                    zcu.transitive_failed_analysis.contains(anal_unit))
+                {
+                    return;
+                }
+            }
+
             const test_nav_name = test_nav.fqn;
             const test_nav_name_len = test_nav_name.length(ip);
             const test_name_anon_decl: InternPool.Key.Ptr.BaseAddr.Uav = n: {