authorgravatar for david@vortan.devDavid Rubin <david@vortan.dev> 2024-01-20 09:23:47-08:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2024-01-20 17:23:47+00:00
log1b8f7e46fa33cda57caf39c5a5a0f5f52335a99b
tree9ae103c1d6c72e8f29aae0adb4dbc99208781244
parent5c4cb60f4fab0f3c0fde43e04bbc4a03c92bef8e
signaturebadge-check Signed by PGP key B5690EEEBB952194

AstGen: detect duplicate field names

This logic was previously in Sema, which was unnecessary complexity, and meant the issue was not detected unless the declaration was semantically analyzed. This commit finishes the work which 941090d started. Resolves: #17916

13 files changed, 138 insertions(+), 87 deletions(-)

src/AstGen.zig+101-11
......@@ -1753,7 +1753,6 @@ fn structInitExpr(
17531753 const sfba_allocator = sfba.get();
17541754
17551755 var duplicate_names = std.AutoArrayHashMap(Zir.NullTerminatedString, ArrayListUnmanaged(Ast.TokenIndex)).init(sfba_allocator);
1756 defer duplicate_names.deinit();
17571756 try duplicate_names.ensureTotalCapacity(@intCast(struct_init.ast.fields.len));
17581757
17591758 // When there aren't errors, use this to avoid a second iteration.
......@@ -1783,12 +1782,14 @@ fn structInitExpr(
17831782 var error_notes = std.ArrayList(u32).init(astgen.arena);
17841783
17851784 for (record.items[1..]) |duplicate| {
1786 try error_notes.append(try astgen.errNoteTok(duplicate, "other field here", .{}));
1785 try error_notes.append(try astgen.errNoteTok(duplicate, "duplicate name here", .{}));
17871786 }
17881787
1788 try error_notes.append(try astgen.errNoteNode(node, "struct declared here", .{}));
1789
17891790 try astgen.appendErrorTokNotes(
17901791 record.items[0],
1791 "duplicate field",
1792 "duplicate struct field name",
17921793 .{},
17931794 error_notes.items,
17941795 );
......@@ -1815,9 +1816,10 @@ fn structInitExpr(
18151816 switch (ri.rl) {
18161817 .none => return structInitExprAnon(gz, scope, node, struct_init),
18171818 .discard => {
1818 // Even if discarding we must perform an anonymous init to check for duplicate field names.
1819 // TODO: should duplicate field names be caught in AstGen?
1820 _ = try structInitExprAnon(gz, scope, node, struct_init);
1819 // Even if discarding we must perform side-effects.
1820 for (struct_init.ast.fields) |field_init| {
1821 _ = try expr(gz, scope, .{ .rl = .discard }, field_init);
1822 }
18211823 return .void_value;
18221824 },
18231825 .ref => {
......@@ -5101,15 +5103,15 @@ fn structDeclInner(
51015103 var error_notes = std.ArrayList(u32).init(astgen.arena);
51025104
51035105 for (record.items[1..]) |duplicate| {
5104 try error_notes.append(try astgen.errNoteTok(duplicate, "other field here", .{}));
5106 try error_notes.append(try astgen.errNoteTok(duplicate, "duplicate field here", .{}));
51055107 }
51065108
51075109 try error_notes.append(try astgen.errNoteNode(node, "struct declared here", .{}));
51085110
51095111 try astgen.appendErrorTokNotes(
51105112 record.items[0],
5111 "duplicate struct field: '{s}'",
5112 .{try astgen.identifierTokenString(record.items[0])},
5113 "duplicate struct field name",
5114 .{},
51135115 error_notes.items,
51145116 );
51155117 }
......@@ -5118,8 +5120,6 @@ fn structDeclInner(
51185120 return error.AnalysisFail;
51195121 }
51205122
5121 duplicate_names.deinit();
5122
51235123 try gz.setStruct(decl_inst, .{
51245124 .src_node = node,
51255125 .layout = layout,
......@@ -5211,6 +5211,15 @@ fn unionDeclInner(
52115211 var wip_members = try WipMembers.init(gpa, &astgen.scratch, decl_count, field_count, bits_per_field, max_field_size);
52125212 defer wip_members.deinit();
52135213
5214 var sfba = std.heap.stackFallback(256, astgen.arena);
5215 const sfba_allocator = sfba.get();
5216
5217 var duplicate_names = std.AutoArrayHashMap(Zir.NullTerminatedString, std.ArrayListUnmanaged(Ast.TokenIndex)).init(sfba_allocator);
5218 try duplicate_names.ensureTotalCapacity(field_count);
5219
5220 // When there aren't errors, use this to avoid a second iteration.
5221 var any_duplicate = false;
5222
52145223 for (members) |member_node| {
52155224 var member = switch (try containerMember(&block_scope, &namespace.base, &wip_members, member_node)) {
52165225 .decl => continue,
......@@ -5227,6 +5236,16 @@ fn unionDeclInner(
52275236 const field_name = try astgen.identAsString(member.ast.main_token);
52285237 wip_members.appendToField(@intFromEnum(field_name));
52295238
5239 const gop = try duplicate_names.getOrPut(field_name);
5240
5241 if (gop.found_existing) {
5242 try gop.value_ptr.append(sfba_allocator, member.ast.main_token);
5243 any_duplicate = true;
5244 } else {
5245 gop.value_ptr.* = .{};
5246 try gop.value_ptr.append(sfba_allocator, member.ast.main_token);
5247 }
5248
52305249 const doc_comment_index = try astgen.docCommentAsString(member.firstToken());
52315250 wip_members.appendToField(@intFromEnum(doc_comment_index));
52325251
......@@ -5281,6 +5300,32 @@ fn unionDeclInner(
52815300 }
52825301 }
52835302
5303 if (any_duplicate) {
5304 var it = duplicate_names.iterator();
5305
5306 while (it.next()) |entry| {
5307 const record = entry.value_ptr.*;
5308 if (record.items.len > 1) {
5309 var error_notes = std.ArrayList(u32).init(astgen.arena);
5310
5311 for (record.items[1..]) |duplicate| {
5312 try error_notes.append(try astgen.errNoteTok(duplicate, "duplicate field here", .{}));
5313 }
5314
5315 try error_notes.append(try astgen.errNoteNode(node, "union declared here", .{}));
5316
5317 try astgen.appendErrorTokNotes(
5318 record.items[0],
5319 "duplicate union field name",
5320 .{},
5321 error_notes.items,
5322 );
5323 }
5324 }
5325
5326 return error.AnalysisFail;
5327 }
5328
52845329 if (!block_scope.isEmpty()) {
52855330 _ = try block_scope.addBreak(.break_inline, decl_inst, .void_value);
52865331 }
......@@ -5490,6 +5535,15 @@ fn containerDecl(
54905535 var wip_members = try WipMembers.init(gpa, &astgen.scratch, @intCast(counts.decls), @intCast(counts.total_fields), bits_per_field, max_field_size);
54915536 defer wip_members.deinit();
54925537
5538 var sfba = std.heap.stackFallback(256, astgen.arena);
5539 const sfba_allocator = sfba.get();
5540
5541 var duplicate_names = std.AutoArrayHashMap(Zir.NullTerminatedString, std.ArrayListUnmanaged(Ast.TokenIndex)).init(sfba_allocator);
5542 try duplicate_names.ensureTotalCapacity(counts.total_fields);
5543
5544 // When there aren't errors, use this to avoid a second iteration.
5545 var any_duplicate = false;
5546
54935547 for (container_decl.ast.members) |member_node| {
54945548 if (member_node == counts.nonexhaustive_node)
54955549 continue;
......@@ -5506,6 +5560,16 @@ fn containerDecl(
55065560 const field_name = try astgen.identAsString(member.ast.main_token);
55075561 wip_members.appendToField(@intFromEnum(field_name));
55085562
5563 const gop = try duplicate_names.getOrPut(field_name);
5564
5565 if (gop.found_existing) {
5566 try gop.value_ptr.append(sfba_allocator, member.ast.main_token);
5567 any_duplicate = true;
5568 } else {
5569 gop.value_ptr.* = .{};
5570 try gop.value_ptr.append(sfba_allocator, member.ast.main_token);
5571 }
5572
55095573 const doc_comment_index = try astgen.docCommentAsString(member.firstToken());
55105574 wip_members.appendToField(@intFromEnum(doc_comment_index));
55115575
......@@ -5533,6 +5597,32 @@ fn containerDecl(
55335597 }
55345598 }
55355599
5600 if (any_duplicate) {
5601 var it = duplicate_names.iterator();
5602
5603 while (it.next()) |entry| {
5604 const record = entry.value_ptr.*;
5605 if (record.items.len > 1) {
5606 var error_notes = std.ArrayList(u32).init(astgen.arena);
5607
5608 for (record.items[1..]) |duplicate| {
5609 try error_notes.append(try astgen.errNoteTok(duplicate, "duplicate field here", .{}));
5610 }
5611
5612 try error_notes.append(try astgen.errNoteNode(node, "enum declared here", .{}));
5613
5614 try astgen.appendErrorTokNotes(
5615 record.items[0],
5616 "duplicate enum field name",
5617 .{},
5618 error_notes.items,
5619 );
5620 }
5621 }
5622
5623 return error.AnalysisFail;
5624 }
5625
55365626 if (!block_scope.isEmpty()) {
55375627 _ = try block_scope.addBreak(.break_inline, decl_inst, .void_value);
55385628 }
src/Sema.zig+4-49
......@@ -3046,23 +3046,10 @@ fn zirEnumDecl(
30463046
30473047 const field_name_index: Zir.NullTerminatedString = @enumFromInt(sema.code.extra[extra_index]);
30483048 const field_name_zir = sema.code.nullTerminatedString(field_name_index);
3049 extra_index += 1;
3050
3051 // doc comment
3052 extra_index += 1;
3049 extra_index += 2; // field name, doc comment
30533050
30543051 const field_name = try mod.intern_pool.getOrPutString(gpa, field_name_zir);
3055 if (incomplete_enum.addFieldName(&mod.intern_pool, field_name)) |other_index| {
3056 const field_src = mod.fieldSrcLoc(new_decl_index, .{ .index = field_i }).lazy;
3057 const other_field_src = mod.fieldSrcLoc(new_decl_index, .{ .index = other_index }).lazy;
3058 const msg = msg: {
3059 const msg = try sema.errMsg(block, field_src, "duplicate enum field '{s}'", .{field_name_zir});
3060 errdefer msg.destroy(gpa);
3061 try sema.errNote(block, other_field_src, msg, "other field here", .{});
3062 break :msg msg;
3063 };
3064 return sema.failWithOwnedErrorMsg(block, msg);
3065 }
3052 assert(incomplete_enum.addFieldName(&mod.intern_pool, field_name) == null);
30663053
30673054 const tag_overflow = if (has_tag_value) overflow: {
30683055 const tag_val_ref: Zir.Inst.Ref = @enumFromInt(sema.code.extra[extra_index]);
......@@ -21732,7 +21719,7 @@ fn reifyStruct(
2173221719 }
2173321720 } else if (struct_type.addFieldName(ip, field_name)) |prev_index| {
2173421721 _ = prev_index; // TODO: better source location
21735 return sema.fail(block, src, "duplicate struct field {}", .{field_name.fmt(ip)});
21722 return sema.fail(block, src, "duplicate struct field name {}", .{field_name.fmt(ip)});
2173621723 }
2173721724
2173821725 const field_ty = type_val.toType();
......@@ -36284,7 +36271,6 @@ fn semaStructFields(
3628436271 const zir = mod.namespacePtr(namespace_index).file_scope.zir;
3628536272 const zir_index = struct_type.zir_index;
3628636273
36287 const src = LazySrcLoc.nodeOffset(0);
3628836274 const fields_len, const small, var extra_index = structZirInfo(zir, zir_index);
3628936275
3629036276 if (fields_len == 0) switch (struct_type.layout) {
......@@ -36384,19 +36370,7 @@ fn semaStructFields(
3638436370 // This string needs to outlive the ZIR code.
3638536371 if (opt_field_name_zir) |field_name_zir| {
3638636372 const field_name = try ip.getOrPutString(gpa, field_name_zir);
36387 if (struct_type.addFieldName(ip, field_name)) |other_index| {
36388 const msg = msg: {
36389 const field_src = mod.fieldSrcLoc(decl_index, .{ .index = field_i }).lazy;
36390 const msg = try sema.errMsg(&block_scope, field_src, "duplicate struct field: '{}'", .{field_name.fmt(ip)});
36391 errdefer msg.destroy(gpa);
36392
36393 const prev_field_src = mod.fieldSrcLoc(decl_index, .{ .index = other_index });
36394 try mod.errNoteNonLazy(prev_field_src, msg, "other field here", .{});
36395 try sema.errNote(&block_scope, src, msg, "struct declared here", .{});
36396 break :msg msg;
36397 };
36398 return sema.failWithOwnedErrorMsg(&block_scope, msg);
36399 }
36373 assert(struct_type.addFieldName(ip, field_name) == null);
3640036374 }
3640136375
3640236376 if (has_align) {
......@@ -36840,12 +36814,10 @@ fn semaUnionFields(mod: *Module, arena: Allocator, union_type: InternPool.Key.Un
3684036814
3684136815 var field_types: std.ArrayListUnmanaged(InternPool.Index) = .{};
3684236816 var field_aligns: std.ArrayListUnmanaged(InternPool.Alignment) = .{};
36843 var field_name_table: std.AutoArrayHashMapUnmanaged(InternPool.NullTerminatedString, void) = .{};
3684436817
3684536818 try field_types.ensureTotalCapacityPrecise(sema.arena, fields_len);
3684636819 if (small.any_aligned_fields)
3684736820 try field_aligns.ensureTotalCapacityPrecise(sema.arena, fields_len);
36848 try field_name_table.ensureTotalCapacity(sema.arena, fields_len);
3684936821
3685036822 const bits_per_field = 4;
3685136823 const fields_per_u32 = 32 / bits_per_field;
......@@ -36961,23 +36933,6 @@ fn semaUnionFields(mod: *Module, arena: Allocator, union_type: InternPool.Key.Un
3696136933 return error.GenericPoison;
3696236934 }
3696336935
36964 const gop = field_name_table.getOrPutAssumeCapacity(field_name);
36965 if (gop.found_existing) {
36966 const msg = msg: {
36967 const field_src = mod.fieldSrcLoc(union_type.decl, .{ .index = field_i }).lazy;
36968 const msg = try sema.errMsg(&block_scope, field_src, "duplicate union field: '{}'", .{
36969 field_name.fmt(ip),
36970 });
36971 errdefer msg.destroy(gpa);
36972
36973 const prev_field_src = mod.fieldSrcLoc(union_type.decl, .{ .index = gop.index }).lazy;
36974 try mod.errNoteNonLazy(prev_field_src.toSrcLoc(decl, mod), msg, "other field here", .{});
36975 try sema.errNote(&block_scope, src, msg, "union declared here", .{});
36976 break :msg msg;
36977 };
36978 return sema.failWithOwnedErrorMsg(&block_scope, msg);
36979 }
36980
3698136936 if (explicit_tags_seen.len > 0) {
3698236937 const tag_info = ip.indexToKey(union_type.tagTypePtr(ip).*).enum_type;
3698336938 const enum_index = tag_info.nameIndex(ip, field_name) orelse {
test/cases/compile_errors/duplicate_enum_field.zig+3-2
......@@ -12,5 +12,6 @@ export fn entry() void {
1212// backend=stage2
1313// target=native
1414//
15// :3:5: error: duplicate enum field 'Bar'
16// :2:5: note: other field here
15// :2:5: error: duplicate enum field name
16// :3:5: note: duplicate field here
17// :1:13: note: enum declared here
test/cases/compile_errors/duplicate_field_in_anonymous_struct_literal.zig+3-2
......@@ -14,5 +14,6 @@ export fn entry() void {
1414// backend=stage2
1515// target=native
1616//
17// :4:14: error: duplicate field
18// :7:14: note: other field here
17// :4:14: error: duplicate struct field name
18// :7:14: note: duplicate name here
19// :3:19: note: struct declared here
test/cases/compile_errors/duplicate_field_in_discarded_anon_init.zig+3-2
......@@ -6,5 +6,6 @@ pub export fn entry() void {
66// backend=stage2
77// target=native
88//
9// :2:13: error: duplicate field
10// :2:21: note: other field here
9// :2:13: error: duplicate struct field name
10// :2:21: note: duplicate name here
11// :2:10: note: struct declared here
test/cases/compile_errors/duplicate_field_in_struct_value_expression.zig+3-2
......@@ -17,5 +17,6 @@ export fn f() void {
1717// backend=stage2
1818// target=native
1919//
20// :8:10: error: duplicate field
21// :11:10: note: other field here
20// :8:10: error: duplicate struct field name
21// :11:10: note: duplicate name here
22// :7:16: note: struct declared here
test/cases/compile_errors/duplicate_struct_field.zig+5-5
......@@ -24,10 +24,10 @@ export fn b() void {
2424// backend=stage2
2525// target=native
2626//
27// :2:5: error: duplicate struct field: 'Bar'
28// :3:5: note: other field here
27// :2:5: error: duplicate struct field name
28// :3:5: note: duplicate field here
2929// :1:13: note: struct declared here
30// :7:5: error: duplicate struct field: 'a'
31// :9:5: note: other field here
32// :10:5: note: other field here
30// :7:5: error: duplicate struct field name
31// :9:5: note: duplicate field here
32// :10:5: note: duplicate field here
3333// :6:11: note: struct declared here
test/cases/compile_errors/duplicate_union_field.zig+2-2
......@@ -11,6 +11,6 @@ export fn entry() void {
1111// backend=stage2
1212// target=native
1313//
14// :3:5: error: duplicate union field: 'Bar'
15// :2:5: note: other field here
14// :2:5: error: duplicate union field name
15// :3:5: note: duplicate field here
1616// :1:13: note: union declared here
test/cases/compile_errors/error_in_struct_initializer_doesnt_crash_the_compiler.zig+2-2
......@@ -11,6 +11,6 @@ pub export fn entry() void {
1111// backend=stage2
1212// target=native
1313//
14// :3:9: error: duplicate struct field: 'e'
15// :4:9: note: other field here
14// :3:9: error: duplicate struct field name
15// :4:9: note: duplicate field here
1616// :2:22: note: struct declared here
test/cases/compile_errors/struct_duplicate_field_name.zig+2-2
......@@ -11,6 +11,6 @@ export fn entry() void {
1111// error
1212// target=native
1313//
14// :2:5: error: duplicate struct field: 'foo'
15// :3:5: note: other field here
14// :2:5: error: duplicate struct field name
15// :3:5: note: duplicate field here
1616// :1:11: note: struct declared here
test/cases/compile_errors/union_duplicate_enum_field.zig+2-2
......@@ -12,6 +12,6 @@ export fn foo() void {
1212// error
1313// target=native
1414//
15// :4:5: error: duplicate union field: 'a'
16// :3:5: note: other field here
15// :3:5: error: duplicate union field name
16// :4:5: note: duplicate field here
1717// :2:11: note: union declared here
test/cases/compile_errors/union_duplicate_field_definition.zig+2-2
......@@ -11,6 +11,6 @@ export fn entry() void {
1111// error
1212// target=native
1313//
14// :3:5: error: duplicate union field: 'foo'
15// :2:5: note: other field here
14// :2:5: error: duplicate union field name
15// :3:5: note: duplicate field here
1616// :1:11: note: union declared here
test/cbe.zig+6-4
......@@ -507,8 +507,9 @@ pub fn addCases(ctx: *Cases, b: *std.Build) !void {
507507 \\ return p.y - p.x - p.x;
508508 \\}
509509 , &.{
510 ":4:10: error: duplicate field",
511 ":6:10: note: other field here",
510 ":4:10: error: duplicate struct field name",
511 ":6:10: note: duplicate name here",
512 ":3:21: note: struct declared here",
512513 });
513514 case.addError(
514515 \\const Point = struct { x: i32, y: i32 };
......@@ -722,8 +723,9 @@ pub fn addCases(ctx: *Cases, b: *std.Build) !void {
722723 \\ _ = E1.a;
723724 \\}
724725 , &.{
725 ":1:28: error: duplicate enum field 'b'",
726 ":1:22: note: other field here",
726 ":1:22: error: duplicate enum field name",
727 ":1:28: note: duplicate field here",
728 ":1:12: note: enum declared here",
727729 });
728730
729731 case.addError(