| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2024-03-08 10:01:33+00:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2024-03-14 07:40:05+00:00 |
| log | 00969062a9d5e8229737f0ec38a8af91822caf31 |
| tree | 0ffc03a74a69ad3ea53b48fb72491c453bf62119 |
| parent | 778ab767b1e367233a1a3284e2c24d2c27b44602 |
| signature |  Commit is signed but in an unrecognized format. |
compiler: detect duplicate test names in AstGen
There is no reason to perform this detection during semantic analysis.
In fact, doing so is problematic, because we wish to utilize detection
of existing decls in a namespace in incremental compilation.3 files changed, 56 insertions(+), 21 deletions(-)
lib- std
- zig
- src
- test
- cases
- compile_errors
lib/std/zig/AstGen.zig+49-1
| ... | ... | @@ -13496,6 +13496,15 @@ fn scanDecls(astgen: *AstGen, namespace: *Scope.Namespace, members: []const Ast. |
| 13496 | 13496 | const node_tags = tree.nodes.items(.tag); |
| 13497 | 13497 | const main_tokens = tree.nodes.items(.main_token); |
| 13498 | 13498 | const token_tags = tree.tokens.items(.tag); |
| 13499 | ||
| 13500 | // We don't have shadowing for test names, so we just track those for duplicate reporting locally. | |
| 13501 | var named_tests: std.AutoHashMapUnmanaged(Zir.NullTerminatedString, Ast.Node.Index) = .{}; | |
| 13502 | var decltests: std.AutoHashMapUnmanaged(Zir.NullTerminatedString, Ast.Node.Index) = .{}; | |
| 13503 | defer { | |
| 13504 | named_tests.deinit(gpa); | |
| 13505 | decltests.deinit(gpa); | |
| 13506 | } | |
| 13507 | ||
| 13499 | 13508 | var decl_count: u32 = 0; |
| 13500 | 13509 | for (members) |member_node| { |
| 13501 | 13510 | const name_token = switch (node_tags[member_node]) { |
| ... | ... | @@ -13525,11 +13534,50 @@ fn scanDecls(astgen: *AstGen, namespace: *Scope.Namespace, members: []const Ast. |
| 13525 | 13534 | break :blk ident; |
| 13526 | 13535 | }, |
| 13527 | 13536 | |
| 13528 | .@"comptime", .@"usingnamespace", .test_decl => { | |
| 13537 | .@"comptime", .@"usingnamespace" => { | |
| 13529 | 13538 | decl_count += 1; |
| 13530 | 13539 | continue; |
| 13531 | 13540 | }, |
| 13532 | 13541 | |
| 13542 | .test_decl => { | |
| 13543 | decl_count += 1; | |
| 13544 | // We don't want shadowing detection here, and test names work a bit differently, so | |
| 13545 | // we must do the redeclaration detection ourselves. | |
| 13546 | const test_name_token = main_tokens[member_node] + 1; | |
| 13547 | switch (token_tags[test_name_token]) { | |
| 13548 | else => {}, // unnamed test | |
| 13549 | .string_literal => { | |
| 13550 | const name = try astgen.strLitAsString(test_name_token); | |
| 13551 | const gop = try named_tests.getOrPut(gpa, name.index); | |
| 13552 | if (gop.found_existing) { | |
| 13553 | const name_slice = astgen.string_bytes.items[@intFromEnum(name.index)..][0..name.len]; | |
| 13554 | const name_duped = try gpa.dupe(u8, name_slice); | |
| 13555 | defer gpa.free(name_duped); | |
| 13556 | try astgen.appendErrorNodeNotes(member_node, "duplicate test name '{s}'", .{name_duped}, &.{ | |
| 13557 | try astgen.errNoteNode(gop.value_ptr.*, "other test here", .{}), | |
| 13558 | }); | |
| 13559 | } else { | |
| 13560 | gop.value_ptr.* = member_node; | |
| 13561 | } | |
| 13562 | }, | |
| 13563 | .identifier => { | |
| 13564 | const name = try astgen.identAsString(test_name_token); | |
| 13565 | const gop = try decltests.getOrPut(gpa, name); | |
| 13566 | if (gop.found_existing) { | |
| 13567 | const name_slice = mem.span(astgen.nullTerminatedString(name)); | |
| 13568 | const name_duped = try gpa.dupe(u8, name_slice); | |
| 13569 | defer gpa.free(name_duped); | |
| 13570 | try astgen.appendErrorNodeNotes(member_node, "duplicate decltest '{s}'", .{name_duped}, &.{ | |
| 13571 | try astgen.errNoteNode(gop.value_ptr.*, "other decltest here", .{}), | |
| 13572 | }); | |
| 13573 | } else { | |
| 13574 | gop.value_ptr.* = member_node; | |
| 13575 | } | |
| 13576 | }, | |
| 13577 | } | |
| 13578 | continue; | |
| 13579 | }, | |
| 13580 | ||
| 13533 | 13581 | else => continue, |
| 13534 | 13582 | }; |
| 13535 | 13583 |
src/Module.zig+5-18
| ... | ... | @@ -4128,6 +4128,7 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4128 | 4128 | .@"comptime" => info: { |
| 4129 | 4129 | const i = iter.comptime_index; |
| 4130 | 4130 | iter.comptime_index += 1; |
| 4131 | // TODO: avoid collisions with named decls with this name | |
| 4131 | 4132 | break :info .{ |
| 4132 | 4133 | try ip.getOrPutStringFmt(gpa, "comptime_{d}", .{i}), |
| 4133 | 4134 | .@"comptime", |
| ... | ... | @@ -4137,6 +4138,7 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4137 | 4138 | .@"usingnamespace" => info: { |
| 4138 | 4139 | const i = iter.usingnamespace_index; |
| 4139 | 4140 | iter.usingnamespace_index += 1; |
| 4141 | // TODO: avoid collisions with named decls with this name | |
| 4140 | 4142 | break :info .{ |
| 4141 | 4143 | try ip.getOrPutStringFmt(gpa, "usingnamespace_{d}", .{i}), |
| 4142 | 4144 | .@"usingnamespace", |
| ... | ... | @@ -4146,6 +4148,7 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4146 | 4148 | .unnamed_test => info: { |
| 4147 | 4149 | const i = iter.unnamed_test_index; |
| 4148 | 4150 | iter.unnamed_test_index += 1; |
| 4151 | // TODO: avoid collisions with named decls with this name | |
| 4149 | 4152 | break :info .{ |
| 4150 | 4153 | try ip.getOrPutStringFmt(gpa, "test_{d}", .{i}), |
| 4151 | 4154 | .@"test", |
| ... | ... | @@ -4155,6 +4158,7 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4155 | 4158 | .decltest => info: { |
| 4156 | 4159 | assert(declaration.flags.has_doc_comment); |
| 4157 | 4160 | const name = zir.nullTerminatedString(@enumFromInt(zir.extra[extra.end])); |
| 4161 | // TODO: avoid collisions with named decls with this name | |
| 4158 | 4162 | break :info .{ |
| 4159 | 4163 | try ip.getOrPutStringFmt(gpa, "decltest.{s}", .{name}), |
| 4160 | 4164 | .@"test", |
| ... | ... | @@ -4162,6 +4166,7 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4162 | 4166 | }; |
| 4163 | 4167 | }, |
| 4164 | 4168 | _ => if (declaration.name.isNamedTest(zir)) .{ |
| 4169 | // TODO: avoid collisions with named decls with this name | |
| 4165 | 4170 | try ip.getOrPutStringFmt(gpa, "test.{s}", .{zir.nullTerminatedString(declaration.name.toString(zir).?)}), |
| 4166 | 4171 | .@"test", |
| 4167 | 4172 | true, |
| ... | ... | @@ -4226,24 +4231,6 @@ fn scanDecl(iter: *ScanDeclIter, decl_inst: Zir.Inst.Index) Allocator.Error!void |
| 4226 | 4231 | } |
| 4227 | 4232 | const decl_index = gop.key_ptr.*; |
| 4228 | 4233 | const decl = zcu.declPtr(decl_index); |
| 4229 | if (kind == .@"test") { | |
| 4230 | const src_loc = SrcLoc{ | |
| 4231 | .file_scope = decl.getFileScope(zcu), | |
| 4232 | .parent_decl_node = decl.src_node, | |
| 4233 | .lazy = .{ .token_offset = 1 }, | |
| 4234 | }; | |
| 4235 | const msg = try ErrorMsg.create(gpa, src_loc, "duplicate test name: {}", .{ | |
| 4236 | decl_name.fmt(ip), | |
| 4237 | }); | |
| 4238 | errdefer msg.destroy(gpa); | |
| 4239 | try zcu.failed_decls.putNoClobber(gpa, decl_index, msg); | |
| 4240 | const other_src_loc = SrcLoc{ | |
| 4241 | .file_scope = namespace.file_scope, | |
| 4242 | .parent_decl_node = decl_node, | |
| 4243 | .lazy = .{ .token_offset = 1 }, | |
| 4244 | }; | |
| 4245 | try zcu.errNoteNonLazy(other_src_loc, msg, "other test here", .{}); | |
| 4246 | } | |
| 4247 | 4234 | // Update the AST node of the decl; even if its contents are unchanged, it may |
| 4248 | 4235 | // have been re-ordered. |
| 4249 | 4236 | decl.src_node = decl_node; |
test/cases/compile_errors/invalid_duplicate_test_decl_name.zig+2-2
| ... | ... | @@ -6,5 +6,5 @@ test "thingy" {} |
| 6 | 6 | // target=native |
| 7 | 7 | // is_test=true |
| 8 | 8 | // |
| 9 | // :1:6: error: duplicate test name: test.thingy | |
| 10 | // :2:6: note: other test here | |
| 9 | // :2:1: error: duplicate test name 'thingy' | |
| 10 | // :1:1: note: other test here |