Skip to content

stage2: resolve panic on array-like tuple initialization #11185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/AstGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 8 additions & 5 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage of OPV seems like it might be incorrect for tuples, since they can have OPV without that being shared by all their elements.

I kept it all the same for now, since this change is just to make sure that the calculation of first_block_index is not skipped.

I think we can tackle the array/tuple init differences in another PR, along with #11182

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]) {
Expand Down Expand Up @@ -20860,6 +20861,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;
}
Expand Down Expand Up @@ -21532,6 +21534,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;
Expand Down
7 changes: 4 additions & 3 deletions src/codegen/llvm.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/type.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions test/behavior.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
23 changes: 23 additions & 0 deletions test/behavior/bugs/11159.zig
Original file line number Diff line number Diff line change
@@ -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;
}
16 changes: 16 additions & 0 deletions test/behavior/bugs/11162.zig
Original file line number Diff line number Diff line change
@@ -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);
}
10 changes: 10 additions & 0 deletions test/behavior/bugs/11182.zig
Original file line number Diff line number Diff line change
@@ -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;
}