From 81b5df347a2b92566dc679fdd4e110812dfe27d0 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 21 Sep 2023 17:29:34 -0700 Subject: [PATCH] compiler: fix structFieldName crash for tuples When struct types have no field names, the names are implicitly understood to be strings corresponding to the field indexes in declaration order. It used to be the case that a NullTerminatedString would be stored for each field in this case, however, now, callers must handle the possibility that there are no names stored at all. This commit introduces `legacyStructFieldName`, a function to fake the previous behavior. Probably something better could be done by reworking all the callsites of this function. --- src/InternPool.zig | 11 +++++++++++ src/Sema.zig | 36 +++++++++++++++++++----------------- src/TypedValue.zig | 6 +++--- src/codegen/c.zig | 14 ++++++++------ src/codegen/c/type.zig | 15 +++++++++------ src/type.zig | 20 ++++++++++++++++---- 6 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/InternPool.zig b/src/InternPool.zig index b7d79d6c29c7762242dc319f00dafaba0b1fc0c8..79d41277870b528f351105c43075b8a1c89071b5 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -656,6 +656,17 @@ pub const Key = union(enum) { pub fn isTuple(self: AnonStructType) bool { return self.names.len == 0; } + + pub fn fieldName( + self: AnonStructType, + ip: *const InternPool, + index: u32, + ) OptionalNullTerminatedString { + if (self.names.len == 0) + return .none; + + return self.names.get(ip)[index].toOptional(); + } }; /// Serves two purposes: diff --git a/src/Sema.zig b/src/Sema.zig index ca5b692da1e52420c520eda38f033fbc6afbd379..5c08102a27926d7369e208c06ef99c996613cfe3 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -4699,12 +4699,13 @@ fn validateStructInit( // In this case the only thing we need to do is evaluate the implicit // store instructions for default field values, and report any missing fields. // Avoid the cost of the extra machinery for detecting a comptime struct init value. - for (found_fields, 0..) |field_ptr, i| { + for (found_fields, 0..) |field_ptr, i_usize| { + const i: u32 = @intCast(i_usize); if (field_ptr != 0) continue; const default_val = struct_ty.structFieldDefaultValue(i, mod); if (default_val.toIntern() == .unreachable_value) { - if (struct_ty.isTuple(mod)) { + const field_name = struct_ty.structFieldName(i, mod).unwrap() orelse { const template = "missing tuple field with index {d}"; if (root_msg) |msg| { try sema.errNote(block, init_src, msg, template, .{i}); @@ -4712,8 +4713,7 @@ fn validateStructInit( root_msg = try sema.errMsg(block, init_src, template, .{i}); } continue; - } - const field_name = struct_ty.structFieldName(i, mod); + }; const template = "missing struct field: {}"; const args = .{field_name.fmt(ip)}; if (root_msg) |msg| { @@ -4763,7 +4763,8 @@ fn validateStructInit( // ends up being comptime-known. const field_values = try sema.arena.alloc(InternPool.Index, struct_ty.structFieldCount(mod)); - field: for (found_fields, 0..) |field_ptr, i| { + field: for (found_fields, 0..) |field_ptr, i_usize| { + const i: u32 = @intCast(i_usize); if (field_ptr != 0) { // Determine whether the value stored to this pointer is comptime-known. const field_ty = struct_ty.structFieldType(i, mod); @@ -4842,7 +4843,7 @@ fn validateStructInit( const default_val = struct_ty.structFieldDefaultValue(i, mod); if (default_val.toIntern() == .unreachable_value) { - if (struct_ty.isTuple(mod)) { + const field_name = struct_ty.structFieldName(i, mod).unwrap() orelse { const template = "missing tuple field with index {d}"; if (root_msg) |msg| { try sema.errNote(block, init_src, msg, template, .{i}); @@ -4850,8 +4851,7 @@ fn validateStructInit( root_msg = try sema.errMsg(block, init_src, template, .{i}); } continue; - } - const field_name = struct_ty.structFieldName(i, mod); + }; const template = "missing struct field: {}"; const args = .{field_name.fmt(ip)}; if (root_msg) |msg| { @@ -21862,10 +21862,10 @@ fn ptrCastFull( const msg = try sema.errMsg(block, src, "cast increases pointer alignment", .{}); errdefer msg.destroy(sema.gpa); try sema.errNote(block, operand_src, msg, "'{}' has alignment '{d}'", .{ - operand_ty.fmt(mod), src_align, + operand_ty.fmt(mod), src_align.toByteUnits(0), }); try sema.errNote(block, src, msg, "'{}' has alignment '{d}'", .{ - dest_ty.fmt(mod), dest_align, + dest_ty.fmt(mod), dest_align.toByteUnits(0), }); try sema.errNote(block, src, msg, "use @alignCast to assert pointer alignment", .{}); break :msg msg; @@ -26376,7 +26376,7 @@ fn fieldCallBind( const max = concrete_ty.structFieldCount(mod); for (0..max) |i_usize| { const i: u32 = @intCast(i_usize); - if (field_name == concrete_ty.structFieldName(i, mod)) { + if (field_name == concrete_ty.structFieldName(i, mod).unwrap().?) { return sema.finishFieldCallBind(block, src, ptr_ty, concrete_ty.structFieldType(i, mod), i, object_ptr); } } @@ -31143,7 +31143,8 @@ fn coerceTupleToTuple( var root_msg: ?*Module.ErrorMsg = null; errdefer if (root_msg) |msg| msg.destroy(sema.gpa); - for (field_refs, 0..) |*field_ref, i| { + for (field_refs, 0..) |*field_ref, i_usize| { + const i: u32 = @intCast(i_usize); if (field_ref.* != .none) continue; const default_val = switch (ip.indexToKey(tuple_ty.toIntern())) { @@ -31154,7 +31155,7 @@ fn coerceTupleToTuple( const field_src = inst_src; // TODO better source location if (default_val == .none) { - if (tuple_ty.isTuple(mod)) { + const field_name = tuple_ty.structFieldName(i, mod).unwrap() orelse { const template = "missing tuple field: {d}"; if (root_msg) |msg| { try sema.errNote(block, field_src, msg, template, .{i}); @@ -31162,9 +31163,9 @@ fn coerceTupleToTuple( root_msg = try sema.errMsg(block, field_src, template, .{i}); } continue; - } + }; const template = "missing struct field: {}"; - const args = .{tuple_ty.structFieldName(i, mod).fmt(ip)}; + const args = .{field_name.fmt(ip)}; if (root_msg) |msg| { try sema.errNote(block, field_src, msg, template, args); } else { @@ -33897,8 +33898,9 @@ fn resolvePeerTypesInner( } if (!is_tuple) { - for (field_names, 0..) |expected, field_idx| { - const actual = ty.structFieldName(field_idx, mod); + for (field_names, 0..) |expected, field_index_usize| { + const field_index: u32 = @intCast(field_index_usize); + const actual = ty.structFieldName(field_index, mod).unwrap().?; if (actual == expected) continue; return .{ .conflict = .{ .peer_idx_a = first_idx, diff --git a/src/TypedValue.zig b/src/TypedValue.zig index 6e85a63e5046bd3a3a24ab64088f100dceb9f268..9b7f3d7e7b472169f9ee083c7c42ccfcb6fc8b1f 100644 --- a/src/TypedValue.zig +++ b/src/TypedValue.zig @@ -355,11 +355,11 @@ pub fn print( const container_ty = ptr_container_ty.childType(mod); switch (container_ty.zigTypeTag(mod)) { .Struct => { - if (container_ty.isTuple(mod)) { + if (container_ty.structFieldName(@intCast(field.index), mod).unwrap()) |field_name| { + try writer.print(".{i}", .{field_name.fmt(ip)}); + } else { try writer.print("[{d}]", .{field.index}); } - const field_name = container_ty.structFieldName(@as(usize, @intCast(field.index)), mod); - try writer.print(".{i}", .{field_name.fmt(ip)}); }, .Union => { const field_name = mod.typeToUnion(container_ty).?.field_names.get(ip)[@intCast(field.index)]; diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 7f3f1b795e5c65e76589611aad3ce139e9cf7e5c..54e13db9a13a492c296fe4f70d16b54a0c91e9ca 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -5226,7 +5226,8 @@ fn fieldLocation( const container_ty = container_ptr_ty.childType(mod); return switch (container_ty.zigTypeTag(mod)) { .Struct => switch (container_ty.containerLayout(mod)) { - .Auto, .Extern => for (field_index..container_ty.structFieldCount(mod)) |next_field_index| { + .Auto, .Extern => for (field_index..container_ty.structFieldCount(mod)) |next_field_index_usize| { + const next_field_index: u32 = @intCast(next_field_index_usize); if (container_ty.structFieldIsComptime(next_field_index, mod)) continue; const field_ty = container_ty.structFieldType(next_field_index, mod); if (!field_ty.hasRuntimeBitsIgnoreComptime(mod)) continue; @@ -5234,7 +5235,7 @@ fn fieldLocation( break .{ .field = if (container_ty.isSimpleTuple(mod)) .{ .field = next_field_index } else - .{ .identifier = ip.stringToSlice(container_ty.structFieldName(next_field_index, mod)) } }; + .{ .identifier = ip.stringToSlice(container_ty.legacyStructFieldName(next_field_index, mod)) } }; } else if (container_ty.hasRuntimeBitsIgnoreComptime(mod)) .end else .begin, .Packed => if (field_ptr_ty.ptrInfo(mod).packed_offset.host_size == 0) .{ .byte_offset = container_ty.packedStructFieldByteOffset(field_index, mod) + @divExact(container_ptr_ty.ptrInfo(mod).packed_offset.bit_offset, 8) } @@ -5421,7 +5422,7 @@ fn airStructFieldVal(f: *Function, inst: Air.Inst.Index) !CValue { .Auto, .Extern => if (struct_ty.isSimpleTuple(mod)) .{ .field = extra.field_index } else - .{ .identifier = ip.stringToSlice(struct_ty.structFieldName(extra.field_index, mod)) }, + .{ .identifier = ip.stringToSlice(struct_ty.legacyStructFieldName(extra.field_index, mod)) }, .Packed => { const struct_type = mod.typeToStruct(struct_ty).?; const int_info = struct_ty.intInfo(mod); @@ -5483,7 +5484,7 @@ fn airStructFieldVal(f: *Function, inst: Air.Inst.Index) !CValue { .anon_struct_type => |anon_struct_type| if (anon_struct_type.names.len == 0) .{ .field = extra.field_index } else - .{ .identifier = ip.stringToSlice(struct_ty.structFieldName(extra.field_index, mod)) }, + .{ .identifier = ip.stringToSlice(struct_ty.legacyStructFieldName(extra.field_index, mod)) }, .union_type => |union_type| field_name: { const union_obj = ip.loadUnionType(union_type); @@ -6816,7 +6817,8 @@ fn airAggregateInit(f: *Function, inst: Air.Inst.Index) !CValue { } }, .Struct => switch (inst_ty.containerLayout(mod)) { - .Auto, .Extern => for (resolved_elements, 0..) |element, field_i| { + .Auto, .Extern => for (resolved_elements, 0..) |element, field_i_usize| { + const field_i: u32 = @intCast(field_i_usize); if (inst_ty.structFieldIsComptime(field_i, mod)) continue; const field_ty = inst_ty.structFieldType(field_i, mod); if (!field_ty.hasRuntimeBitsIgnoreComptime(mod)) continue; @@ -6825,7 +6827,7 @@ fn airAggregateInit(f: *Function, inst: Air.Inst.Index) !CValue { try f.writeCValueMember(writer, local, if (inst_ty.isSimpleTuple(mod)) .{ .field = field_i } else - .{ .identifier = ip.stringToSlice(inst_ty.structFieldName(field_i, mod)) }); + .{ .identifier = ip.stringToSlice(inst_ty.legacyStructFieldName(field_i, mod)) }); try a.assign(f, writer); try f.writeCValue(writer, element, .Other); try a.end(f, writer); diff --git a/src/codegen/c/type.zig b/src/codegen/c/type.zig index 731d2b6557962de6fa5528282230ca02795a5d0a..81cbe467681587a1effbf0ce48bde3aab13ff0f2 100644 --- a/src/codegen/c/type.zig +++ b/src/codegen/c/type.zig @@ -1953,7 +1953,8 @@ pub const CType = extern union { const fields_pl = try arena.alloc(Payload.Fields.Field, c_fields_len); var c_field_i: usize = 0; - for (0..fields_len) |field_i| { + for (0..fields_len) |field_i_usize| { + const field_i: u32 = @intCast(field_i_usize); const field_ty = ty.structFieldType(field_i, mod); if ((zig_ty_tag == .Struct and ty.structFieldIsComptime(field_i, mod)) or !field_ty.hasRuntimeBitsIgnoreComptime(mod)) continue; @@ -1964,7 +1965,7 @@ pub const CType = extern union { std.fmt.allocPrintZ(arena, "f{}", .{field_i}) else arena.dupeZ(u8, ip.stringToSlice(switch (zig_ty_tag) { - .Struct => ty.structFieldName(field_i, mod), + .Struct => ty.legacyStructFieldName(field_i, mod), .Union => mod.typeToUnion(ty).?.field_names.get(ip)[field_i], else => unreachable, })), @@ -2097,7 +2098,8 @@ pub const CType = extern union { .Struct => ty.structFieldCount(mod), .Union => mod.typeToUnion(ty).?.field_names.len, else => unreachable, - }) |field_i| { + }) |field_i_usize| { + const field_i: u32 = @intCast(field_i_usize); const field_ty = ty.structFieldType(field_i, mod); if ((zig_ty_tag == .Struct and ty.structFieldIsComptime(field_i, mod)) or !field_ty.hasRuntimeBitsIgnoreComptime(mod)) continue; @@ -2116,7 +2118,7 @@ pub const CType = extern union { std.fmt.bufPrintZ(&name_buf, "f{}", .{field_i}) catch unreachable else ip.stringToSlice(switch (zig_ty_tag) { - .Struct => ty.structFieldName(field_i, mod), + .Struct => ty.legacyStructFieldName(field_i, mod), .Union => mod.typeToUnion(ty).?.field_names.get(ip)[field_i], else => unreachable, }), @@ -2225,7 +2227,8 @@ pub const CType = extern union { .Struct => ty.structFieldCount(mod), .Union => mod.typeToUnion(ty).?.field_names.len, else => unreachable, - }) |field_i| { + }) |field_i_usize| { + const field_i: u32 = @intCast(field_i_usize); const field_ty = ty.structFieldType(field_i, mod); if ((zig_ty_tag == .Struct and ty.structFieldIsComptime(field_i, mod)) or !field_ty.hasRuntimeBitsIgnoreComptime(mod)) continue; @@ -2240,7 +2243,7 @@ pub const CType = extern union { std.fmt.bufPrint(&name_buf, "f{}", .{field_i}) catch unreachable else mod.intern_pool.stringToSlice(switch (zig_ty_tag) { - .Struct => ty.structFieldName(field_i, mod), + .Struct => ty.legacyStructFieldName(field_i, mod), .Union => mod.typeToUnion(ty).?.field_names.get(ip)[field_i], else => unreachable, })); diff --git a/src/type.zig b/src/type.zig index a42f7dd4ee66ac57938f2059551bd31a066f7394..0ed8c394fce5fba775b7392ae93aa7629f634476 100644 --- a/src/type.zig +++ b/src/type.zig @@ -2907,16 +2907,28 @@ pub const Type = struct { return enum_type.tagValueIndex(ip, int_tag); } - pub fn structFieldName(ty: Type, field_index: usize, mod: *Module) InternPool.NullTerminatedString { + /// Returns none in the case of a tuple which uses the integer index as the field name. + pub fn structFieldName(ty: Type, field_index: u32, mod: *Module) InternPool.OptionalNullTerminatedString { const ip = &mod.intern_pool; return switch (ip.indexToKey(ty.toIntern())) { - .struct_type => |struct_type| struct_type.field_names.get(ip)[field_index], - .anon_struct_type => |anon_struct| anon_struct.names.get(ip)[field_index], + .struct_type => |struct_type| struct_type.fieldName(ip, field_index), + .anon_struct_type => |anon_struct| anon_struct.fieldName(ip, field_index), else => unreachable, }; } - pub fn structFieldCount(ty: Type, mod: *Module) usize { + /// When struct types have no field names, the names are implicitly understood to be + /// strings corresponding to the field indexes in declaration order. It used to be the + /// case that a NullTerminatedString would be stored for each field in this case, however, + /// now, callers must handle the possibility that there are no names stored at all. + /// Here we fake the previous behavior. Probably something better could be done by examining + /// all the callsites of this function. + pub fn legacyStructFieldName(ty: Type, i: u32, mod: *Module) InternPool.NullTerminatedString { + return ty.structFieldName(i, mod).unwrap() orelse + mod.intern_pool.getOrPutStringFmt(mod.gpa, "{d}", .{i}) catch @panic("OOM"); + } + + pub fn structFieldCount(ty: Type, mod: *Module) u32 { const ip = &mod.intern_pool; return switch (ip.indexToKey(ty.toIntern())) { .struct_type => |struct_type| struct_type.field_types.len, -- 2.54.0