From 1479c28b496e7c1db134b51f23dd2eb934b123bb Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Mar 2020 18:42:01 +0100 Subject: [PATCH 1/3] ir: Correct ABI size calculation for arrays Zero-length array with a sentinel may not have zero size. Closes #4749 --- src/analyze.cpp | 17 +++++++---------- src/codegen.cpp | 4 +++- test/stage1/behavior/array.zig | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/analyze.cpp b/src/analyze.cpp index 33d28269b9..d0f8979c79 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -803,13 +803,7 @@ ZigType *get_array_type(CodeGen *g, ZigType *child_type, uint64_t array_size, Zi } buf_appendf(&entry->name, "]%s", buf_ptr(&child_type->name)); - size_t full_array_size; - if (array_size == 0) { - full_array_size = 0; - } else { - full_array_size = array_size + ((sentinel != nullptr) ? 1 : 0); - } - + size_t full_array_size = array_size + ((sentinel != nullptr) ? 1 : 0); entry->size_in_bits = child_type->size_in_bits * full_array_size; entry->abi_align = child_type->abi_align; entry->abi_size = child_type->abi_size * full_array_size; @@ -1197,7 +1191,8 @@ Error type_val_resolve_zero_bits(CodeGen *g, ZigValue *type_val, ZigType *parent LazyValueArrayType *lazy_array_type = reinterpret_cast(type_val->data.x_lazy); - if (lazy_array_type->length < 1) { + // The sentinel counts as an extra element + if (lazy_array_type->length == 0 && lazy_array_type->sentinel == nullptr) { *is_zero_bits = true; return ErrorNone; } @@ -1452,7 +1447,8 @@ static OnePossibleValue type_val_resolve_has_one_possible_value(CodeGen *g, ZigV case LazyValueIdArrayType: { LazyValueArrayType *lazy_array_type = reinterpret_cast(type_val->data.x_lazy); - if (lazy_array_type->length < 1) + // The sentinel counts as an extra element + if (lazy_array_type->length == 0 && lazy_array_type->sentinel == nullptr) return OnePossibleValueYes; return type_val_resolve_has_one_possible_value(g, lazy_array_type->elem_type->value); } @@ -5739,7 +5735,8 @@ OnePossibleValue type_has_one_possible_value(CodeGen *g, ZigType *type_entry) { case ZigTypeIdUnreachable: return OnePossibleValueYes; case ZigTypeIdArray: - if (type_entry->data.array.len == 0) + // The sentinel counts as an extra element + if (type_entry->data.array.len == 0 && type_entry->data.array.sentinel == nullptr) return OnePossibleValueYes; return type_has_one_possible_value(g, type_entry->data.array.child_type); case ZigTypeIdStruct: diff --git a/src/codegen.cpp b/src/codegen.cpp index dc6fe04cb4..75f3223250 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3584,7 +3584,9 @@ static bool value_is_all_undef(CodeGen *g, ZigValue *const_val) { } return true; } else if (const_val->type->id == ZigTypeIdArray) { - return value_is_all_undef_array(g, const_val, const_val->type->data.array.len); + const size_t full_len = const_val->type->data.array.len + + (const_val->type->data.array.sentinel != nullptr); + return value_is_all_undef_array(g, const_val, full_len); } else if (const_val->type->id == ZigTypeIdVector) { return value_is_all_undef_array(g, const_val, const_val->type->data.vector.len); } else { diff --git a/test/stage1/behavior/array.zig b/test/stage1/behavior/array.zig index da56864cc9..1f2d4a2f6b 100644 --- a/test/stage1/behavior/array.zig +++ b/test/stage1/behavior/array.zig @@ -376,3 +376,23 @@ test "type deduction for array subscript expression" { S.doTheTest(); comptime S.doTheTest(); } + +test "sentinel element count towards the ABI size calculation" { + const S = struct { + fn doTheTest() void { + const T = packed struct { + fill_pre: u8 = 0x55, + data: [0:0]u8 = undefined, + fill_post: u8 = 0xAA, + }; + var x = T{}; + var as_slice = mem.asBytes(&x); + expectEqual(@as(usize, 3), as_slice.len); + expectEqual(@as(u8, 0x55), as_slice[0]); + expectEqual(@as(u8, 0xAA), as_slice[2]); + } + }; + + S.doTheTest(); + comptime S.doTheTest(); +} From 63a4dbc30d3ef3c7f8a8c6a2ba2087eaab8b830a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 18 Mar 2020 11:11:41 -0400 Subject: [PATCH 2/3] array sentinel does not count towards type_has_one_possible_value --- src/analyze.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/analyze.cpp b/src/analyze.cpp index d0f8979c79..fbd7d85ac1 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -1447,8 +1447,7 @@ static OnePossibleValue type_val_resolve_has_one_possible_value(CodeGen *g, ZigV case LazyValueIdArrayType: { LazyValueArrayType *lazy_array_type = reinterpret_cast(type_val->data.x_lazy); - // The sentinel counts as an extra element - if (lazy_array_type->length == 0 && lazy_array_type->sentinel == nullptr) + if (lazy_array_type->length == 0) return OnePossibleValueYes; return type_val_resolve_has_one_possible_value(g, lazy_array_type->elem_type->value); } @@ -5735,8 +5734,7 @@ OnePossibleValue type_has_one_possible_value(CodeGen *g, ZigType *type_entry) { case ZigTypeIdUnreachable: return OnePossibleValueYes; case ZigTypeIdArray: - // The sentinel counts as an extra element - if (type_entry->data.array.len == 0 && type_entry->data.array.sentinel == nullptr) + if (type_entry->data.array.len == 0) return OnePossibleValueYes; return type_has_one_possible_value(g, type_entry->data.array.child_type); case ZigTypeIdStruct: From 11a4ce42c16c17422cd272f154c9c33231bcc61a Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Fri, 28 Feb 2020 12:06:43 +0100 Subject: [PATCH 3/3] zig fmt: Respect trailing commas in error set declarations The logic is not perfect as it doesn't take into account the presence of doc comments, but it's an improvement over the status quo. --- lib/std/zig/parser_test.zig | 2 ++ lib/std/zig/render.zig | 58 ++++++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/lib/std/zig/parser_test.zig b/lib/std/zig/parser_test.zig index d00568e49f..e1fe07a57c 100644 --- a/lib/std/zig/parser_test.zig +++ b/lib/std/zig/parser_test.zig @@ -1509,6 +1509,8 @@ test "zig fmt: error set declaration" { \\const Error = error{OutOfMemory}; \\const Error = error{}; \\ + \\const Error = error{ OutOfMemory, OutOfTime }; + \\ ); } diff --git a/lib/std/zig/render.zig b/lib/std/zig/render.zig index a3a72fb23f..23dc9e02ac 100644 --- a/lib/std/zig/render.zig +++ b/lib/std/zig/render.zig @@ -1268,25 +1268,51 @@ fn renderExpression( } try renderToken(tree, stream, err_set_decl.error_token, indent, start_col, Space.None); // error - try renderToken(tree, stream, lbrace, indent, start_col, Space.Newline); // { - const new_indent = indent + indent_delta; - var it = err_set_decl.decls.iterator(0); - while (it.next()) |node| { - try stream.writeByteNTimes(' ', new_indent); + const src_has_trailing_comma = blk: { + const maybe_comma = tree.prevToken(err_set_decl.rbrace_token); + break :blk tree.tokens.at(maybe_comma).id == .Comma; + }; - if (it.peek()) |next_node| { - try renderExpression(allocator, stream, tree, new_indent, start_col, node.*, Space.None); - try renderToken(tree, stream, tree.nextToken(node.*.lastToken()), new_indent, start_col, Space.Newline); // , + if (src_has_trailing_comma) { + try renderToken(tree, stream, lbrace, indent, start_col, Space.Newline); // { + const new_indent = indent + indent_delta; - try renderExtraNewline(tree, stream, start_col, next_node.*); - } else { - try renderExpression(allocator, stream, tree, new_indent, start_col, node.*, Space.Comma); + var it = err_set_decl.decls.iterator(0); + while (it.next()) |node| { + try stream.writeByteNTimes(' ', new_indent); + + if (it.peek()) |next_node| { + try renderExpression(allocator, stream, tree, new_indent, start_col, node.*, Space.None); + try renderToken(tree, stream, tree.nextToken(node.*.lastToken()), new_indent, start_col, Space.Newline); // , + + try renderExtraNewline(tree, stream, start_col, next_node.*); + } else { + try renderExpression(allocator, stream, tree, new_indent, start_col, node.*, Space.Comma); + } } - } - try stream.writeByteNTimes(' ', indent); - return renderToken(tree, stream, err_set_decl.rbrace_token, indent, start_col, space); // } + try stream.writeByteNTimes(' ', indent); + return renderToken(tree, stream, err_set_decl.rbrace_token, indent, start_col, space); // } + } else { + try renderToken(tree, stream, lbrace, indent, start_col, Space.Space); // { + + var it = err_set_decl.decls.iterator(0); + while (it.next()) |node| { + if (it.peek()) |next_node| { + try renderExpression(allocator, stream, tree, indent, start_col, node.*, Space.None); + + const comma_token = tree.nextToken(node.*.lastToken()); + assert(tree.tokens.at(comma_token).id == .Comma); + try renderToken(tree, stream, comma_token, indent, start_col, Space.Space); // , + try renderExtraNewline(tree, stream, start_col, next_node.*); + } else { + try renderExpression(allocator, stream, tree, indent, start_col, node.*, Space.Space); + } + } + + return renderToken(tree, stream, err_set_decl.rbrace_token, indent, start_col, space); // } + } }, .ErrorTag => { @@ -1589,8 +1615,7 @@ fn renderExpression( } } else { var it = switch_case.items.iterator(0); - while (true) { - const node = it.next().?; + while (it.next()) |node| { if (it.peek()) |next_node| { try renderExpression(allocator, stream, tree, indent, start_col, node.*, Space.None); @@ -1601,7 +1626,6 @@ fn renderExpression( } else { try renderExpression(allocator, stream, tree, indent, start_col, node.*, Space.Comma); try stream.writeByteNTimes(' ', indent); - break; } } }