From 941090d94f6b6c7c5207f06917743e79ff06426f Mon Sep 17 00:00:00 2001 From: David <87927264+Rexicon226@users.noreply.github.com> Date: Thu, 16 Nov 2023 06:38:16 -0800 Subject: [PATCH] Move duplicate field detection for struct init expressions into AstGen Partially addresses #17916. --- lib/std/os/linux/sparc64.zig | 2 +- src/AstGen.zig | 101 +++++++++++++++++- src/Sema.zig | 59 ++-------- ...cate_field_in_anonymous_struct_literal.zig | 4 +- ...duplicate_field_in_discarded_anon_init.zig | 4 +- ...icate_field_in_struct_value_expression.zig | 4 +- .../compile_errors/duplicate_struct_field.zig | 27 ++++- ..._initializer_doesnt_crash_the_compiler.zig | 4 +- .../struct_duplicate_field_name.zig | 4 +- test/cbe.zig | 4 +- 10 files changed, 146 insertions(+), 67 deletions(-) diff --git a/lib/std/os/linux/sparc64.zig b/lib/std/os/linux/sparc64.zig index c741df9897..293f8b6ce2 100644 --- a/lib/std/os/linux/sparc64.zig +++ b/lib/std/os/linux/sparc64.zig @@ -445,7 +445,7 @@ pub const ucontext_t = extern struct { sigmask: u64, mcontext: mcontext_t, stack: stack_t, - sigmask: sigset_t, + sigset: sigset_t, }; pub const rlimit_resource = enum(c_int) { diff --git a/src/AstGen.zig b/src/AstGen.zig index 5b957e48c5..5638216ed1 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -1722,6 +1722,57 @@ fn structInitExpr( } } + { + var sfba = std.heap.stackFallback(256, astgen.arena); + const sfba_allocator = sfba.get(); + + var duplicate_names = std.AutoArrayHashMap(u32, ArrayListUnmanaged(Ast.TokenIndex)).init(sfba_allocator); + defer duplicate_names.deinit(); + try duplicate_names.ensureTotalCapacity(@intCast(struct_init.ast.fields.len)); + + // When there aren't errors, use this to avoid a second iteration. + var any_duplicate = false; + + for (struct_init.ast.fields) |field| { + const name_token = tree.firstToken(field) - 2; + const name_index = try astgen.identAsString(name_token); + + const gop = try duplicate_names.getOrPut(name_index); + + if (gop.found_existing) { + try gop.value_ptr.append(sfba_allocator, name_token); + any_duplicate = true; + } else { + gop.value_ptr.* = .{}; + try gop.value_ptr.append(sfba_allocator, name_token); + } + } + + if (any_duplicate) { + var it = duplicate_names.iterator(); + + while (it.next()) |entry| { + const record = entry.value_ptr.*; + if (record.items.len > 1) { + var error_notes = std.ArrayList(u32).init(astgen.arena); + + for (record.items[1..]) |duplicate| { + try error_notes.append(try astgen.errNoteTok(duplicate, "other field here", .{})); + } + + try astgen.appendErrorTokNotes( + record.items[0], + "duplicate field", + .{}, + error_notes.items, + ); + } + } + + return error.AnalysisFail; + } + } + if (struct_init.ast.type_expr != 0) { // Typed inits do not use RLS for language simplicity. const ty_inst = try typeExpr(gz, scope, struct_init.ast.type_expr); @@ -4874,6 +4925,15 @@ fn structDeclInner( } }; + var sfba = std.heap.stackFallback(256, astgen.arena); + const sfba_allocator = sfba.get(); + + var duplicate_names = std.AutoArrayHashMap(u32, std.ArrayListUnmanaged(Ast.TokenIndex)).init(sfba_allocator); + try duplicate_names.ensureTotalCapacity(field_count); + + // When there aren't errors, use this to avoid a second iteration. + var any_duplicate = false; + var known_non_opv = false; var known_comptime_only = false; var any_comptime_fields = false; @@ -4886,11 +4946,22 @@ fn structDeclInner( }; if (!is_tuple) { + const field_name = try astgen.identAsString(member.ast.main_token); + member.convertToNonTupleLike(astgen.tree.nodes); assert(!member.ast.tuple_like); - const field_name = try astgen.identAsString(member.ast.main_token); wip_members.appendToField(field_name); + + const gop = try duplicate_names.getOrPut(field_name); + + if (gop.found_existing) { + try gop.value_ptr.append(sfba_allocator, member.ast.main_token); + any_duplicate = true; + } else { + gop.value_ptr.* = .{}; + try gop.value_ptr.append(sfba_allocator, member.ast.main_token); + } } else if (!member.ast.tuple_like) { return astgen.failTok(member.ast.main_token, "tuple field has a name", .{}); } @@ -4975,6 +5046,34 @@ fn structDeclInner( } } + if (any_duplicate) { + var it = duplicate_names.iterator(); + + while (it.next()) |entry| { + const record = entry.value_ptr.*; + if (record.items.len > 1) { + var error_notes = std.ArrayList(u32).init(astgen.arena); + + for (record.items[1..]) |duplicate| { + try error_notes.append(try astgen.errNoteTok(duplicate, "other field here", .{})); + } + + try error_notes.append(try astgen.errNoteNode(node, "struct declared here", .{})); + + try astgen.appendErrorTokNotes( + record.items[0], + "duplicate struct field: '{s}'", + .{try astgen.identifierTokenString(record.items[0])}, + error_notes.items, + ); + } + } + + return error.AnalysisFail; + } + + duplicate_names.deinit(); + try gz.setStruct(decl_inst, .{ .src_node = node, .layout = layout, diff --git a/src/Sema.zig b/src/Sema.zig index 2615a5b27d..39fd818d97 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -4690,17 +4690,7 @@ fn validateStructInit( try sema.tupleFieldIndex(block, struct_ty, field_name, field_src) else try sema.structFieldIndex(block, struct_ty, field_name, field_src); - if (found_fields[field_index.*].unwrap()) |other_field_ptr| { - const other_field_ptr_data = sema.code.instructions.items(.data)[@intFromEnum(other_field_ptr)].pl_node; - const other_field_src: LazySrcLoc = .{ .node_offset_initializer = other_field_ptr_data.src_node }; - const msg = msg: { - const msg = try sema.errMsg(block, field_src, "duplicate field", .{}); - errdefer msg.destroy(gpa); - try sema.errNote(block, other_field_src, msg, "other field here", .{}); - break :msg msg; - }; - return sema.failWithOwnedErrorMsg(block, msg); - } + assert(found_fields[field_index.*] == .none); found_fields[field_index.*] = field_ptr.toOptional(); } @@ -19222,18 +19212,7 @@ fn zirStructInit( try sema.tupleFieldIndex(block, resolved_ty, field_name, field_src) else try sema.structFieldIndex(block, resolved_ty, field_name, field_src); - if (field_inits[field_index] != .none) { - const other_field_type = found_fields[field_index]; - const other_field_type_data = zir_datas[@intFromEnum(other_field_type)].pl_node; - const other_field_src: LazySrcLoc = .{ .node_offset_initializer = other_field_type_data.src_node }; - const msg = msg: { - const msg = try sema.errMsg(block, field_src, "duplicate field", .{}); - errdefer msg.destroy(gpa); - try sema.errNote(block, other_field_src, msg, "other field here", .{}); - break :msg msg; - }; - return sema.failWithOwnedErrorMsg(block, msg); - } + assert(field_inits[field_index] == .none); found_fields[field_index] = item.data.field_type; const uncoerced_init = try sema.resolveInst(item.data.init); const field_ty = resolved_ty.structFieldType(field_index, mod); @@ -19533,16 +19512,13 @@ fn structInitAnon( const types = try sema.arena.alloc(InternPool.Index, extra_data.fields_len); const values = try sema.arena.alloc(InternPool.Index, types.len); - - var fields = std.AutoArrayHashMap(InternPool.NullTerminatedString, u32).init(sema.arena); - try fields.ensureUnusedCapacity(types.len); + const names = try sema.arena.alloc(InternPool.NullTerminatedString, types.len); // Find which field forces the expression to be runtime, if any. const opt_runtime_index = rs: { var runtime_index: ?usize = null; var extra_index = extra_end; - for (types, 0..) |*field_ty, i_usize| { - const i: u32 = @intCast(i_usize); + for (types, values, names, 0..) |*field_ty, *field_val, *field_name, i_usize| { const item = switch (kind) { .anon_init => sema.code.extraData(Zir.Inst.StructInitAnon.Item, extra_index), .typed_init => sema.code.extraData(Zir.Inst.StructInit.Item, extra_index), @@ -19558,29 +19534,16 @@ fn structInitAnon( break :name sema.code.nullTerminatedString(field_type_extra.data.name_start); }, }; - const name_ip = try mod.intern_pool.getOrPutString(gpa, name); - const gop = fields.getOrPutAssumeCapacity(name_ip); - if (gop.found_existing) { - const msg = msg: { - const decl = mod.declPtr(block.src_decl); - const field_src = mod.initSrc(src.node_offset.x, decl, i); - const msg = try sema.errMsg(block, field_src, "duplicate field", .{}); - errdefer msg.destroy(gpa); - const prev_source = mod.initSrc(src.node_offset.x, decl, gop.value_ptr.*); - try sema.errNote(block, prev_source, msg, "other field here", .{}); - break :msg msg; - }; - return sema.failWithOwnedErrorMsg(block, msg); - } - gop.value_ptr.* = i; + const name_ip = try mod.intern_pool.getOrPutString(gpa, name); + field_name.* = name_ip; const init = try sema.resolveInst(item.data.init); field_ty.* = sema.typeOf(init).toIntern(); if (field_ty.toType().zigTypeTag(mod) == .Opaque) { const msg = msg: { const decl = mod.declPtr(block.src_decl); - const field_src = mod.initSrc(src.node_offset.x, decl, i); + const field_src = mod.initSrc(src.node_offset.x, decl, @intCast(i_usize)); const msg = try sema.errMsg(block, field_src, "opaque types have unknown size and therefore cannot be directly embedded in structs", .{}); errdefer msg.destroy(sema.gpa); @@ -19590,17 +19553,17 @@ fn structInitAnon( return sema.failWithOwnedErrorMsg(block, msg); } if (try sema.resolveValue(init)) |init_val| { - values[i] = try init_val.intern(field_ty.toType(), mod); + field_val.* = try init_val.intern(field_ty.toType(), mod); } else { - values[i] = .none; - runtime_index = i; + field_val.* = .none; + runtime_index = @intCast(i_usize); } } break :rs runtime_index; }; const tuple_ty = try ip.getAnonStructType(gpa, .{ - .names = fields.keys(), + .names = names, .types = types, .values = values, }); diff --git a/test/cases/compile_errors/duplicate_field_in_anonymous_struct_literal.zig b/test/cases/compile_errors/duplicate_field_in_anonymous_struct_literal.zig index ed153c1542..ee81f72b1d 100644 --- a/test/cases/compile_errors/duplicate_field_in_anonymous_struct_literal.zig +++ b/test/cases/compile_errors/duplicate_field_in_anonymous_struct_literal.zig @@ -14,5 +14,5 @@ export fn entry() void { // backend=stage2 // target=native // -// :7:16: error: duplicate field -// :4:16: note: other field here +// :4:14: error: duplicate field +// :7:14: note: other field here diff --git a/test/cases/compile_errors/duplicate_field_in_discarded_anon_init.zig b/test/cases/compile_errors/duplicate_field_in_discarded_anon_init.zig index 6f850e6fe4..ff1912d27b 100644 --- a/test/cases/compile_errors/duplicate_field_in_discarded_anon_init.zig +++ b/test/cases/compile_errors/duplicate_field_in_discarded_anon_init.zig @@ -6,5 +6,5 @@ pub export fn entry() void { // backend=stage2 // target=native // -// :2:21: error: duplicate field -// :2:13: note: other field here +// :2:13: error: duplicate field +// :2:21: note: other field here diff --git a/test/cases/compile_errors/duplicate_field_in_struct_value_expression.zig b/test/cases/compile_errors/duplicate_field_in_struct_value_expression.zig index eda001c086..e52f835146 100644 --- a/test/cases/compile_errors/duplicate_field_in_struct_value_expression.zig +++ b/test/cases/compile_errors/duplicate_field_in_struct_value_expression.zig @@ -17,5 +17,5 @@ export fn f() void { // backend=stage2 // target=native // -// :11:10: error: duplicate field -// :8:10: note: other field here +// :8:10: error: duplicate field +// :11:10: note: other field here diff --git a/test/cases/compile_errors/duplicate_struct_field.zig b/test/cases/compile_errors/duplicate_struct_field.zig index bb37178a85..61340a423e 100644 --- a/test/cases/compile_errors/duplicate_struct_field.zig +++ b/test/cases/compile_errors/duplicate_struct_field.zig @@ -2,15 +2,32 @@ const Foo = struct { Bar: i32, Bar: usize, }; -export fn entry() void { - const a: Foo = undefined; - _ = a; + +const S = struct { + a: u32, + b: u32, + a: u32, + a: u64, +}; + +export fn a() void { + const f: Foo = undefined; + _ = f; +} + +export fn b() void { + const s: S = undefined; + _ = s; } // error // backend=stage2 // target=native // -// :3:5: error: duplicate struct field: 'Bar' -// :2:5: note: other field here +// :2:5: error: duplicate struct field: 'Bar' +// :3:5: note: other field here // :1:13: note: struct declared here +// :7:5: error: duplicate struct field: 'a' +// :9:5: note: other field here +// :10:5: note: other field here +// :6:11: note: struct declared here diff --git a/test/cases/compile_errors/error_in_struct_initializer_doesnt_crash_the_compiler.zig b/test/cases/compile_errors/error_in_struct_initializer_doesnt_crash_the_compiler.zig index 0c6c1cfb35..c0b1193fb1 100644 --- a/test/cases/compile_errors/error_in_struct_initializer_doesnt_crash_the_compiler.zig +++ b/test/cases/compile_errors/error_in_struct_initializer_doesnt_crash_the_compiler.zig @@ -11,6 +11,6 @@ pub export fn entry() void { // backend=stage2 // target=native // -// :4:9: error: duplicate struct field: 'e' -// :3:9: note: other field here +// :3:9: error: duplicate struct field: 'e' +// :4:9: note: other field here // :2:22: note: struct declared here diff --git a/test/cases/compile_errors/struct_duplicate_field_name.zig b/test/cases/compile_errors/struct_duplicate_field_name.zig index 7e4434f1d9..d19e94b9d2 100644 --- a/test/cases/compile_errors/struct_duplicate_field_name.zig +++ b/test/cases/compile_errors/struct_duplicate_field_name.zig @@ -11,6 +11,6 @@ export fn entry() void { // error // target=native // -// :3:5: error: duplicate struct field: 'foo' -// :2:5: note: other field here +// :2:5: error: duplicate struct field: 'foo' +// :3:5: note: other field here // :1:11: note: struct declared here diff --git a/test/cbe.zig b/test/cbe.zig index 7eb212b2fe..bee7138438 100644 --- a/test/cbe.zig +++ b/test/cbe.zig @@ -507,8 +507,8 @@ pub fn addCases(ctx: *Cases) !void { \\ return p.y - p.x - p.x; \\} , &.{ - ":6:10: error: duplicate field", - ":4:10: note: other field here", + ":4:10: error: duplicate field", + ":6:10: note: other field here", }); case.addError( \\const Point = struct { x: i32, y: i32 };