From deb9f3e88ff309da542f4fd5063f1f1a05cae27d Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 14 Jul 2025 18:37:28 -0700 Subject: [PATCH] std.Io: handle packed structs better Rather than having the endian-suffixed functions be the preferred ones the unsuffixed ones are the preferred ones and the tricky functions get a special suffix. Makes packed structs read and written the same as integers. closes #12960 --- lib/std/Io/Reader.zig | 130 ++++++++++++++++++++++++++---------------- lib/std/Io/Writer.zig | 9 +-- src/Zcu.zig | 2 +- src/Zcu/PerThread.zig | 2 +- 4 files changed, 83 insertions(+), 60 deletions(-) diff --git a/lib/std/Io/Reader.zig b/lib/std/Io/Reader.zig index c8bd0f3000..d236b67748 100644 --- a/lib/std/Io/Reader.zig +++ b/lib/std/Io/Reader.zig @@ -1094,33 +1094,41 @@ pub inline fn takeInt(r: *Reader, comptime T: type, endian: std.builtin.Endian) return std.mem.readInt(T, try r.takeArray(n), endian); } +/// Asserts the buffer was initialized with a capacity at least `@bitSizeOf(T) / 8`. +pub inline fn peekInt(r: *Reader, comptime T: type, endian: std.builtin.Endian) Error!T { + const n = @divExact(@typeInfo(T).int.bits, 8); + return std.mem.readInt(T, try r.peekArray(n), endian); +} + /// Asserts the buffer was initialized with a capacity at least `n`. pub fn takeVarInt(r: *Reader, comptime Int: type, endian: std.builtin.Endian, n: usize) Error!Int { assert(n <= @sizeOf(Int)); return std.mem.readVarInt(Int, try r.take(n), endian); } +/// Obtains an unaligned pointer to the beginning of the stream, reinterpreted +/// as a pointer to the provided type, advancing the seek position. +/// /// Asserts the buffer was initialized with a capacity at least `@sizeOf(T)`. /// -/// Advances the seek position. -/// /// See also: -/// * `peekStruct` -/// * `takeStructEndian` -pub fn takeStruct(r: *Reader, comptime T: type) Error!*align(1) T { +/// * `peekStructReference` +/// * `takeStruct` +pub fn takeStructReference(r: *Reader, comptime T: type) Error!*align(1) T { // Only extern and packed structs have defined in-memory layout. comptime assert(@typeInfo(T).@"struct".layout != .auto); return @ptrCast(try r.takeArray(@sizeOf(T))); } +/// Obtains an unaligned pointer to the beginning of the stream, reinterpreted +/// as a pointer to the provided type, without advancing the seek position. +/// /// Asserts the buffer was initialized with a capacity at least `@sizeOf(T)`. /// -/// Does not advance the seek position. -/// /// See also: -/// * `takeStruct` -/// * `peekStructEndian` -pub fn peekStruct(r: *Reader, comptime T: type) Error!*align(1) T { +/// * `takeStructReference` +/// * `peekStruct` +pub fn peekStructReference(r: *Reader, comptime T: type) Error!*align(1) T { // Only extern and packed structs have defined in-memory layout. comptime assert(@typeInfo(T).@"struct".layout != .auto); return @ptrCast(try r.peekArray(@sizeOf(T))); @@ -1132,12 +1140,23 @@ pub fn peekStruct(r: *Reader, comptime T: type) Error!*align(1) T { /// when `endian` is comptime-known and matches the host endianness. /// /// See also: -/// * `takeStruct` -/// * `peekStructEndian` -pub inline fn takeStructEndian(r: *Reader, comptime T: type, endian: std.builtin.Endian) Error!T { - var res = (try r.takeStruct(T)).*; - if (native_endian != endian) std.mem.byteSwapAllFields(T, &res); - return res; +/// * `takeStructReference` +/// * `peekStruct` +pub inline fn takeStruct(r: *Reader, comptime T: type, endian: std.builtin.Endian) Error!T { + switch (@typeInfo(T)) { + .@"struct" => |info| switch (info.layout) { + .auto => @compileError("ill-defined memory layout"), + .@"extern" => { + var res = (try r.takeStructReference(T)).*; + if (native_endian != endian) std.mem.byteSwapAllFields(T, &res); + return res; + }, + .@"packed" => { + return takeInt(r, info.backing_integer.?, endian); + }, + }, + else => @compileError("not a struct"), + } } /// Asserts the buffer was initialized with a capacity at least `@sizeOf(T)`. @@ -1146,12 +1165,23 @@ pub inline fn takeStructEndian(r: *Reader, comptime T: type, endian: std.builtin /// when `endian` is comptime-known and matches the host endianness. /// /// See also: -/// * `takeStructEndian` -/// * `peekStruct` -pub inline fn peekStructEndian(r: *Reader, comptime T: type, endian: std.builtin.Endian) Error!T { - var res = (try r.peekStruct(T)).*; - if (native_endian != endian) std.mem.byteSwapAllFields(T, &res); - return res; +/// * `takeStruct` +/// * `peekStructReference` +pub inline fn peekStruct(r: *Reader, comptime T: type, endian: std.builtin.Endian) Error!T { + switch (@typeInfo(T)) { + .@"struct" => |info| switch (info.layout) { + .auto => @compileError("ill-defined memory layout"), + .@"extern" => { + var res = (try r.peekStructReference(T)).*; + if (native_endian != endian) std.mem.byteSwapAllFields(T, &res); + return res; + }, + .@"packed" => { + return peekInt(r, info.backing_integer.?, endian); + }, + }, + else => @compileError("not a struct"), + } } pub const TakeEnumError = Error || error{InvalidEnumTag}; @@ -1517,43 +1547,43 @@ test takeVarInt { try testing.expectError(error.EndOfStream, r.takeVarInt(u16, .little, 1)); } -test takeStruct { +test takeStructReference { var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); const S = extern struct { a: u8, b: u16 }; switch (native_endian) { - .little => try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.takeStruct(S)).*), - .big => try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.takeStruct(S)).*), + .little => try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.takeStructReference(S)).*), + .big => try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.takeStructReference(S)).*), } - try testing.expectError(error.EndOfStream, r.takeStruct(S)); + try testing.expectError(error.EndOfStream, r.takeStructReference(S)); +} + +test peekStructReference { + var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); + const S = extern struct { a: u8, b: u16 }; + switch (native_endian) { + .little => { + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.peekStructReference(S)).*); + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.peekStructReference(S)).*); + }, + .big => { + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.peekStructReference(S)).*); + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.peekStructReference(S)).*); + }, + } +} + +test takeStruct { + var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); + const S = extern struct { a: u8, b: u16 }; + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), try r.takeStruct(S, .big)); + try testing.expectError(error.EndOfStream, r.takeStruct(S, .little)); } test peekStruct { var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); const S = extern struct { a: u8, b: u16 }; - switch (native_endian) { - .little => { - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.peekStruct(S)).*); - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), (try r.peekStruct(S)).*); - }, - .big => { - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.peekStruct(S)).*); - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), (try r.peekStruct(S)).*); - }, - } -} - -test takeStructEndian { - var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); - const S = extern struct { a: u8, b: u16 }; - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), try r.takeStructEndian(S, .big)); - try testing.expectError(error.EndOfStream, r.takeStructEndian(S, .little)); -} - -test peekStructEndian { - var r: Reader = .fixed(&.{ 0x12, 0x00, 0x34, 0x56 }); - const S = extern struct { a: u8, b: u16 }; - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), try r.peekStructEndian(S, .big)); - try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), try r.peekStructEndian(S, .little)); + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x3456 }), try r.peekStruct(S, .big)); + try testing.expectEqual(@as(S, .{ .a = 0x12, .b = 0x5634 }), try r.peekStruct(S, .little)); } test takeEnum { diff --git a/lib/std/Io/Writer.zig b/lib/std/Io/Writer.zig index d931248c4d..163e7d7eea 100644 --- a/lib/std/Io/Writer.zig +++ b/lib/std/Io/Writer.zig @@ -796,16 +796,9 @@ pub inline fn writeInt(w: *Writer, comptime T: type, value: T, endian: std.built return w.writeAll(&bytes); } -pub fn writeStruct(w: *Writer, value: anytype) Error!void { - // Only extern and packed structs have defined in-memory layout. - comptime assert(@typeInfo(@TypeOf(value)).@"struct".layout != .auto); - return w.writeAll(std.mem.asBytes(&value)); -} - /// The function is inline to avoid the dead code in case `endian` is /// comptime-known and matches host endianness. -/// TODO: make sure this value is not a reference type -pub inline fn writeStructEndian(w: *Writer, value: anytype, endian: std.builtin.Endian) Error!void { +pub inline fn writeStruct(w: *Writer, value: anytype, endian: std.builtin.Endian) Error!void { switch (@typeInfo(@TypeOf(value))) { .@"struct" => |info| switch (info.layout) { .auto => @compileError("ill-defined memory layout"), diff --git a/src/Zcu.zig b/src/Zcu.zig index 6d07477edc..b2b0f71e32 100644 --- a/src/Zcu.zig +++ b/src/Zcu.zig @@ -2809,7 +2809,7 @@ pub fn loadZirCache(gpa: Allocator, cache_file: std.fs.File) !Zir { var buffer: [2000]u8 = undefined; var file_reader = cache_file.reader(&buffer); return result: { - const header = file_reader.interface.takeStruct(Zir.Header) catch |err| break :result err; + const header = file_reader.interface.takeStructReference(Zir.Header) catch |err| break :result err; break :result loadZirCacheBody(gpa, header.*, &file_reader.interface); } catch |err| switch (err) { error.ReadFailed => return file_reader.err.?, diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index d4a3d1598f..1be84d964f 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -349,7 +349,7 @@ fn loadZirZoirCache( const cache_br = &cache_fr.interface; // First we read the header to determine the lengths of arrays. - const header = (cache_br.takeStruct(Header) catch |err| switch (err) { + const header = (cache_br.takeStructReference(Header) catch |err| switch (err) { error.ReadFailed => return cache_fr.err.?, // This can happen if Zig bails out of this function between creating // the cached file and writing it.