From ba774c5697e5dcdf0f0676e2077a3034c310baf3 Mon Sep 17 00:00:00 2001 From: tgschultz Date: Wed, 3 Apr 2019 15:47:46 +0000 Subject: [PATCH 1/2] (De)serializer now uses enum instead of bool to determine packing mode (byte/bit). Optional is initialized in a more straight-forward way by deserializer. --- std/io.zig | 50 +++++++++++++++++++++++-------------------- std/io_test.zig | 56 ++++++++++++++++++++++++------------------------- 2 files changed, 55 insertions(+), 51 deletions(-) diff --git a/std/io.zig b/std/io.zig index b5071be385..7f153c37cd 100644 --- a/std/io.zig +++ b/std/io.zig @@ -1088,6 +1088,11 @@ test "io.readLineSliceFrom" { testing.expectError(error.OutOfMemory, readLineSliceFrom(stream, buf[0..])); } +pub const Packing = enum { + Byte, /// Pack data to byte alignment + Bit, /// Pack data to bit alignment +}; + /// Creates a deserializer that deserializes types from any stream. /// If `is_packed` is true, the data stream is treated as bit-packed, /// otherwise data is expected to be packed to the smallest byte. @@ -1097,18 +1102,18 @@ test "io.readLineSliceFrom" { /// which will be called when the deserializer is used to deserialize /// that type. It will pass a pointer to the type instance to deserialize /// into and a pointer to the deserializer struct. -pub fn Deserializer(comptime endian: builtin.Endian, is_packed: bool, comptime Error: type) type { +pub fn Deserializer(comptime endian: builtin.Endian, comptime packing: Packing, comptime Error: type) type { return struct { const Self = @This(); - in_stream: if (is_packed) BitInStream(endian, Stream.Error) else *Stream, + in_stream: if (packing == .Bit) BitInStream(endian, Stream.Error) else *Stream, pub const Stream = InStream(Error); pub fn init(in_stream: *Stream) Self { - return Self{ .in_stream = switch (is_packed) { - true => BitInStream(endian, Stream.Error).init(in_stream), - else => in_stream, + return Self{ .in_stream = switch (packing) { + .Bit => BitInStream(endian, Stream.Error).init(in_stream), + .Byte => in_stream, } }; } @@ -1128,7 +1133,7 @@ pub fn Deserializer(comptime endian: builtin.Endian, is_packed: bool, comptime E const Log2U = math.Log2Int(U); const int_size = (U.bit_count + 7) / 8; - if (is_packed) { + if (packing == .Bit) { const result = try self.in_stream.readBitsNoEof(U, t_bit_count); return @bitCast(T, result); } @@ -1211,8 +1216,8 @@ pub fn Deserializer(comptime endian: builtin.Endian, is_packed: bool, comptime E //custom deserializer: fn(self: *Self, deserializer: var) !void if (comptime trait.hasFn("deserialize")(C)) return C.deserialize(ptr, self); - if (comptime trait.isPacked(C) and !is_packed) { - var packed_deserializer = Deserializer(endian, true, Error).init(self.in_stream); + if (comptime trait.isPacked(C) and packing != .Bit) { + var packed_deserializer = Deserializer(endian, .Bit, Error).init(self.in_stream); return packed_deserializer.deserializeInto(ptr); } @@ -1276,14 +1281,13 @@ pub fn Deserializer(comptime endian: builtin.Endian, is_packed: bool, comptime E ptr.* = null; return; } - + + //This should ensure that the optional is set to non-null. + ptr.* = OC(undefined); //The way non-pointer optionals are implemented ensures a pointer to them // will point to the value. The flag is stored at the end of that data. var val_ptr = @ptrCast(*OC, ptr); try self.deserializeInto(val_ptr); - //This bit ensures the null flag isn't set. Any actual copying should be - // optimized out... I hope. - ptr.* = val_ptr.*; }, builtin.TypeId.Enum => { var value = try self.deserializeInt(@TagType(C)); @@ -1310,24 +1314,24 @@ pub fn Deserializer(comptime endian: builtin.Endian, is_packed: bool, comptime E /// which will be called when the serializer is used to serialize that type. It will /// pass a const pointer to the type instance to be serialized and a pointer /// to the serializer struct. -pub fn Serializer(comptime endian: builtin.Endian, comptime is_packed: bool, comptime Error: type) type { +pub fn Serializer(comptime endian: builtin.Endian, comptime packing: Packing, comptime Error: type) type { return struct { const Self = @This(); - out_stream: if (is_packed) BitOutStream(endian, Stream.Error) else *Stream, + out_stream: if (packing == .Bit) BitOutStream(endian, Stream.Error) else *Stream, pub const Stream = OutStream(Error); pub fn init(out_stream: *Stream) Self { - return Self{ .out_stream = switch (is_packed) { - true => BitOutStream(endian, Stream.Error).init(out_stream), - else => out_stream, + return Self{ .out_stream = switch (packing) { + .Bit => BitOutStream(endian, Stream.Error).init(out_stream), + .Byte => out_stream, } }; } /// Flushes any unwritten bits to the stream pub fn flush(self: *Self) Error!void { - if (is_packed) return self.out_stream.flushBits(); + if (packing == .Bit) return self.out_stream.flushBits(); } fn serializeInt(self: *Self, value: var) Error!void { @@ -1343,15 +1347,15 @@ pub fn Serializer(comptime endian: builtin.Endian, comptime is_packed: bool, com const u_value = @bitCast(U, value); - if (is_packed) return self.out_stream.writeBits(u_value, t_bit_count); + if (packing == .Bit) return self.out_stream.writeBits(u_value, t_bit_count); var buffer: [int_size]u8 = undefined; if (int_size == 1) buffer[0] = u_value; for (buffer) |*byte, i| { const idx = switch (endian) { - builtin.Endian.Big => int_size - i - 1, - builtin.Endian.Little => i, + .Big => int_size - i - 1, + .Little => i, }; const shift = @intCast(Log2U, idx * u8_bit_count); const v = u_value >> shift; @@ -1374,8 +1378,8 @@ pub fn Serializer(comptime endian: builtin.Endian, comptime is_packed: bool, com //custom serializer: fn(self: Self, serializer: var) !void if (comptime trait.hasFn("serialize")(T)) return T.serialize(value, self); - if (comptime trait.isPacked(T) and !is_packed) { - var packed_serializer = Serializer(endian, true, Error).init(self.out_stream); + if (comptime trait.isPacked(T) and packing != .Bit) { + var packed_serializer = Serializer(endian, .Bit, Error).init(self.out_stream); try packed_serializer.serialize(value); try packed_serializer.flush(); return; diff --git a/std/io_test.zig b/std/io_test.zig index 58d542307c..d6f2264a56 100644 --- a/std/io_test.zig +++ b/std/io_test.zig @@ -318,7 +318,7 @@ test "BitStreams with File Stream" { try os.deleteFile(tmp_file_name); } -fn testIntSerializerDeserializer(comptime endian: builtin.Endian, comptime is_packed: bool) !void { +fn testIntSerializerDeserializer(comptime endian: builtin.Endian, comptime packing: io.Packing) !void { //@NOTE: if this test is taking too long, reduce the maximum tested bitsize const max_test_bitsize = 128; @@ -333,12 +333,12 @@ fn testIntSerializerDeserializer(comptime endian: builtin.Endian, comptime is_pa var out = io.SliceOutStream.init(data_mem[0..]); const OutError = io.SliceOutStream.Error; var out_stream = &out.stream; - var serializer = io.Serializer(endian, is_packed, OutError).init(out_stream); + var serializer = io.Serializer(endian, packing, OutError).init(out_stream); var in = io.SliceInStream.init(data_mem[0..]); const InError = io.SliceInStream.Error; var in_stream = &in.stream; - var deserializer = io.Deserializer(endian, is_packed, InError).init(in_stream); + var deserializer = io.Deserializer(endian, packing, InError).init(in_stream); comptime var i = 0; inline while (i <= max_test_bitsize) : (i += 1) { @@ -366,21 +366,21 @@ fn testIntSerializerDeserializer(comptime endian: builtin.Endian, comptime is_pa const extra_packed_byte = @boolToInt(total_bits % u8_bit_count > 0); const total_packed_bytes = (total_bits / u8_bit_count) + extra_packed_byte; - expect(in.pos == if (is_packed) total_packed_bytes else total_bytes); + expect(in.pos == if (packing == .Bit) total_packed_bytes else total_bytes); //Verify that empty error set works with serializer. //deserializer is covered by SliceInStream const NullError = io.NullOutStream.Error; var null_out = io.NullOutStream.init(); var null_out_stream = &null_out.stream; - var null_serializer = io.Serializer(endian, is_packed, NullError).init(null_out_stream); + var null_serializer = io.Serializer(endian, packing, NullError).init(null_out_stream); try null_serializer.serialize(data_mem[0..]); try null_serializer.flush(); } test "Serializer/Deserializer Int" { - try testIntSerializerDeserializer(builtin.Endian.Big, false); - try testIntSerializerDeserializer(builtin.Endian.Little, false); + try testIntSerializerDeserializer(.Big, .Byte); + try testIntSerializerDeserializer(.Little, .Byte); // TODO these tests are disabled due to tripping an LLVM assertion // https://github.com/ziglang/zig/issues/2019 //try testIntSerializerDeserializer(builtin.Endian.Big, true); @@ -389,7 +389,7 @@ test "Serializer/Deserializer Int" { fn testIntSerializerDeserializerInfNaN( comptime endian: builtin.Endian, - comptime is_packed: bool, + comptime packing: io.Packing, ) !void { const mem_size = (16 * 2 + 32 * 2 + 64 * 2 + 128 * 2) / comptime meta.bitCount(u8); var data_mem: [mem_size]u8 = undefined; @@ -397,12 +397,12 @@ fn testIntSerializerDeserializerInfNaN( var out = io.SliceOutStream.init(data_mem[0..]); const OutError = io.SliceOutStream.Error; var out_stream = &out.stream; - var serializer = io.Serializer(endian, is_packed, OutError).init(out_stream); + var serializer = io.Serializer(endian, packing, OutError).init(out_stream); var in = io.SliceInStream.init(data_mem[0..]); const InError = io.SliceInStream.Error; var in_stream = &in.stream; - var deserializer = io.Deserializer(endian, is_packed, InError).init(in_stream); + var deserializer = io.Deserializer(endian, packing, InError).init(in_stream); //@TODO: isInf/isNan not currently implemented for f128. try serializer.serialize(std.math.nan(f16)); @@ -432,17 +432,17 @@ fn testIntSerializerDeserializerInfNaN( } test "Serializer/Deserializer Int: Inf/NaN" { - try testIntSerializerDeserializerInfNaN(builtin.Endian.Big, false); - try testIntSerializerDeserializerInfNaN(builtin.Endian.Little, false); - try testIntSerializerDeserializerInfNaN(builtin.Endian.Big, true); - try testIntSerializerDeserializerInfNaN(builtin.Endian.Little, true); + try testIntSerializerDeserializerInfNaN(.Big, .Byte); + try testIntSerializerDeserializerInfNaN(.Little, .Byte); + try testIntSerializerDeserializerInfNaN(.Big, .Bit); + try testIntSerializerDeserializerInfNaN(.Little, .Bit); } fn testAlternateSerializer(self: var, serializer: var) !void { try serializer.serialize(self.f_f16); } -fn testSerializerDeserializer(comptime endian: builtin.Endian, comptime is_packed: bool) !void { +fn testSerializerDeserializer(comptime endian: builtin.Endian, comptime packing: io.Packing) !void { const ColorType = enum(u4) { RGB8 = 1, RA16 = 2, @@ -529,12 +529,12 @@ fn testSerializerDeserializer(comptime endian: builtin.Endian, comptime is_packe var out = io.SliceOutStream.init(data_mem[0..]); const OutError = io.SliceOutStream.Error; var out_stream = &out.stream; - var serializer = io.Serializer(endian, is_packed, OutError).init(out_stream); + var serializer = io.Serializer(endian, packing, OutError).init(out_stream); var in = io.SliceInStream.init(data_mem[0..]); const InError = io.SliceInStream.Error; var in_stream = &in.stream; - var deserializer = io.Deserializer(endian, is_packed, InError).init(in_stream); + var deserializer = io.Deserializer(endian, packing, InError).init(in_stream); try serializer.serialize(my_inst); @@ -543,13 +543,13 @@ fn testSerializerDeserializer(comptime endian: builtin.Endian, comptime is_packe } test "Serializer/Deserializer generic" { - try testSerializerDeserializer(builtin.Endian.Big, false); - try testSerializerDeserializer(builtin.Endian.Little, false); - try testSerializerDeserializer(builtin.Endian.Big, true); - try testSerializerDeserializer(builtin.Endian.Little, true); + try testSerializerDeserializer(builtin.Endian.Big, .Byte); + try testSerializerDeserializer(builtin.Endian.Little, .Byte); + try testSerializerDeserializer(builtin.Endian.Big, .Bit); + try testSerializerDeserializer(builtin.Endian.Little, .Bit); } -fn testBadData(comptime endian: builtin.Endian, comptime is_packed: bool) !void { +fn testBadData(comptime endian: builtin.Endian, comptime packing: io.Packing) !void { const E = enum(u14) { One = 1, Two = 2, @@ -568,12 +568,12 @@ fn testBadData(comptime endian: builtin.Endian, comptime is_packed: bool) !void var out = io.SliceOutStream.init(data_mem[0..]); const OutError = io.SliceOutStream.Error; var out_stream = &out.stream; - var serializer = io.Serializer(endian, is_packed, OutError).init(out_stream); + var serializer = io.Serializer(endian, packing, OutError).init(out_stream); var in = io.SliceInStream.init(data_mem[0..]); const InError = io.SliceInStream.Error; var in_stream = &in.stream; - var deserializer = io.Deserializer(endian, is_packed, InError).init(in_stream); + var deserializer = io.Deserializer(endian, packing, InError).init(in_stream); try serializer.serialize(u14(3)); expectError(error.InvalidEnumTag, deserializer.deserialize(A)); @@ -584,8 +584,8 @@ fn testBadData(comptime endian: builtin.Endian, comptime is_packed: bool) !void } test "Deserializer bad data" { - try testBadData(builtin.Endian.Big, false); - try testBadData(builtin.Endian.Little, false); - try testBadData(builtin.Endian.Big, true); - try testBadData(builtin.Endian.Little, true); + try testBadData(.Big, .Byte); + try testBadData(.Little, .Byte); + try testBadData(.Big, .Bit); + try testBadData(.Little, .Bit); } From fe33d8ea146429af7db514621e25870508975d62 Mon Sep 17 00:00:00 2001 From: tgschultz Date: Wed, 3 Apr 2019 20:05:24 +0000 Subject: [PATCH 2/2] Changes as suggested by andrewrk --- std/io.zig | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/std/io.zig b/std/io.zig index 7f153c37cd..afbd8198fd 100644 --- a/std/io.zig +++ b/std/io.zig @@ -1272,7 +1272,7 @@ pub fn Deserializer(comptime endian: builtin.Endian, comptime packing: Packing, return error.InvalidEnumTag; } @compileError("Cannot meaningfully deserialize " ++ @typeName(C) ++ - " because it is an untagged union Use a custom deserialize()."); + " because it is an untagged union. Use a custom deserialize()."); }, builtin.TypeId.Optional => { const OC = comptime meta.Child(C); @@ -1282,11 +1282,8 @@ pub fn Deserializer(comptime endian: builtin.Endian, comptime packing: Packing, return; } - //This should ensure that the optional is set to non-null. - ptr.* = OC(undefined); - //The way non-pointer optionals are implemented ensures a pointer to them - // will point to the value. The flag is stored at the end of that data. - var val_ptr = @ptrCast(*OC, ptr); + ptr.* = OC(undefined); //make it non-null so the following .? is guaranteed safe + const val_ptr = &ptr.*.?; try self.deserializeInto(val_ptr); }, builtin.TypeId.Enum => { @@ -1426,7 +1423,7 @@ pub fn Serializer(comptime endian: builtin.Endian, comptime packing: Packing, co unreachable; } @compileError("Cannot meaningfully serialize " ++ @typeName(T) ++ - " because it is an untagged union Use a custom serialize()."); + " because it is an untagged union. Use a custom serialize()."); }, builtin.TypeId.Optional => { if (value == null) { @@ -1436,10 +1433,7 @@ pub fn Serializer(comptime endian: builtin.Endian, comptime packing: Packing, co try self.serializeInt(u1(@boolToInt(true))); const OC = comptime meta.Child(T); - - //The way non-pointer optionals are implemented ensures a pointer to them - // will point to the value. The flag is stored at the end of that data. - var val_ptr = @ptrCast(*const OC, &value); + const val_ptr = &value.?; try self.serialize(val_ptr.*); }, builtin.TypeId.Enum => {