From 762c4a876bfa999d5f49edc7bbaf2917c06d320d Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Tue, 15 Mar 2022 13:53:17 -0700 Subject: [PATCH 1/2] stage2: comptime fields should not affect opv/comptime-only --- src/AstGen.zig | 12 +++++++----- src/Sema.zig | 2 ++ src/type.zig | 1 + 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index c3e94ef631fbe1cd55c6dd1496a9c1cf01210cc5..60a7f54e54ef8646d1611e784462336e4ba29e81 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -4092,15 +4092,17 @@ fn structDeclInner( const doc_comment_index = try astgen.docCommentAsString(member.firstToken()); wip_members.appendToField(doc_comment_index); - known_non_opv = known_non_opv or - nodeImpliesMoreThanOnePossibleValue(tree, member.ast.type_expr); - known_comptime_only = known_comptime_only or - nodeImpliesComptimeOnly(tree, member.ast.type_expr); - const have_align = member.ast.align_expr != 0; const have_value = member.ast.value_expr != 0; const is_comptime = member.comptime_token != null; const unused = false; + + if (!is_comptime) { + known_non_opv = known_non_opv or + nodeImpliesMoreThanOnePossibleValue(tree, member.ast.type_expr); + known_comptime_only = known_comptime_only or + nodeImpliesComptimeOnly(tree, member.ast.type_expr); + } wip_members.nextField(bits_per_field, .{ have_align, have_value, is_comptime, unused }); if (have_align) { diff --git a/src/Sema.zig b/src/Sema.zig index 8c81f96edf1f016bc7a464b516ee18090c68ac7d..55d33d47e112501c215b1a9f3d2d16611bfa6e00 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -20860,6 +20860,7 @@ pub fn typeHasOnePossibleValue( 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) { return null; } @@ -21532,6 +21533,7 @@ fn typeRequiresComptime(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) C struct_obj.requires_comptime = .wip; for (struct_obj.fields.values()) |field| { + if (field.is_comptime) continue; if (try sema.typeRequiresComptime(block, src, field.ty)) { struct_obj.requires_comptime = .yes; return true; diff --git a/src/type.zig b/src/type.zig index 2df7cc83d84566cd11aad6a70b1d84960ce061a1..ffa43d32c0aa181a3b449e0af79f909351e21c2f 100644 --- a/src/type.zig +++ b/src/type.zig @@ -2098,6 +2098,7 @@ pub const Type = extern union { } assert(struct_obj.haveFieldTypes()); for (struct_obj.fields.values()) |value| { + if (value.is_comptime) continue; if (value.ty.hasRuntimeBitsAdvanced(ignore_comptime_only)) return true; } else { -- 2.54.0 From 480e7eec65c02952b71ecc2f4ff2adccb6092a5f Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Tue, 15 Mar 2022 14:00:33 -0700 Subject: [PATCH 2/2] stage2: Fix panic on initializing comptime fields in tuple This resolves https://github.com/ziglang/zig/issues/11159 The problem was that: 1. We were not correctly deleting the field stores after recognizing that an array initializer was a comptime-known value. 2. LLVM was not checking that the final type had no runtime bits, and so would generate an invalid store. This also adds several test cases for related bugs, just to check these in for later work. --- src/Sema.zig | 11 ++++++----- src/codegen/llvm.zig | 7 ++++--- test/behavior.zig | 3 +++ test/behavior/bugs/11159.zig | 23 +++++++++++++++++++++++ test/behavior/bugs/11162.zig | 16 ++++++++++++++++ test/behavior/bugs/11182.zig | 10 ++++++++++ 6 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 test/behavior/bugs/11159.zig create mode 100644 test/behavior/bugs/11162.zig create mode 100644 test/behavior/bugs/11182.zig diff --git a/src/Sema.zig b/src/Sema.zig index 55d33d47e112501c215b1a9f3d2d16611bfa6e00..2ee64ca73de2c1995a78229624f21dae42c9f7bf 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -3203,11 +3203,6 @@ fn zirValidateArrayInit( // Determine whether the value stored to this pointer is comptime-known. - if (opt_opv) |opv| { - element_vals[i] = opv; - continue; - } - const elem_ptr_air_ref = sema.inst_map.get(elem_ptr).?; const elem_ptr_air_inst = Air.refToIndex(elem_ptr_air_ref).?; // Find the block index of the elem_ptr so that we can look at the next @@ -3223,6 +3218,12 @@ fn zirValidateArrayInit( break :inst block.instructions.items[block_index + 1]; }; + // Array has one possible value, so value is always comptime-known + if (opt_opv) |opv| { + element_vals[i] = opv; + continue; + } + // If the next instructon is a store with a comptime operand, this element // is comptime. switch (air_tags[next_air_inst]) { diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 692edd48c15d771a9f0e7f8df8140ec43e819495..343e503aea837ad287d505785ec441d16a371793 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -5422,21 +5422,22 @@ pub const FuncGen = struct { const bin_op = self.air.instructions.items(.data)[inst].bin_op; const dest_ptr = try self.resolveInst(bin_op.lhs); const ptr_ty = self.air.typeOf(bin_op.lhs); + const operand_ty = ptr_ty.childType(); + if (!operand_ty.isFnOrHasRuntimeBitsIgnoreComptime()) return null; // TODO Sema should emit a different instruction when the store should // possibly do the safety 0xaa bytes for undefined. const val_is_undef = if (self.air.value(bin_op.rhs)) |val| val.isUndefDeep() else false; if (val_is_undef) { - const elem_ty = ptr_ty.childType(); const target = self.dg.module.getTarget(); - const elem_size = elem_ty.abiSize(target); + const operand_size = operand_ty.abiSize(target); const u8_llvm_ty = self.context.intType(8); const ptr_u8_llvm_ty = u8_llvm_ty.pointerType(0); const dest_ptr_u8 = self.builder.buildBitCast(dest_ptr, ptr_u8_llvm_ty, ""); const fill_char = u8_llvm_ty.constInt(0xaa, .False); const dest_ptr_align = ptr_ty.ptrAlignment(target); const usize_llvm_ty = try self.dg.llvmType(Type.usize); - const len = usize_llvm_ty.constInt(elem_size, .False); + const len = usize_llvm_ty.constInt(operand_size, .False); _ = self.builder.buildMemSet(dest_ptr_u8, fill_char, len, dest_ptr_align, ptr_ty.isVolatilePtr()); if (self.dg.module.comp.bin_file.options.valgrind) { // TODO generate valgrind client request to mark byte range as undefined diff --git a/test/behavior.zig b/test/behavior.zig index edc9953e59b08f074f0b3bf04769a3c51b5a8df1..f142464dc3ff3cf49f5f09a1c0641ddcf8a30331 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -65,8 +65,11 @@ test { _ = @import("behavior/bugs/10970.zig"); _ = @import("behavior/bugs/11046.zig"); _ = @import("behavior/bugs/11139.zig"); + _ = @import("behavior/bugs/11159.zig"); + _ = @import("behavior/bugs/11162.zig"); _ = @import("behavior/bugs/11165.zig"); _ = @import("behavior/bugs/11181.zig"); + _ = @import("behavior/bugs/11182.zig"); _ = @import("behavior/call.zig"); _ = @import("behavior/cast.zig"); _ = @import("behavior/comptime_memory.zig"); diff --git a/test/behavior/bugs/11159.zig b/test/behavior/bugs/11159.zig new file mode 100644 index 0000000000000000000000000000000000000000..fdd380705923821eb4ae54311d77dd166b648129 --- /dev/null +++ b/test/behavior/bugs/11159.zig @@ -0,0 +1,23 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +test { + const T = @TypeOf(.{ @as(i32, 0), @as(u32, 0) }); + var a: T = .{ 0, 0 }; + _ = a; +} + +test { + if (builtin.zig_backend == .stage1) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + + const S = struct { + comptime x: i32 = 0, + comptime y: u32 = 0, + }; + var a: S = .{}; + _ = a; + var b = S{}; + _ = b; +} diff --git a/test/behavior/bugs/11162.zig b/test/behavior/bugs/11162.zig new file mode 100644 index 0000000000000000000000000000000000000000..ced435a30573ee6a996dcff7a758f6ee4c84efee --- /dev/null +++ b/test/behavior/bugs/11162.zig @@ -0,0 +1,16 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const expect = std.testing.expect; + +test { + if (builtin.zig_backend == .stage1) return error.SkipZigTest; // TODO + if (builtin.zig_backend != .stage1) return error.SkipZigTest; // TODO + + var x: u32 = 15; + const T = @TypeOf(.{ @as(i32, -1234), @as(u32, 5678), x }); + var a: T = .{ -1234, 5678, x + 1 }; + + try expect(a[0] == -1234); + try expect(a[1] == 5678); + try expect(a[2] == 16); +} diff --git a/test/behavior/bugs/11182.zig b/test/behavior/bugs/11182.zig new file mode 100644 index 0000000000000000000000000000000000000000..eb4ee8ac9d9d9fa6fd6716d91d41ebc9ea263e98 --- /dev/null +++ b/test/behavior/bugs/11182.zig @@ -0,0 +1,10 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +test { + if (builtin.zig_backend != .stage1) return error.SkipZigTest; // TODO + + const T = @TypeOf(.{ @as(i32, 0), @as(u32, 0) }); + var a = T{ 0, 0 }; + _ = a; +} -- 2.54.0