compiler: more progress on incremental

lib/std/zig/AstGen.zig

@@ -13496,6 +13496,15 @@ fn scanDecls(astgen: *AstGen, namespace: *Scope.Namespace, members: []const Ast.
     const node_tags = tree.nodes.items(.tag);
     const main_tokens = tree.nodes.items(.main_token);
     const token_tags = tree.tokens.items(.tag);
+
+    // We don't have shadowing for test names, so we just track those for duplicate reporting locally.
+    var named_tests: std.AutoHashMapUnmanaged(Zir.NullTerminatedString, Ast.Node.Index) = .{};
+    var decltests: std.AutoHashMapUnmanaged(Zir.NullTerminatedString, Ast.Node.Index) = .{};
+    defer {
+        named_tests.deinit(gpa);
+        decltests.deinit(gpa);
+    }
+
     var decl_count: u32 = 0;
     for (members) |member_node| {
         const name_token = switch (node_tags[member_node]) {
@@ -13525,11 +13534,50 @@ fn scanDecls(astgen: *AstGen, namespace: *Scope.Namespace, members: []const Ast.
                 break :blk ident;
             },
 
-            .@"comptime", .@"usingnamespace", .test_decl => {
+            .@"comptime", .@"usingnamespace" => {
                 decl_count += 1;
                 continue;
             },
 
+            .test_decl => {
+                decl_count += 1;
+                // We don't want shadowing detection here, and test names work a bit differently, so
+                // we must do the redeclaration detection ourselves.
+                const test_name_token = main_tokens[member_node] + 1;
+                switch (token_tags[test_name_token]) {
+                    else => {}, // unnamed test
+                    .string_literal => {
+                        const name = try astgen.strLitAsString(test_name_token);
+                        const gop = try named_tests.getOrPut(gpa, name.index);
+                        if (gop.found_existing) {
+                            const name_slice = astgen.string_bytes.items[@intFromEnum(name.index)..][0..name.len];
+                            const name_duped = try gpa.dupe(u8, name_slice);
+                            defer gpa.free(name_duped);
+                            try astgen.appendErrorNodeNotes(member_node, "duplicate test name '{s}'", .{name_duped}, &.{
+                                try astgen.errNoteNode(gop.value_ptr.*, "other test here", .{}),
+                            });
+                        } else {
+                            gop.value_ptr.* = member_node;
+                        }
+                    },
+                    .identifier => {
+                        const name = try astgen.identAsString(test_name_token);
+                        const gop = try decltests.getOrPut(gpa, name);
+                        if (gop.found_existing) {
+                            const name_slice = mem.span(astgen.nullTerminatedString(name));
+                            const name_duped = try gpa.dupe(u8, name_slice);
+                            defer gpa.free(name_duped);
+                            try astgen.appendErrorNodeNotes(member_node, "duplicate decltest '{s}'", .{name_duped}, &.{
+                                try astgen.errNoteNode(gop.value_ptr.*, "other decltest here", .{}),
+                            });
+                        } else {
+                            gop.value_ptr.* = member_node;
+                        }
+                    },
+                }
+                continue;
+            },
+
             else => continue,
         };
 

src/Module.zig

@@ -4083,26 +4083,69 @@ pub fn scanNamespace(
     const gpa = zcu.gpa;
     const namespace = zcu.namespacePtr(namespace_index);
 
+    var seen_decls: std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, void) = .{};
+    defer seen_decls.deinit(gpa);
+
     try zcu.comp.work_queue.ensureUnusedCapacity(decls.len);
     try namespace.decls.ensureTotalCapacity(gpa, decls.len);
 
     var scan_decl_iter: ScanDeclIter = .{
         .zcu = zcu,
         .namespace_index = namespace_index,
         .parent_decl = parent_decl,
+        .seen_decls = &seen_decls,
+        .pass = .named,
     };
     for (decls) |decl_inst| {
         try scanDecl(&scan_decl_iter, decl_inst);
     }
+    scan_decl_iter.pass = .unnamed;
+    for (decls) |decl_inst| {
+        try scanDecl(&scan_decl_iter, decl_inst);
+    }
+
+    if (seen_decls.count() != namespace.decls.count()) {
+        // Do a pass over the namespace contents and remove any decls from the last update
+        // which were removed in this one.
+        var i: usize = 0;
+        while (i < namespace.decls.count()) {
+            const decl_index = namespace.decls.keys()[i];
+            const decl = zcu.declPtr(decl_index);
+            if (!seen_decls.contains(decl.name)) {
+                // We must preserve namespace ordering for @typeInfo.
+                namespace.decls.orderedRemoveAt(i);
+                i -= 1;
+            }
+        }
+    }
 }
 
 const ScanDeclIter = struct {
     zcu: *Zcu,
     namespace_index: Namespace.Index,
     parent_decl: *Decl,
+    seen_decls: *std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, void),
+    /// Decl scanning is run in two passes, so that we can detect when a generated
+    /// name would clash with an explicit name and use a different one.
+    pass: enum { named, unnamed },
     usingnamespace_index: usize = 0,
     comptime_index: usize = 0,
     unnamed_test_index: usize = 0,
+
+    fn avoidNameConflict(iter: *ScanDeclIter, comptime fmt: []const u8, args: anytype) !InternPool.NullTerminatedString {
+        const zcu = iter.zcu;
+        const gpa = zcu.gpa;
+        const ip = &zcu.intern_pool;
+        var name = try ip.getOrPutStringFmt(gpa, fmt, args);
+        var gop = try iter.seen_decls.getOrPut(gpa, name);
+        var next_suffix: u32 = 0;
+        while (gop.found_existing) {
+            name = try ip.getOrPutStringFmt(gpa, fmt ++ "_{d}", args ++ .{next_suffix});
+            gop = try iter.seen_decls.getOrPut(gpa, name);
+            next_suffix += 1;
+        }
+        return name;
+    }
 };
 
 fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void {
@@ -4126,49 +4169,63 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void
     // Every Decl needs a name.
     const decl_name: InternPool.NullTerminatedString, const kind: Decl.Kind, const is_named_test: bool = switch (declaration.name) {
         .@"comptime" => info: {
+            if (iter.pass != .unnamed) return;
             const i = iter.comptime_index;
             iter.comptime_index += 1;
             break :info .{
-                try ip.getOrPutStringFmt(gpa, "comptime_{d}", .{i}),
+                try iter.avoidNameConflict("comptime_{d}", .{i}),
                 .@"comptime",
                 false,
             };
         },
         .@"usingnamespace" => info: {
+            if (iter.pass != .unnamed) return;
             const i = iter.usingnamespace_index;
             iter.usingnamespace_index += 1;
             break :info .{
-                try ip.getOrPutStringFmt(gpa, "usingnamespace_{d}", .{i}),
+                try iter.avoidNameConflict("usingnamespace_{d}", .{i}),
                 .@"usingnamespace",
                 false,
             };
         },
         .unnamed_test => info: {
+            if (iter.pass != .unnamed) return;
             const i = iter.unnamed_test_index;
             iter.unnamed_test_index += 1;
             break :info .{
-                try ip.getOrPutStringFmt(gpa, "test_{d}", .{i}),
+                try iter.avoidNameConflict("test_{d}", .{i}),
                 .@"test",
                 false,
             };
         },
         .decltest => info: {
+            // We consider these to be unnamed since the decl name can be adjusted to avoid conflicts if necessary.
+            if (iter.pass != .unnamed) return;
             assert(declaration.flags.has_doc_comment);
             const name = zir.nullTerminatedString(@enumFromInt(zir.extra[extra.end]));
             break :info .{
-                try ip.getOrPutStringFmt(gpa, "decltest.{s}", .{name}),
+                try iter.avoidNameConflict("decltest.{s}", .{name}),
                 .@"test",
                 true,
             };
         },
-        _ => if (declaration.name.isNamedTest(zir)) .{
-            try ip.getOrPutStringFmt(gpa, "test.{s}", .{zir.nullTerminatedString(declaration.name.toString(zir).?)}),
-            .@"test",
-            true,
-        } else .{
-            try ip.getOrPutString(gpa, zir.nullTerminatedString(declaration.name.toString(zir).?)),
-            .named,
-            false,
+        _ => if (declaration.name.isNamedTest(zir)) info: {
+            // We consider these to be unnamed since the decl name can be adjusted to avoid conflicts if necessary.
+            if (iter.pass != .unnamed) return;
+            break :info .{
+                try iter.avoidNameConflict("test.{s}", .{zir.nullTerminatedString(declaration.name.toString(zir).?)}),
+                .@"test",
+                true,
+            };
+        } else info: {
+            if (iter.pass != .named) return;
+            const name = try ip.getOrPutString(gpa, zir.nullTerminatedString(declaration.name.toString(zir).?));
+            try iter.seen_decls.putNoClobber(gpa, name, {});
+            break :info .{
+                name,
+                .named,
+                false,
+            };
         },
     };
 
@@ -4226,24 +4283,6 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void
     }
     const decl_index = gop.key_ptr.*;
     const decl = zcu.declPtr(decl_index);
-    if (kind == .@"test") {
-        const src_loc = SrcLoc{
-            .file_scope = decl.getFileScope(zcu),
-            .parent_decl_node = decl.src_node,
-            .lazy = .{ .token_offset = 1 },
-        };
-        const msg = try ErrorMsg.create(gpa, src_loc, "duplicate test name: {}", .{
-            decl_name.fmt(ip),
-        });
-        errdefer msg.destroy(gpa);
-        try zcu.failed_decls.putNoClobber(gpa, decl_index, msg);
-        const other_src_loc = SrcLoc{
-            .file_scope = namespace.file_scope,
-            .parent_decl_node = decl_node,
-            .lazy = .{ .token_offset = 1 },
-        };
-        try zcu.errNoteNonLazy(other_src_loc, msg, "other test here", .{});
-    }
     // Update the AST node of the decl; even if its contents are unchanged, it may
     // have been re-ordered.
     decl.src_node = decl_node;

test/cases/compile_errors/comptime_decl_name_conflict_resolved.zig

@@ -0,0 +1,8 @@
+comptime {
+    @compileError("should be reached");
+}
+const comptime_0 = {};
+
+// error
+//
+// :2:5: error: should be reached

test/cases/compile_errors/invalid_duplicate_test_decl_name.zig

@@ -6,5 +6,5 @@ test "thingy" {}
 // target=native
 // is_test=true
 //
-// :1:6: error: duplicate test name: test.thingy
-// :2:6: note: other test here
+// :2:1: error: duplicate test name 'thingy'
+// :1:1: note: other test here