From aa78ebaf95af5a3587194d8dbcb101a49c0bb898 Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Tue, 2 Aug 2022 19:54:54 +0300 Subject: [PATCH] Sema: improve circular dependency errors --- src/Sema.zig | 109 +++++++++++++----- .../compile_errors/direct_struct_loop.zig | 9 ++ .../compile_errors/indirect_struct_loop.zig | 13 +++ ...an_invalid_struct_that_contains_itself.zig | 5 +- ..._an_invalid_union_that_contains_itself.zig | 16 +++ .../stage1/obj/direct_struct_loop.zig | 8 -- .../stage1/obj/indirect_struct_loop.zig | 10 -- ...t_depends_on_itself_via_optional_field.zig | 8 +- 8 files changed, 128 insertions(+), 50 deletions(-) create mode 100644 test/cases/compile_errors/direct_struct_loop.zig create mode 100644 test/cases/compile_errors/indirect_struct_loop.zig rename test/cases/compile_errors/{stage1/obj => }/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig (58%) create mode 100644 test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_union_that_contains_itself.zig delete mode 100644 test/cases/compile_errors/stage1/obj/direct_struct_loop.zig delete mode 100644 test/cases/compile_errors/stage1/obj/indirect_struct_loop.zig rename test/cases/compile_errors/{stage1/obj => }/struct_depends_on_itself_via_optional_field.zig (61%) diff --git a/src/Sema.zig b/src/Sema.zig index a4d815ea3c..6b376f3d6c 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -26629,21 +26629,41 @@ fn resolveStructLayout( switch (struct_obj.status) { .none, .have_field_types => {}, .field_types_wip, .layout_wip => { - return sema.fail(block, src, "struct '{}' depends on itself", .{ty.fmt(sema.mod)}); + const msg = try Module.ErrorMsg.create( + sema.gpa, + struct_obj.srcLoc(sema.mod), + "struct '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + return sema.failWithOwnedErrorMsg(msg); }, .have_layout, .fully_resolved_wip, .fully_resolved => return, } struct_obj.status = .layout_wip; - for (struct_obj.fields.values()) |field| { - try sema.resolveTypeLayout(block, src, field.ty); + for (struct_obj.fields.values()) |field, i| { + sema.resolveTypeLayout(block, src, field.ty) catch |err| switch (err) { + error.AnalysisFail => { + const msg = sema.err orelse return err; + try sema.addFieldErrNote(block, ty, i, msg, "while checking this field", .{}); + return err; + }, + else => return err, + }; } struct_obj.status = .have_layout; // In case of querying the ABI alignment of this struct, we will ask // for hasRuntimeBits() of each field, so we need "requires comptime" // to be known already before this function returns. - for (struct_obj.fields.values()) |field| { - _ = try sema.typeRequiresComptime(block, src, field.ty); + for (struct_obj.fields.values()) |field, i| { + _ = sema.typeRequiresComptime(block, src, field.ty) catch |err| switch (err) { + error.AnalysisFail => { + const msg = sema.err orelse return err; + try sema.addFieldErrNote(block, ty, i, msg, "while checking this field", .{}); + return err; + }, + else => return err, + }; } } // otherwise it's a tuple; no need to resolve anything @@ -26660,13 +26680,26 @@ fn resolveUnionLayout( switch (union_obj.status) { .none, .have_field_types => {}, .field_types_wip, .layout_wip => { - return sema.fail(block, src, "union '{}' depends on itself", .{ty.fmt(sema.mod)}); + const msg = try Module.ErrorMsg.create( + sema.gpa, + union_obj.srcLoc(sema.mod), + "union '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + return sema.failWithOwnedErrorMsg(msg); }, .have_layout, .fully_resolved_wip, .fully_resolved => return, } union_obj.status = .layout_wip; - for (union_obj.fields.values()) |field| { - try sema.resolveTypeLayout(block, src, field.ty); + for (union_obj.fields.values()) |field, i| { + sema.resolveTypeLayout(block, src, field.ty) catch |err| switch (err) { + error.AnalysisFail => { + const msg = sema.err orelse return err; + try sema.addFieldErrNote(block, ty, i, msg, "while checking this field", .{}); + return err; + }, + else => return err, + }; } union_obj.status = .have_layout; } @@ -26794,12 +26827,12 @@ pub fn resolveTypeFields(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) switch (ty.tag()) { .@"struct" => { const struct_obj = ty.castTag(.@"struct").?.data; - try sema.resolveTypeFieldsStruct(block, src, ty, struct_obj); + try sema.resolveTypeFieldsStruct(ty, struct_obj); return ty; }, .@"union", .union_safety_tagged, .union_tagged => { const union_obj = ty.cast(Type.Payload.Union).?.data; - try sema.resolveTypeFieldsUnion(block, src, ty, union_obj); + try sema.resolveTypeFieldsUnion(ty, union_obj); return ty; }, .type_info => return sema.resolveBuiltinTypeFields(block, src, "Type"), @@ -26820,15 +26853,19 @@ pub fn resolveTypeFields(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) fn resolveTypeFieldsStruct( sema: *Sema, - block: *Block, - src: LazySrcLoc, ty: Type, struct_obj: *Module.Struct, ) CompileError!void { switch (struct_obj.status) { .none => {}, .field_types_wip => { - return sema.fail(block, src, "struct '{}' depends on itself", .{ty.fmt(sema.mod)}); + const msg = try Module.ErrorMsg.create( + sema.gpa, + struct_obj.srcLoc(sema.mod), + "struct '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + return sema.failWithOwnedErrorMsg(msg); }, .have_field_types, .have_layout, @@ -26842,17 +26879,17 @@ fn resolveTypeFieldsStruct( try semaStructFields(sema.mod, struct_obj); } -fn resolveTypeFieldsUnion( - sema: *Sema, - block: *Block, - src: LazySrcLoc, - ty: Type, - union_obj: *Module.Union, -) CompileError!void { +fn resolveTypeFieldsUnion(sema: *Sema, ty: Type, union_obj: *Module.Union) CompileError!void { switch (union_obj.status) { .none => {}, .field_types_wip => { - return sema.fail(block, src, "union '{}' depends on itself", .{ty.fmt(sema.mod)}); + const msg = try Module.ErrorMsg.create( + sema.gpa, + union_obj.srcLoc(sema.mod), + "union '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + return sema.failWithOwnedErrorMsg(msg); }, .have_field_types, .have_layout, @@ -27786,9 +27823,19 @@ pub fn typeHasOnePossibleValue( .@"struct" => { const resolved_ty = try sema.resolveTypeFields(block, src, ty); const s = resolved_ty.castTag(.@"struct").?.data; - for (s.fields.values()) |value| { - if (value.is_comptime) continue; - if ((try sema.typeHasOnePossibleValue(block, src, value.ty)) == null) { + for (s.fields.values()) |field, i| { + if (field.is_comptime) continue; + if (field.ty.eql(resolved_ty, sema.mod)) { + const msg = try Module.ErrorMsg.create( + sema.gpa, + s.srcLoc(sema.mod), + "struct '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + try sema.addFieldErrNote(block, resolved_ty, i, msg, "while checking this field", .{}); + return sema.failWithOwnedErrorMsg(msg); + } + if ((try sema.typeHasOnePossibleValue(block, src, field.ty)) == null) { return null; } } @@ -27854,6 +27901,16 @@ pub fn typeHasOnePossibleValue( const tag_val = (try sema.typeHasOnePossibleValue(block, src, union_obj.tag_ty)) orelse return null; const only_field = union_obj.fields.values()[0]; + if (only_field.ty.eql(resolved_ty, sema.mod)) { + const msg = try Module.ErrorMsg.create( + sema.gpa, + union_obj.srcLoc(sema.mod), + "union '{}' depends on itself", + .{ty.fmt(sema.mod)}, + ); + try sema.addFieldErrNote(block, resolved_ty, 0, msg, "while checking this field", .{}); + return sema.failWithOwnedErrorMsg(msg); + } const val_val = (try sema.typeHasOnePossibleValue(block, src, only_field.ty)) orelse return null; // TODO make this not allocate. The function in `Type.onePossibleValue` @@ -28493,7 +28550,7 @@ pub fn typeRequiresComptime(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ if (struct_obj.status == .field_types_wip) return false; - try sema.resolveTypeFieldsStruct(block, src, ty, struct_obj); + try sema.resolveTypeFieldsStruct(ty, struct_obj); struct_obj.requires_comptime = .wip; for (struct_obj.fields.values()) |field| { @@ -28518,7 +28575,7 @@ pub fn typeRequiresComptime(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ if (union_obj.status == .field_types_wip) return false; - try sema.resolveTypeFieldsUnion(block, src, ty, union_obj); + try sema.resolveTypeFieldsUnion(ty, union_obj); union_obj.requires_comptime = .wip; for (union_obj.fields.values()) |field| { diff --git a/test/cases/compile_errors/direct_struct_loop.zig b/test/cases/compile_errors/direct_struct_loop.zig new file mode 100644 index 0000000000..0abc1a4f73 --- /dev/null +++ b/test/cases/compile_errors/direct_struct_loop.zig @@ -0,0 +1,9 @@ +const A = struct { a : A, }; +export fn entry() usize { return @sizeOf(A); } + +// error +// backend=stage2 +// target=native +// +// :1:11: error: struct 'tmp.A' depends on itself +// :1:20: note: while checking this field diff --git a/test/cases/compile_errors/indirect_struct_loop.zig b/test/cases/compile_errors/indirect_struct_loop.zig new file mode 100644 index 0000000000..dca2b9c3f6 --- /dev/null +++ b/test/cases/compile_errors/indirect_struct_loop.zig @@ -0,0 +1,13 @@ +const A = struct { b : B, }; +const B = struct { c : C, }; +const C = struct { a : A, }; +export fn entry() usize { return @sizeOf(A); } + +// error +// backend=stage2 +// target=native +// +// :1:11: error: struct 'tmp.A' depends on itself +// :3:20: note: while checking this field +// :2:20: note: while checking this field +// :1:20: note: while checking this field diff --git a/test/cases/compile_errors/stage1/obj/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig b/test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig similarity index 58% rename from test/cases/compile_errors/stage1/obj/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig rename to test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig index dd6909b1c2..74cafabe7c 100644 --- a/test/cases/compile_errors/stage1/obj/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig +++ b/test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_struct_that_contains_itself.zig @@ -9,7 +9,8 @@ export fn entry() usize { } // error -// backend=stage1 +// backend=stage2 // target=native // -// tmp.zig:1:13: error: struct 'Foo' depends on itself +// :1:13: error: struct 'tmp.Foo' depends on itself +// :2:5: note: while checking this field diff --git a/test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_union_that_contains_itself.zig b/test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_union_that_contains_itself.zig new file mode 100644 index 0000000000..6030ca4d3e --- /dev/null +++ b/test/cases/compile_errors/instantiating_an_undefined_value_for_an_invalid_union_that_contains_itself.zig @@ -0,0 +1,16 @@ +const Foo = union { + x: Foo, +}; + +var foo: Foo = undefined; + +export fn entry() usize { + return @sizeOf(@TypeOf(foo.x)); +} + +// error +// backend=stage2 +// target=native +// +// :1:13: error: union 'tmp.Foo' depends on itself +// :2:5: note: while checking this field diff --git a/test/cases/compile_errors/stage1/obj/direct_struct_loop.zig b/test/cases/compile_errors/stage1/obj/direct_struct_loop.zig deleted file mode 100644 index 3062e617d6..0000000000 --- a/test/cases/compile_errors/stage1/obj/direct_struct_loop.zig +++ /dev/null @@ -1,8 +0,0 @@ -const A = struct { a : A, }; -export fn entry() usize { return @sizeOf(A); } - -// error -// backend=stage1 -// target=native -// -// tmp.zig:1:11: error: struct 'A' depends on itself diff --git a/test/cases/compile_errors/stage1/obj/indirect_struct_loop.zig b/test/cases/compile_errors/stage1/obj/indirect_struct_loop.zig deleted file mode 100644 index 12214923d0..0000000000 --- a/test/cases/compile_errors/stage1/obj/indirect_struct_loop.zig +++ /dev/null @@ -1,10 +0,0 @@ -const A = struct { b : B, }; -const B = struct { c : C, }; -const C = struct { a : A, }; -export fn entry() usize { return @sizeOf(A); } - -// error -// backend=stage1 -// target=native -// -// tmp.zig:1:11: error: struct 'A' depends on itself diff --git a/test/cases/compile_errors/stage1/obj/struct_depends_on_itself_via_optional_field.zig b/test/cases/compile_errors/struct_depends_on_itself_via_optional_field.zig similarity index 61% rename from test/cases/compile_errors/stage1/obj/struct_depends_on_itself_via_optional_field.zig rename to test/cases/compile_errors/struct_depends_on_itself_via_optional_field.zig index 46086172f7..cad779e3d7 100644 --- a/test/cases/compile_errors/stage1/obj/struct_depends_on_itself_via_optional_field.zig +++ b/test/cases/compile_errors/struct_depends_on_itself_via_optional_field.zig @@ -11,9 +11,9 @@ export fn entry() void { } // error -// backend=stage1 +// backend=stage2 // target=native // -// tmp.zig:1:17: error: struct 'LhsExpr' depends on itself -// tmp.zig:5:5: note: while checking this field -// tmp.zig:2:5: note: while checking this field +// :1:17: error: struct 'tmp.LhsExpr' depends on itself +// :5:5: note: while checking this field +// :2:5: note: while checking this field