From 794beafb9c2c687d993a0933be258a2ccdf0be4f Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Wed, 20 Jul 2022 12:30:11 +0300 Subject: [PATCH] Sema: validate extern struct field types earlier `validateExternType` does not require the type to be resolved so we can check it earlier. Only doing it in `resolveTypeFully` lead to worse or missing compile errors. --- src/Sema.zig | 108 +++++++++++++----- test/behavior/align.zig | 4 +- test/behavior/bugs/1310.zig | 4 +- ...edding_opaque_type_in_struct_and_union.zig | 38 ++++++ ...mpatible_but_inferred_integer_tag_type.zig | 6 +- ...non-extern-compatible_integer_tag_type.zig | 6 +- ...invalid_optional_type_in_extern_struct.zig | 4 +- ...edding_opaque_type_in_struct_and_union.zig | 29 ----- 8 files changed, 129 insertions(+), 70 deletions(-) create mode 100644 test/cases/compile_errors/directly_embedding_opaque_type_in_struct_and_union.zig delete mode 100644 test/cases/compile_errors/stage1/obj/directly_embedding_opaque_type_in_struct_and_union.zig diff --git a/src/Sema.zig b/src/Sema.zig index 6a64dc78d8..47429b9dbf 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -14559,13 +14559,23 @@ fn zirStructInitAnon( var runtime_src: ?LazySrcLoc = null; var extra_index = extra.end; for (types) |*field_ty, i| { + const init_src = src; // TODO better source location const item = sema.code.extraData(Zir.Inst.StructInitAnon.Item, extra_index); extra_index = item.end; names[i] = sema.code.nullTerminatedString(item.data.field_name); const init = try sema.resolveInst(item.data.init); field_ty.* = sema.typeOf(init); - const init_src = src; // TODO better source location + if (types[i].zigTypeTag() == .Opaque) { + const msg = msg: { + const msg = try sema.errMsg(block, init_src, "opaque types have unknown size and therefore cannot be directly embedded in structs", .{}); + errdefer msg.destroy(sema.gpa); + + try sema.addDeclaredHereNote(msg, types[i]); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(block, msg); + } if (try sema.resolveMaybeUndefVal(block, init_src, init)) |init_val| { values[i] = init_val; } else { @@ -14742,9 +14752,19 @@ fn zirArrayInitAnon( const opt_runtime_src = rs: { var runtime_src: ?LazySrcLoc = null; for (operands) |operand, i| { + const operand_src = src; // TODO better source location const elem = try sema.resolveInst(operand); types[i] = sema.typeOf(elem); - const operand_src = src; // TODO better source location + if (types[i].zigTypeTag() == .Opaque) { + const msg = msg: { + const msg = try sema.errMsg(block, operand_src, "opaque types have unknown size and therefore cannot be directly embedded in structs", .{}); + errdefer msg.destroy(sema.gpa); + + try sema.addDeclaredHereNote(msg, types[i]); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(block, msg); + } if (try sema.resolveMaybeUndefVal(block, operand_src, elem)) |val| { values[i] = val; } else { @@ -18641,6 +18661,8 @@ const ExternPosition = enum { other, }; +/// Returns true if `ty` is allowed in extern types. +/// Does *NOT* require `ty` to be resolved in any way. fn validateExternType(sema: *Sema, ty: Type, position: ExternPosition) CompileError!bool { switch (ty.zigTypeTag()) { .Type, @@ -25214,20 +25236,6 @@ fn resolveStructFully( struct_obj.status = .fully_resolved_wip; for (struct_obj.fields.values()) |field| { try sema.resolveTypeFully(block, src, field.ty); - - if (struct_obj.layout == .Extern and !(try sema.validateExternType(field.ty, .other))) { - const msg = msg: { - const msg = try sema.errMsg(block, src, "extern structs cannot contain fields of type '{}'", .{field.ty.fmt(sema.mod)}); - errdefer msg.destroy(sema.gpa); - - const src_decl = sema.mod.declPtr(block.src_decl); - try sema.explainWhyTypeIsNotExtern(block, src, msg, src.toSrcLoc(src_decl), field.ty, .other); - - try sema.addDeclaredHereNote(msg, field.ty); - break :msg msg; - }; - return sema.failWithOwnedErrorMsg(block, msg); - } } struct_obj.status = .fully_resolved; } @@ -25261,20 +25269,6 @@ fn resolveUnionFully( union_obj.status = .fully_resolved_wip; for (union_obj.fields.values()) |field| { try sema.resolveTypeFully(block, src, field.ty); - - if (union_obj.layout == .Extern and !(try sema.validateExternType(field.ty, .union_field))) { - const msg = msg: { - const msg = try sema.errMsg(block, src, "extern unions cannot contain fields of type '{}'", .{field.ty.fmt(sema.mod)}); - errdefer msg.destroy(sema.gpa); - - const src_decl = sema.mod.declPtr(block.src_decl); - try sema.explainWhyTypeIsNotExtern(block, src, msg, src.toSrcLoc(src_decl), field.ty, .union_field); - - try sema.addDeclaredHereNote(msg, field.ty); - break :msg msg; - }; - return sema.failWithOwnedErrorMsg(block, msg); - } } union_obj.status = .fully_resolved; } @@ -25612,6 +25606,33 @@ fn semaStructFields(mod: *Module, struct_obj: *Module.Struct) CompileError!void const field = &struct_obj.fields.values()[i]; field.ty = try field_ty.copy(decl_arena_allocator); + if (struct_obj.layout == .Extern and !(try sema.validateExternType(field.ty, .other))) { + const msg = msg: { + const tree = try sema.getAstTree(&block_scope); + const fields_src = enumFieldSrcLoc(decl, tree.*, struct_obj.node_offset, i); + const msg = try sema.errMsg(&block_scope, fields_src, "extern structs cannot contain fields of type '{}'", .{field.ty.fmt(sema.mod)}); + errdefer msg.destroy(sema.gpa); + + try sema.explainWhyTypeIsNotExtern(&block_scope, fields_src, msg, fields_src.toSrcLoc(decl), field.ty, .other); + + try sema.addDeclaredHereNote(msg, field.ty); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(&block_scope, msg); + } + if (field_ty.zigTypeTag() == .Opaque) { + const msg = msg: { + const tree = try sema.getAstTree(&block_scope); + const field_src = enumFieldSrcLoc(decl, tree.*, struct_obj.node_offset, i); + const msg = try sema.errMsg(&block_scope, field_src, "opaque types have unknown size and therefore cannot be directly embedded in structs", .{}); + errdefer msg.destroy(sema.gpa); + + try sema.addDeclaredHereNote(msg, field_ty); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(&block_scope, msg); + } + if (zir_field.align_body_len > 0) { const body = zir.extra[extra_index..][0..zir_field.align_body_len]; extra_index += body.len; @@ -25909,6 +25930,33 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void { } } + if (union_obj.layout == .Extern and !(try sema.validateExternType(field_ty, .union_field))) { + 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, "extern unions cannot contain fields of type '{}'", .{field_ty.fmt(sema.mod)}); + errdefer msg.destroy(sema.gpa); + + try sema.explainWhyTypeIsNotExtern(&block_scope, field_src, msg, field_src.toSrcLoc(decl), field_ty, .union_field); + + try sema.addDeclaredHereNote(msg, field_ty); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(&block_scope, msg); + } + if (field_ty.zigTypeTag() == .Opaque) { + 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, "opaque types have unknown size and therefore cannot be directly embedded in unions", .{}); + errdefer msg.destroy(sema.gpa); + + try sema.addDeclaredHereNote(msg, field_ty); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(&block_scope, msg); + } + gop.value_ptr.* = .{ .ty = try field_ty.copy(decl_arena_allocator), .abi_align = 0, diff --git a/test/behavior/align.zig b/test/behavior/align.zig index ad9a6d9616..d09e97c05b 100644 --- a/test/behavior/align.zig +++ b/test/behavior/align.zig @@ -299,7 +299,7 @@ test "implicitly decreasing fn alignment" { try testImplicitlyDecreaseFnAlign(alignedBig, 5678); } -fn testImplicitlyDecreaseFnAlign(ptr: *align(1) const fn () i32, answer: i32) !void { +fn testImplicitlyDecreaseFnAlign(ptr: *const fn () align(1) i32, answer: i32) !void { try expect(ptr() == answer); } @@ -325,7 +325,7 @@ test "@alignCast functions" { fn fnExpectsOnly1(ptr: *const fn () align(1) i32) i32 { return fnExpects4(@alignCast(4, ptr)); } -fn fnExpects4(ptr: *align(4) const fn () i32) i32 { +fn fnExpects4(ptr: *const fn () align(4) i32) i32 { return ptr(); } fn simple4() align(4) i32 { diff --git a/test/behavior/bugs/1310.zig b/test/behavior/bugs/1310.zig index 1f19ec20c2..0f574942d3 100644 --- a/test/behavior/bugs/1310.zig +++ b/test/behavior/bugs/1310.zig @@ -4,7 +4,7 @@ const builtin = @import("builtin"); pub const VM = ?[*]const struct_InvocationTable_; pub const struct_InvocationTable_ = extern struct { - GetVM: ?fn (?[*]VM) callconv(.C) c_int, + GetVM: ?*const fn (?[*]VM) callconv(.C) c_int, }; pub const struct_VM_ = extern struct { @@ -23,5 +23,7 @@ fn agent_callback(_vm: [*]VM, options: [*]u8) callconv(.C) i32 { } test "fixed" { + if (builtin.zig_backend == .stage1) return error.SkipZigTest; + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; try expect(agent_callback(undefined, undefined) == 11); } diff --git a/test/cases/compile_errors/directly_embedding_opaque_type_in_struct_and_union.zig b/test/cases/compile_errors/directly_embedding_opaque_type_in_struct_and_union.zig new file mode 100644 index 0000000000..5cfce8e61e --- /dev/null +++ b/test/cases/compile_errors/directly_embedding_opaque_type_in_struct_and_union.zig @@ -0,0 +1,38 @@ +const O = opaque {}; +const Foo = struct { + o: O, +}; +const Bar = union { + One: i32, + Two: O, +}; +export fn a() void { + var foo: Foo = undefined; + _ = foo; +} +export fn b() void { + var bar: Bar = undefined; + _ = bar; +} +export fn c() void { + const baz = &@as(opaque {}, undefined); + const qux = .{baz.*}; + _ = qux; +} +export fn d() void { + const baz = &@as(opaque {}, undefined); + const qux = .{ .a = baz.* }; + _ = qux; +} + +// error +// backend=stage2 +// target=native +// +// :3:5: error: opaque types have unknown size and therefore cannot be directly embedded in structs +// :1:11: note: opaque declared here +// :7:5: error: opaque types have unknown size and therefore cannot be directly embedded in unions +// :19:18: error: opaque types have unknown size and therefore cannot be directly embedded in structs +// :18:22: note: opaque declared here +// :24:18: error: opaque types have unknown size and therefore cannot be directly embedded in structs +// :23:22: note: opaque declared here diff --git a/test/cases/compile_errors/extern_struct_with_extern-compatible_but_inferred_integer_tag_type.zig b/test/cases/compile_errors/extern_struct_with_extern-compatible_but_inferred_integer_tag_type.zig index 1740574389..d12bea66d7 100644 --- a/test/cases/compile_errors/extern_struct_with_extern-compatible_but_inferred_integer_tag_type.zig +++ b/test/cases/compile_errors/extern_struct_with_extern-compatible_but_inferred_integer_tag_type.zig @@ -39,7 +39,7 @@ export fn entry() void { // backend=stage2 // target=native // -// :33:8: error: extern structs cannot contain fields of type 'tmp.E' -// :33:8: note: enum tag type 'u9' is not extern compatible -// :33:8: note: only integers with power of two bits are extern compatible +// :31:5: error: extern structs cannot contain fields of type 'tmp.E' +// :31:5: note: enum tag type 'u9' is not extern compatible +// :31:5: note: only integers with power of two bits are extern compatible // :1:15: note: enum declared here diff --git a/test/cases/compile_errors/extern_struct_with_non-extern-compatible_integer_tag_type.zig b/test/cases/compile_errors/extern_struct_with_non-extern-compatible_integer_tag_type.zig index 5fb36b707d..61073e9803 100644 --- a/test/cases/compile_errors/extern_struct_with_non-extern-compatible_integer_tag_type.zig +++ b/test/cases/compile_errors/extern_struct_with_non-extern-compatible_integer_tag_type.zig @@ -11,7 +11,7 @@ export fn entry() void { // backend=stage2 // target=native // -// :5:8: error: extern structs cannot contain fields of type 'tmp.E' -// :5:8: note: enum tag type 'u31' is not extern compatible -// :5:8: note: only integers with power of two bits are extern compatible +// :3:5: error: extern structs cannot contain fields of type 'tmp.E' +// :3:5: note: enum tag type 'u31' is not extern compatible +// :3:5: note: only integers with power of two bits are extern compatible // :1:15: note: enum declared here diff --git a/test/cases/compile_errors/invalid_optional_type_in_extern_struct.zig b/test/cases/compile_errors/invalid_optional_type_in_extern_struct.zig index 9a6c77363c..3dc9e64765 100644 --- a/test/cases/compile_errors/invalid_optional_type_in_extern_struct.zig +++ b/test/cases/compile_errors/invalid_optional_type_in_extern_struct.zig @@ -7,5 +7,5 @@ export fn testf(fluff: *stroo) void { _ = fluff; } // backend=stage2 // target=native // -// :4:8: error: extern structs cannot contain fields of type '?[*c]u8' -// :4:8: note: only pointer like optionals are extern compatible +// :2:5: error: extern structs cannot contain fields of type '?[*c]u8' +// :2:5: note: only pointer like optionals are extern compatible diff --git a/test/cases/compile_errors/stage1/obj/directly_embedding_opaque_type_in_struct_and_union.zig b/test/cases/compile_errors/stage1/obj/directly_embedding_opaque_type_in_struct_and_union.zig deleted file mode 100644 index 8b8d84ad2a..0000000000 --- a/test/cases/compile_errors/stage1/obj/directly_embedding_opaque_type_in_struct_and_union.zig +++ /dev/null @@ -1,29 +0,0 @@ -const O = opaque {}; -const Foo = struct { - o: O, -}; -const Bar = union { - One: i32, - Two: O, -}; -export fn a() void { - var foo: Foo = undefined; - _ = foo; -} -export fn b() void { - var bar: Bar = undefined; - _ = bar; -} -export fn c() void { - var baz: *opaque {} = undefined; - const qux = .{baz.*}; - _ = qux; -} - -// error -// backend=stage1 -// target=native -// -// tmp.zig:3:5: error: opaque types have unknown size and therefore cannot be directly embedded in structs -// tmp.zig:7:5: error: opaque types have unknown size and therefore cannot be directly embedded in unions -// tmp.zig:19:22: error: opaque types have unknown size and therefore cannot be directly embedded in structs