From 17330867eb8f9e24ec8aadf23592e36f90f9dcb3 Mon Sep 17 00:00:00 2001 From: David Rubin Date: Wed, 19 Feb 2025 09:46:30 -0800 Subject: Sema: compile error on reifying align(0) struct fields --- src/Sema.zig | 63 +++++++++++++++++++++--------------------------------------- 1 file changed, 22 insertions(+), 41 deletions(-) (limited to 'src/Sema.zig') diff --git a/src/Sema.zig b/src/Sema.zig index d7add0724d..b855e4cc9d 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -2649,7 +2649,13 @@ pub fn analyzeAsAlign( src: LazySrcLoc, air_ref: Air.Inst.Ref, ) !Alignment { - const alignment_big = try sema.analyzeAsInt(block, src, air_ref, align_ty, .{ .simple = .@"align" }); + const alignment_big = try sema.analyzeAsInt( + block, + src, + air_ref, + align_ty, + .{ .simple = .@"align" }, + ); return sema.validateAlign(block, src, alignment_big); } @@ -18807,7 +18813,7 @@ fn zirPtrType(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air const abi_align: Alignment = if (inst_data.flags.has_align) blk: { const ref: Zir.Inst.Ref = @enumFromInt(sema.code.extra[extra_i]); extra_i += 1; - const coerced = try sema.coerce(block, .u32, try sema.resolveInst(ref), align_src); + const coerced = try sema.coerce(block, align_ty, try sema.resolveInst(ref), align_src); const val = try sema.resolveConstDefinedValue(block, align_src, coerced, .{ .simple = .@"align" }); // Check if this happens to be the lazy alignment of our element type, in // which case we can make this 0 without resolving it. @@ -20325,15 +20331,11 @@ fn zirReify( try ip.getOrPutString(gpa, pt.tid, "sentinel_ptr", .no_embedded_nulls), ).?); - if (!try sema.intFitsInType(alignment_val, .u32, null)) { - return sema.fail(block, src, "alignment must fit in 'u32'", .{}); + if (!try sema.intFitsInType(alignment_val, align_ty, null)) { + return sema.fail(block, src, "alignment must fit in '{}'", .{align_ty.fmt(pt)}); } - const alignment_val_int = try alignment_val.toUnsignedIntSema(pt); - if (alignment_val_int > 0 and !math.isPowerOfTwo(alignment_val_int)) { - return sema.fail(block, src, "alignment value '{d}' is not a power of two or zero", .{alignment_val_int}); - } - const abi_align = Alignment.fromByteUnits(alignment_val_int); + const abi_align = try sema.validateAlign(block, src, alignment_val_int); const elem_ty = child_val.toType(); if (abi_align != .none) { @@ -21017,11 +21019,7 @@ fn reifyUnion( field_ty.* = field_type_val.toIntern(); if (any_aligns) { const byte_align = try (try field_info.fieldValue(pt, 2)).toUnsignedIntSema(pt); - if (byte_align > 0 and !math.isPowerOfTwo(byte_align)) { - // TODO: better source location - return sema.fail(block, src, "alignment value '{d}' is not a power of two or zero", .{byte_align}); - } - field_aligns[field_idx] = Alignment.fromByteUnits(byte_align); + field_aligns[field_idx] = try sema.validateAlign(block, src, byte_align); } } @@ -21062,11 +21060,7 @@ fn reifyUnion( field_ty.* = field_type_val.toIntern(); if (any_aligns) { const byte_align = try (try field_info.fieldValue(pt, 2)).toUnsignedIntSema(pt); - if (byte_align > 0 and !math.isPowerOfTwo(byte_align)) { - // TODO: better source location - return sema.fail(block, src, "alignment value '{d}' is not a power of two or zero", .{byte_align}); - } - field_aligns[field_idx] = Alignment.fromByteUnits(byte_align); + field_aligns[field_idx] = try sema.validateAlign(block, src, byte_align); } } @@ -21266,7 +21260,6 @@ fn reifyStruct( var any_comptime_fields = false; var any_default_inits = false; - var any_aligned_fields = false; for (0..fields_len) |field_idx| { const field_info = try fields_val.elemValue(pt, field_idx); @@ -21301,11 +21294,6 @@ fn reifyStruct( if (field_is_comptime) any_comptime_fields = true; if (field_default_value != .none) any_default_inits = true; - switch (try field_alignment_val.orderAgainstZeroSema(pt)) { - .eq => {}, - .gt => any_aligned_fields = true, - .lt => unreachable, - } } const tracked_inst = try block.trackZir(inst); @@ -21317,7 +21305,7 @@ fn reifyStruct( .requires_comptime = .unknown, .any_comptime_fields = any_comptime_fields, .any_default_inits = any_default_inits, - .any_aligned_fields = any_aligned_fields, + .any_aligned_fields = true, .inits_resolved = true, .key = .{ .reified = .{ .zir_index = tracked_inst, @@ -21361,21 +21349,14 @@ fn reifyStruct( return sema.fail(block, src, "duplicate struct field name {f}", .{field_name.fmt(ip)}); } - if (any_aligned_fields) { - if (!try sema.intFitsInType(field_alignment_val, .u32, null)) { - return sema.fail(block, src, "alignment must fit in 'u32'", .{}); - } - - const byte_align = try field_alignment_val.toUnsignedIntSema(pt); - if (byte_align == 0) { - if (layout != .@"packed") { - struct_type.field_aligns.get(ip)[field_idx] = .none; - } - } else { - if (layout == .@"packed") return sema.fail(block, src, "alignment in a packed struct field must be set to 0", .{}); - if (!math.isPowerOfTwo(byte_align)) return sema.fail(block, src, "alignment value '{d}' is not a power of two or zero", .{byte_align}); - struct_type.field_aligns.get(ip)[field_idx] = Alignment.fromNonzeroByteUnits(byte_align); - } + if (!try sema.intFitsInType(field_alignment_val, align_ty, null)) { + return sema.fail(block, src, "alignment must fit in '{f}'", .{align_ty.fmt(pt)}); + } + const byte_align = try field_alignment_val.toUnsignedIntSema(pt); + if (layout == .@"packed") { + if (byte_align != 0) return sema.fail(block, src, "alignment in a packed struct field must be set to 0", .{}); + } else { + struct_type.field_aligns.get(ip)[field_idx] = try sema.validateAlign(block, src, byte_align); } const field_is_comptime = field_is_comptime_val.toBool(); -- cgit v1.2.3 From d6c74a95fdaba4ed373f80baa725dcc53b27e402 Mon Sep 17 00:00:00 2001 From: David Rubin Date: Mon, 24 Feb 2025 04:02:06 -0800 Subject: remove usages of `.alignment = 0` --- lib/compiler/aro/aro/Attribute.zig | 2 +- lib/std/meta.zig | 2 +- lib/std/zig/llvm/Builder.zig | 7 +++--- src/InternPool.zig | 38 ++++++++++++++++++-------------- src/Sema.zig | 2 +- src/codegen/aarch64/Assemble.zig | 17 ++++++++------ test/behavior/tuple.zig | 6 +++-- test/behavior/type.zig | 2 +- test/cases/compile_errors/align_zero.zig | 20 +---------------- 9 files changed, 45 insertions(+), 51 deletions(-) (limited to 'src/Sema.zig') diff --git a/lib/compiler/aro/aro/Attribute.zig b/lib/compiler/aro/aro/Attribute.zig index a5b78b8463..4db287b65c 100644 --- a/lib/compiler/aro/aro/Attribute.zig +++ b/lib/compiler/aro/aro/Attribute.zig @@ -708,7 +708,7 @@ pub const Arguments = blk: { field.* = .{ .name = decl.name, .type = @field(attributes, decl.name), - .alignment = 0, + .alignment = @alignOf(@field(attributes, decl.name)), }; } diff --git a/lib/std/meta.zig b/lib/std/meta.zig index 0cee23cfa8..65b7d60c18 100644 --- a/lib/std/meta.zig +++ b/lib/std/meta.zig @@ -939,7 +939,7 @@ fn CreateUniqueTuple(comptime N: comptime_int, comptime types: [N]type) type { .type = T, .default_value_ptr = null, .is_comptime = false, - .alignment = 0, + .alignment = @alignOf(T), }; } diff --git a/lib/std/zig/llvm/Builder.zig b/lib/std/zig/llvm/Builder.zig index f3ff63ec33..ba6faaec2c 100644 --- a/lib/std/zig/llvm/Builder.zig +++ b/lib/std/zig/llvm/Builder.zig @@ -8533,18 +8533,19 @@ pub const Metadata = enum(u32) { .type = []const u8, .default_value_ptr = null, .is_comptime = false, - .alignment = 0, + .alignment = @alignOf([]const u8), }; } fmt_str = fmt_str ++ "("; inline for (fields[2..], names) |*field, name| { fmt_str = fmt_str ++ "{[" ++ name ++ "]f}"; + const T = std.fmt.Formatter(FormatData, format); field.* = .{ .name = name, - .type = std.fmt.Formatter(FormatData, format), + .type = T, .default_value_ptr = null, .is_comptime = false, - .alignment = 0, + .alignment = @alignOf(T), }; } fmt_str = fmt_str ++ ")\n"; diff --git a/src/InternPool.zig b/src/InternPool.zig index 15d895aed0..ed922db742 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -1137,13 +1137,16 @@ const Local = struct { const elem_info = @typeInfo(Elem).@"struct"; const elem_fields = elem_info.fields; var new_fields: [elem_fields.len]std.builtin.Type.StructField = undefined; - for (&new_fields, elem_fields) |*new_field, elem_field| new_field.* = .{ - .name = elem_field.name, - .type = *[len]elem_field.type, - .default_value_ptr = null, - .is_comptime = false, - .alignment = 0, - }; + for (&new_fields, elem_fields) |*new_field, elem_field| { + const T = *[len]elem_field.type; + new_field.* = .{ + .name = elem_field.name, + .type = T, + .default_value_ptr = null, + .is_comptime = false, + .alignment = @alignOf(T), + }; + } return @Type(.{ .@"struct" = .{ .layout = .auto, .fields = &new_fields, @@ -1158,22 +1161,25 @@ const Local = struct { const elem_info = @typeInfo(Elem).@"struct"; const elem_fields = elem_info.fields; var new_fields: [elem_fields.len]std.builtin.Type.StructField = undefined; - for (&new_fields, elem_fields) |*new_field, elem_field| new_field.* = .{ - .name = elem_field.name, - .type = @Type(.{ .pointer = .{ + for (&new_fields, elem_fields) |*new_field, elem_field| { + const T = @Type(.{ .pointer = .{ .size = opts.size, .is_const = opts.is_const, .is_volatile = false, - .alignment = 0, + .alignment = @alignOf(elem_field.type), .address_space = .generic, .child = elem_field.type, .is_allowzero = false, .sentinel_ptr = null, - } }), - .default_value_ptr = null, - .is_comptime = false, - .alignment = 0, - }; + } }); + new_field.* = .{ + .name = elem_field.name, + .type = T, + .default_value_ptr = null, + .is_comptime = false, + .alignment = @alignOf(T), + }; + } return @Type(.{ .@"struct" = .{ .layout = .auto, .fields = &new_fields, diff --git a/src/Sema.zig b/src/Sema.zig index b855e4cc9d..3bee1130af 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -20332,7 +20332,7 @@ fn zirReify( ).?); if (!try sema.intFitsInType(alignment_val, align_ty, null)) { - return sema.fail(block, src, "alignment must fit in '{}'", .{align_ty.fmt(pt)}); + return sema.fail(block, src, "alignment must fit in '{f}'", .{align_ty.fmt(pt)}); } const alignment_val_int = try alignment_val.toUnsignedIntSema(pt); const abi_align = try sema.validateAlign(block, src, alignment_val_int); diff --git a/src/codegen/aarch64/Assemble.zig b/src/codegen/aarch64/Assemble.zig index 494e012d80..098065bb16 100644 --- a/src/codegen/aarch64/Assemble.zig +++ b/src/codegen/aarch64/Assemble.zig @@ -33,13 +33,16 @@ pub fn nextInstruction(as: *Assemble) !?Instruction { var symbols: Symbols: { const symbols = @typeInfo(@TypeOf(instruction.symbols)).@"struct".fields; var symbol_fields: [symbols.len]std.builtin.Type.StructField = undefined; - for (&symbol_fields, symbols) |*symbol_field, symbol| symbol_field.* = .{ - .name = symbol.name, - .type = zonCast(SymbolSpec, @field(instruction.symbols, symbol.name), .{}).Storage(), - .default_value_ptr = null, - .is_comptime = false, - .alignment = 0, - }; + for (&symbol_fields, symbols) |*symbol_field, symbol| { + const Storage = zonCast(SymbolSpec, @field(instruction.symbols, symbol.name), .{}).Storage(); + symbol_field.* = .{ + .name = symbol.name, + .type = Storage, + .default_value_ptr = null, + .is_comptime = false, + .alignment = @alignOf(Storage), + }; + } break :Symbols @Type(.{ .@"struct" = .{ .layout = .auto, .fields = &symbol_fields, diff --git a/test/behavior/tuple.zig b/test/behavior/tuple.zig index e760455e09..9e17d61856 100644 --- a/test/behavior/tuple.zig +++ b/test/behavior/tuple.zig @@ -318,6 +318,8 @@ test "tuple type with void field" { test "zero sized struct in tuple handled correctly" { const State = struct { const Self = @This(); + const Inner = struct {}; + data: @Type(.{ .@"struct" = .{ .is_tuple = true, @@ -325,10 +327,10 @@ test "zero sized struct in tuple handled correctly" { .decls = &.{}, .fields = &.{.{ .name = "0", - .type = struct {}, + .type = Inner, .default_value_ptr = null, .is_comptime = false, - .alignment = 0, + .alignment = @alignOf(Inner), }}, }, }), diff --git a/test/behavior/type.zig b/test/behavior/type.zig index 58e049d896..b5ac2d95f7 100644 --- a/test/behavior/type.zig +++ b/test/behavior/type.zig @@ -735,7 +735,7 @@ test "struct field names sliced at comptime from larger string" { var it = std.mem.tokenizeScalar(u8, text, '\n'); while (it.next()) |name| { fields = fields ++ &[_]Type.StructField{.{ - .alignment = 0, + .alignment = @alignOf(usize), .name = name ++ "", .type = usize, .default_value_ptr = null, diff --git a/test/cases/compile_errors/align_zero.zig b/test/cases/compile_errors/align_zero.zig index e6d1a993d4..e54a32ce31 100644 --- a/test/cases/compile_errors/align_zero.zig +++ b/test/cases/compile_errors/align_zero.zig @@ -65,24 +65,7 @@ export fn k() void { } }); } -export fn l() void { - _ = @Type(.{ .@"struct" = .{ - .layout = .@"packed", - .fields = &.{.{ - .name = "test", - .type = u32, - .default_value_ptr = null, - .is_comptime = false, - .alignment = 8, - }}, - .decls = &.{}, - .is_tuple = false, - } }); -} - // error -// backend=stage2 -// target=native // // :1:27: error: alignment must be >= 1 // :7:34: error: alignment must be >= 1 @@ -93,6 +76,5 @@ export fn l() void { // :29:17: error: alignment must be >= 1 // :33:35: error: alignment must be >= 1 // :37:34: error: alignment must be >= 1 -// :41:9: error: alignment can only be 0 on packed struct fields +// :41:9: error: alignment must be >= 1 // :56:9: error: alignment must be >= 1 -// :69:9: error: alignment in a packed struct field must be set to 0 -- cgit v1.2.3 From 5678a600ffb2ec27f88ab6a308b4eaf56c60daed Mon Sep 17 00:00:00 2001 From: David Rubin Date: Fri, 1 Aug 2025 12:00:34 -0700 Subject: refactor `reifyUnion` alignment handling --- src/Sema.zig | 49 +++++++++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 28 deletions(-) (limited to 'src/Sema.zig') diff --git a/src/Sema.zig b/src/Sema.zig index 3bee1130af..8ea0a19fe8 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -20912,8 +20912,6 @@ fn reifyUnion( std.hash.autoHash(&hasher, opt_tag_type_val.toIntern()); std.hash.autoHash(&hasher, fields_len); - var any_aligns = false; - for (0..fields_len) |field_idx| { const field_info = try fields_val.elemValue(pt, field_idx); @@ -20922,16 +20920,11 @@ fn reifyUnion( const field_align_val = try sema.resolveLazyValue(try field_info.fieldValue(pt, 2)); const field_name = try sema.sliceToIpString(block, src, field_name_val, .{ .simple = .union_field_name }); - std.hash.autoHash(&hasher, .{ field_name, field_type_val.toIntern(), field_align_val.toIntern(), }); - - if (field_align_val.toUnsignedInt(zcu) != 0) { - any_aligns = true; - } } const tracked_inst = try block.trackZir(inst); @@ -20948,7 +20941,7 @@ fn reifyUnion( true => .safety, false => .none, }, - .any_aligned_fields = any_aligns, + .any_aligned_fields = layout != .@"packed", .requires_comptime = .unknown, .assumed_runtime_bits = false, .assumed_pointer_aligned = false, @@ -20981,8 +20974,7 @@ fn reifyUnion( ); wip_ty.setName(ip, type_name.name, type_name.nav); - const field_types = try sema.arena.alloc(InternPool.Index, fields_len); - const field_aligns = if (any_aligns) try sema.arena.alloc(InternPool.Alignment, fields_len) else undefined; + const loaded_union = ip.loadUnionType(wip_ty.index); const enum_tag_ty, const has_explicit_tag = if (opt_tag_type_val.optionalValue(zcu)) |tag_type_val| tag_ty: { switch (ip.indexToKey(tag_type_val.toIntern())) { @@ -20995,11 +20987,12 @@ fn reifyUnion( const tag_ty_fields_len = enum_tag_ty.enumFieldCount(zcu); var seen_tags = try std.DynamicBitSetUnmanaged.initEmpty(sema.arena, tag_ty_fields_len); - for (field_types, 0..) |*field_ty, field_idx| { + for (0..fields_len) |field_idx| { const field_info = try fields_val.elemValue(pt, field_idx); const field_name_val = try field_info.fieldValue(pt, 0); const field_type_val = try field_info.fieldValue(pt, 1); + const field_alignment_val = try field_info.fieldValue(pt, 2); // Don't pass a reason; first loop acts as an assertion that this is valid. const field_name = try sema.sliceToIpString(block, src, field_name_val, undefined); @@ -21016,10 +21009,12 @@ fn reifyUnion( } seen_tags.set(enum_index); - field_ty.* = field_type_val.toIntern(); - if (any_aligns) { - const byte_align = try (try field_info.fieldValue(pt, 2)).toUnsignedIntSema(pt); - field_aligns[field_idx] = try sema.validateAlign(block, src, byte_align); + loaded_union.field_types.get(ip)[field_idx] = field_type_val.toIntern(); + const byte_align = try field_alignment_val.toUnsignedIntSema(pt); + if (layout == .@"packed") { + if (byte_align != 0) return sema.fail(block, src, "alignment of a packed union field must be set to 0", .{}); + } else { + loaded_union.field_aligns.get(ip)[field_idx] = try sema.validateAlign(block, src, byte_align); } } @@ -21043,11 +21038,12 @@ fn reifyUnion( var field_names: std.AutoArrayHashMapUnmanaged(InternPool.NullTerminatedString, void) = .empty; try field_names.ensureTotalCapacity(sema.arena, fields_len); - for (field_types, 0..) |*field_ty, field_idx| { + for (0..fields_len) |field_idx| { const field_info = try fields_val.elemValue(pt, field_idx); const field_name_val = try field_info.fieldValue(pt, 0); const field_type_val = try field_info.fieldValue(pt, 1); + const field_alignment_val = try field_info.fieldValue(pt, 2); // Don't pass a reason; first loop acts as an assertion that this is valid. const field_name = try sema.sliceToIpString(block, src, field_name_val, undefined); @@ -21057,10 +21053,12 @@ fn reifyUnion( return sema.fail(block, src, "duplicate union field {f}", .{field_name.fmt(ip)}); } - field_ty.* = field_type_val.toIntern(); - if (any_aligns) { - const byte_align = try (try field_info.fieldValue(pt, 2)).toUnsignedIntSema(pt); - field_aligns[field_idx] = try sema.validateAlign(block, src, byte_align); + loaded_union.field_types.get(ip)[field_idx] = field_type_val.toIntern(); + const byte_align = try field_alignment_val.toUnsignedIntSema(pt); + if (layout == .@"packed") { + if (byte_align != 0) return sema.fail(block, src, "alignment of a packed union field must be set to 0", .{}); + } else { + loaded_union.field_aligns.get(ip)[field_idx] = try sema.validateAlign(block, src, byte_align); } } @@ -21069,7 +21067,7 @@ fn reifyUnion( }; errdefer if (!has_explicit_tag) ip.remove(pt.tid, enum_tag_ty); // remove generated tag type on error - for (field_types) |field_ty_ip| { + for (loaded_union.field_types.get(ip)) |field_ty_ip| { const field_ty: Type = .fromInterned(field_ty_ip); if (field_ty.zigTypeTag(zcu) == .@"opaque") { return sema.failWithOwnedErrorMsg(block, msg: { @@ -21103,11 +21101,6 @@ fn reifyUnion( } } - const loaded_union = ip.loadUnionType(wip_ty.index); - loaded_union.setFieldTypes(ip, field_types); - if (any_aligns) { - loaded_union.setFieldAligns(ip, field_aligns); - } loaded_union.setTagType(ip, enum_tag_ty); loaded_union.setStatus(ip, .have_field_types); @@ -21305,7 +21298,7 @@ fn reifyStruct( .requires_comptime = .unknown, .any_comptime_fields = any_comptime_fields, .any_default_inits = any_default_inits, - .any_aligned_fields = true, + .any_aligned_fields = layout != .@"packed", .inits_resolved = true, .key = .{ .reified = .{ .zir_index = tracked_inst, @@ -21354,7 +21347,7 @@ fn reifyStruct( } const byte_align = try field_alignment_val.toUnsignedIntSema(pt); if (layout == .@"packed") { - if (byte_align != 0) return sema.fail(block, src, "alignment in a packed struct field must be set to 0", .{}); + if (byte_align != 0) return sema.fail(block, src, "alignment of a packed struct field must be set to 0", .{}); } else { struct_type.field_aligns.get(ip)[field_idx] = try sema.validateAlign(block, src, byte_align); } -- cgit v1.2.3