Merge pull request #11242 from schmee/sema-handle-more-union-errors

stage2: add more union compile errors / improve error messages

src/Sema.zig

@@ -1589,6 +1589,21 @@ fn errNote(
     return sema.mod.errNoteNonLazy(src.toSrcLoc(block.src_decl), parent, format, args);
 }
 
+fn addFieldErrNote(
+    sema: *Sema,
+    block: *Block,
+    container_ty: Type,
+    field_index: usize,
+    parent: *Module.ErrorMsg,
+    comptime format: []const u8,
+    args: anytype,
+) !void {
+    const decl = container_ty.getOwnerDecl();
+    const tree = try sema.getAstTree(block);
+    const field_src = enumFieldSrcLoc(decl, tree.*, container_ty.getNodeOffset(), field_index);
+    try sema.mod.errNoteNonLazy(field_src.toSrcLoc(decl), parent, format, args);
+}
+
 fn errMsg(
     sema: *Sema,
     block: *Block,
@@ -17573,9 +17588,15 @@ fn unionFieldVal(
                 if (tag_matches) {
                     return sema.addConstant(field.ty, tag_and_val.val);
                 } else {
-                    // TODO enhance this saying which one was active
-                    // and which one was accessed, and showing where the union was declared.
-                    return sema.fail(block, src, "access of inactive union field", .{});
+                    const msg = msg: {
+                        const active_index = tag_and_val.tag.castTag(.enum_field_index).?.data;
+                        const active_field_name = union_obj.fields.keys()[active_index];
+                        const msg = try sema.errMsg(block, src, "access of union field '{s}' while field '{s}' is active", .{ field_name, active_field_name });
+                        errdefer msg.destroy(sema.gpa);
+                        try sema.addDeclaredHereNote(msg, union_ty);
+                        break :msg msg;
+                    };
+                    return sema.failWithOwnedErrorMsg(block, msg);
                 }
             },
             .Packed, .Extern => {
@@ -19702,13 +19723,14 @@ fn coerceEnumToUnion(
         const field = union_obj.fields.values()[field_index];
         const field_ty = try sema.resolveTypeFields(block, inst_src, field.ty);
         const opv = (try sema.typeHasOnePossibleValue(block, inst_src, field_ty)) orelse {
-            // TODO resolve the field names and include in the error message,
-            // also instead of 'union declared here' make it 'field "foo" declared here'.
             const msg = msg: {
-                const msg = try sema.errMsg(block, inst_src, "coercion to union {} must initialize {} field", .{
-                    union_ty.fmt(target), field_ty.fmt(target),
+                const field_name = union_obj.fields.keys()[field_index];
+                const msg = try sema.errMsg(block, inst_src, "coercion from enum '{}' to union '{}' must initialize '{}' field '{s}'", .{
+                    inst_ty.fmt(target), union_ty.fmt(target), field_ty.fmt(target), field_name,
                 });
                 errdefer msg.destroy(sema.gpa);
+
+                try sema.addFieldErrNote(block, union_ty, field_index, msg, "field '{s}' declared here", .{field_name});
                 try sema.addDeclaredHereNote(msg, union_ty);
                 break :msg msg;
             };
@@ -19740,13 +19762,24 @@ fn coerceEnumToUnion(
         return block.addBitCast(union_ty, enum_tag);
     }
 
-    // TODO resolve the field names and add a hint that says "field 'foo' has type 'bar'"
-    // instead of the "union declared here" hint
     const msg = msg: {
-        const msg = try sema.errMsg(block, inst_src, "runtime coercion to union {} which has non-void fields", .{
-            union_ty.fmt(target),
-        });
+        const union_obj = union_ty.cast(Type.Payload.Union).?.data;
+        const msg = try sema.errMsg(
+            block,
+            inst_src,
+            "runtime coercion from enum '{}' to union '{}' which has non-void fields",
+            .{ tag_ty.fmt(target), union_ty.fmt(target) },
+        );
         errdefer msg.destroy(sema.gpa);
+
+        var it = union_obj.fields.iterator();
+        var field_index: usize = 0;
+        while (it.next()) |field| {
+            const field_name = field.key_ptr.*;
+            const field_ty = field.value_ptr.ty;
+            try sema.addFieldErrNote(block, union_ty, field_index, msg, "field '{s}' has type '{}'", .{ field_name, field_ty.fmt(target) });
+            field_index += 1;
+        }
         try sema.addDeclaredHereNote(msg, union_ty);
         break :msg msg;
     };
@@ -21835,7 +21868,7 @@ fn resolveTypeFieldsUnion(
     }
 
     union_obj.status = .field_types_wip;
-    try semaUnionFields(sema.mod, union_obj);
+    try semaUnionFields(block, sema.mod, union_obj);
     union_obj.status = .have_field_types;
 }
 
@@ -22044,7 +22077,21 @@ fn semaStructFields(
         }
 
         const gop = struct_obj.fields.getOrPutAssumeCapacity(field_name);
-        assert(!gop.found_existing);
+        if (gop.found_existing) {
+            const msg = msg: {
+                const tree = try sema.getAstTree(&block_scope);
+                const field_src = enumFieldSrcLoc(decl, tree.*, struct_obj.node_offset, field_i);
+                const msg = try sema.errMsg(&block_scope, field_src, "duplicate struct field: '{s}'", .{field_name});
+                errdefer msg.destroy(gpa);
+
+                const prev_field_index = struct_obj.fields.getIndex(field_name).?;
+                const prev_field_src = enumFieldSrcLoc(decl, tree.*, struct_obj.node_offset, prev_field_index);
+                try sema.mod.errNoteNonLazy(prev_field_src.toSrcLoc(decl), msg, "other field here", .{});
+                try sema.errNote(&block_scope, src, msg, "struct declared here", .{});
+                break :msg msg;
+            };
+            return sema.failWithOwnedErrorMsg(&block_scope, msg);
+        }
         gop.value_ptr.* = .{
             .ty = try field_ty.copy(decl_arena_allocator),
             .abi_align = 0,
@@ -22075,7 +22122,7 @@ fn semaStructFields(
     }
 }
 
-fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
+fn semaUnionFields(block: *Block, mod: *Module, union_obj: *Module.Union) CompileError!void {
     const tracy = trace(@src());
     defer tracy.end();
 
@@ -22175,6 +22222,7 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
     var int_tag_ty: Type = undefined;
     var enum_field_names: ?*Module.EnumNumbered.NameMap = null;
     var enum_value_map: ?*Module.EnumNumbered.ValueMap = null;
+    var tag_ty_field_names: ?Module.EnumFull.NameMap = null;
     if (tag_type_ref != .none) {
         const provided_ty = try sema.resolveType(&block_scope, src, tag_type_ref);
         if (small.auto_enum_tag) {
@@ -22187,6 +22235,10 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
         } else {
             // The provided type is the enum tag type.
             union_obj.tag_ty = try provided_ty.copy(decl_arena_allocator);
+            // The fields of the union must match the enum exactly.
+            // Store a copy of the enum field names so we can check for
+            // missing or extraneous fields later.
+            tag_ty_field_names = try union_obj.tag_ty.enumFields().clone(sema.arena);
         }
     } else {
         // If auto_enum_tag is false, this is an untagged union. However, for semantic analysis
@@ -22295,7 +22347,35 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
         }
 
         const gop = union_obj.fields.getOrPutAssumeCapacity(field_name);
-        assert(!gop.found_existing);
+        if (gop.found_existing) {
+            const msg = msg: {
+                const tree = try sema.getAstTree(&block_scope);
+                const field_src = enumFieldSrcLoc(decl, tree.*, union_obj.node_offset, field_i);
+                const msg = try sema.errMsg(&block_scope, field_src, "duplicate union field: '{s}'", .{field_name});
+                errdefer msg.destroy(gpa);
+
+                const prev_field_index = union_obj.fields.getIndex(field_name).?;
+                const prev_field_src = enumFieldSrcLoc(decl, tree.*, union_obj.node_offset, prev_field_index);
+                try sema.mod.errNoteNonLazy(prev_field_src.toSrcLoc(decl), msg, "other field here", .{});
+                try sema.errNote(&block_scope, src, msg, "union declared here", .{});
+                break :msg msg;
+            };
+            return sema.failWithOwnedErrorMsg(&block_scope, msg);
+        }
+
+        if (tag_ty_field_names) |*names| {
+            const enum_has_field = names.orderedRemove(field_name);
+            if (!enum_has_field) {
+                const msg = msg: {
+                    const msg = try sema.errMsg(block, src, "enum '{}' has no field named '{s}'", .{ union_obj.tag_ty.fmt(target), field_name });
+                    errdefer msg.destroy(sema.gpa);
+                    try sema.addDeclaredHereNote(msg, union_obj.tag_ty);
+                    break :msg msg;
+                };
+                return sema.failWithOwnedErrorMsg(block, msg);
+            }
+        }
+
         gop.value_ptr.* = .{
             .ty = try field_ty.copy(decl_arena_allocator),
             .abi_align = 0,
@@ -22310,6 +22390,24 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
             gop.value_ptr.abi_align = 0;
         }
     }
+
+    if (tag_ty_field_names) |names| {
+        if (names.count() > 0) {
+            const msg = msg: {
+                const msg = try sema.errMsg(block, src, "enum field(s) missing in union", .{});
+                errdefer msg.destroy(sema.gpa);
+
+                const enum_ty = union_obj.tag_ty;
+                for (names.keys()) |field_name| {
+                    const field_index = enum_ty.enumFieldIndex(field_name).?;
+                    try sema.addFieldErrNote(block, enum_ty, field_index, msg, "field '{s}' missing, declared here", .{field_name});
+                }
+                try sema.addDeclaredHereNote(msg, union_obj.tag_ty);
+                break :msg msg;
+            };
+            return sema.failWithOwnedErrorMsg(block, msg);
+        }
+    }
 }
 
 fn generateUnionTagTypeNumbered(

src/type.zig

@@ -5312,6 +5312,50 @@ pub const Type = extern union {
         }
     }
 
+    pub fn getNodeOffset(ty: Type) i32 {
+        switch (ty.tag()) {
+            .enum_full, .enum_nonexhaustive => {
+                const enum_full = ty.cast(Payload.EnumFull).?.data;
+                return enum_full.node_offset;
+            },
+            .enum_numbered => return ty.castTag(.enum_numbered).?.data.node_offset,
+            .enum_simple => {
+                const enum_simple = ty.castTag(.enum_simple).?.data;
+                return enum_simple.node_offset;
+            },
+            .@"struct" => {
+                const struct_obj = ty.castTag(.@"struct").?.data;
+                return struct_obj.node_offset;
+            },
+            .error_set => {
+                const error_set = ty.castTag(.error_set).?.data;
+                return error_set.node_offset;
+            },
+            .@"union", .union_tagged => {
+                const union_obj = ty.cast(Payload.Union).?.data;
+                return union_obj.node_offset;
+            },
+            .@"opaque" => {
+                const opaque_obj = ty.cast(Payload.Opaque).?.data;
+                return opaque_obj.node_offset;
+            },
+            .atomic_order,
+            .atomic_rmw_op,
+            .calling_convention,
+            .address_space,
+            .float_mode,
+            .reduce_op,
+            .call_options,
+            .prefetch_options,
+            .export_options,
+            .extern_options,
+            .type_info,
+            => unreachable, // These need to be resolved earlier.
+
+            else => unreachable,
+        }
+    }
+
     /// Asserts the type is an enum.
     pub fn enumHasInt(ty: Type, int: Value, target: Target) bool {
         const S = struct {

test/compile_errors/stage2/struct_duplicate_field_name.zig

@@ -0,0 +1,15 @@
+const S = struct {
+    foo: u32,
+    foo: u32,
+};
+
+export fn entry() void {
+    const s: S = .{ .foo = 100 };
+    _ = s;
+}
+
+// duplicate struct field name
+//
+// :3:5: error: duplicate struct field: 'foo'
+// :2:5: note: other field here
+// :1:11: note: struct declared here

test/compile_errors/stage2/union_access_of_inactive_field.zig

@@ -0,0 +1,14 @@
+const U = union {
+    a: void,
+    b: u64,
+};
+comptime {
+    var u: U = .{.a = {}};
+    const v = u.b;
+    _ = v;
+}
+
+// access of inactive union field
+//
+// :7:16: error: access of union field 'b' while field 'a' is active
+// :1:11: note: union declared here

test/compile_errors/stage2/union_duplicate_enum_field.zig

@@ -0,0 +1,16 @@
+const E = enum {a, b};
+const U = union(E) {
+    a: u32,
+    a: u32,
+};
+
+export fn foo() void {
+    var u: U = .{ .a = 123 };
+    _ = u;
+}
+
+// union with enum and duplicate fields
+//
+// :4:5: error: duplicate union field: 'a'
+// :3:5: note: other field here
+// :2:11: note: union declared here

test/compile_errors/stage2/union_duplicate_field_definition.zig

@@ -0,0 +1,15 @@
+const U = union {
+    foo: u32,
+    foo: u32,
+};
+
+export fn entry() void {
+    const u: U = .{ .foo = 100 };
+    _ = u;
+}
+
+// duplicate union field name
+//
+// :3:5: error: duplicate union field: 'foo'
+// :2:5: note: other field here
+// :1:11: note: union declared here

test/compile_errors/stage2/union_enum_field_missing.zig

@@ -0,0 +1,20 @@
+const E = enum {
+    a,
+    b,
+    c,
+};
+
+const U = union(E) {
+    a: i32,
+    b: f64,
+};
+
+export fn entry() usize {
+    return @sizeOf(U);
+}
+
+// enum field missing in union
+//
+// :7:1: error: enum field(s) missing in union
+// :4:5: note: field 'c' missing, declared here
+// :1:11: note: enum declared here

test/compile_errors/stage2/union_extra_field.zig

@@ -0,0 +1,19 @@
+const E = enum {
+    a,
+    b,
+    c,
+};
+const U = union(E) {
+    a: i32,
+    b: f64,
+    c: f64,
+    d: f64,
+};
+export fn entry() usize {
+    return @sizeOf(U);
+}
+
+// union extra field
+//
+// :6:1: error: enum 'tmp.E' has no field named 'd'
+// :1:11: note: enum declared here

test/compile_errors/stage2/union_runtime_coercion_from_enum.zig

@@ -0,0 +1,22 @@
+const E = enum {
+    a,
+    b,
+};
+const U = union(E) {
+    a: u32,
+    b: u64,
+};
+fn foo() E {
+    return E.b;
+}
+export fn doTheTest() u64 {
+    var u: U = foo();
+    return u.b;
+}
+
+// runtime coercion from enum to union
+//
+// :13:19: error: runtime coercion from enum 'tmp.E' to union 'tmp.U' which has non-void fields
+// :6:5: note: field 'a' has type 'u32'
+// :7:5: note: field 'b' has type 'u64'
+// :5:11: note: union declared here