From bb9a4ad6e903ad2f9c137bca56bdf2dd6fc65d03 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 16 Sep 2020 13:45:54 +0200 Subject: [PATCH 1/3] std: Fix {*} printing of non-pointer types Fixes a regression introduced in #6246. Adds a test to make sure this won't happen again. --- lib/std/fmt.zig | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/std/fmt.zig b/lib/std/fmt.zig index 8d31733959..b18c9f2f44 100644 --- a/lib/std/fmt.zig +++ b/lib/std/fmt.zig @@ -327,7 +327,7 @@ pub fn formatType( max_depth: usize, ) @TypeOf(writer).Error!void { if (comptime std.mem.eql(u8, fmt, "*")) { - try writer.writeAll(@typeName(@typeInfo(@TypeOf(value)).Pointer.child)); + try writer.writeAll(@typeName(std.meta.Child(@TypeOf(value)))); try writer.writeAll("@"); try formatInt(@ptrToInt(value), 16, false, FormatOptions{}, writer); return; @@ -1246,6 +1246,10 @@ test "optional" { const value: ?i32 = null; try testFmt("optional: null\n", "optional: {}\n", .{value}); } + { + const value = @intToPtr(?*i32, 0xf000d000); + try testFmt("optional: *i32@f000d000\n", "optional: {*}\n", .{value}); + } } test "error" { From 27adb82fda516e95c3c03df80a95b895969fdd56 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 17 Sep 2020 00:41:26 +0200 Subject: [PATCH 2/3] std: Respect user-specified alignment when formatting ints This implementation tries to do the right thing (TM) by treating the sign as part of the number itself, therefore the alignment parameter applies to both the sign and the digits. In other words the format string `{:>4}` with -1 as input will not output `- 1` but ` -1`. And let's default to right alignment for everything as that's what users want, especially when printing numbers. Many implementations use different defaults for numeric vs non-numeric types, let's strive for a consistent behaviour here. --- lib/std/fmt.zig | 133 +++++++++++++++++++----------------------------- 1 file changed, 51 insertions(+), 82 deletions(-) diff --git a/lib/std/fmt.zig b/lib/std/fmt.zig index b18c9f2f44..56a1aba217 100644 --- a/lib/std/fmt.zig +++ b/lib/std/fmt.zig @@ -22,7 +22,7 @@ pub const Alignment = enum { pub const FormatOptions = struct { precision: ?usize = null, width: ?usize = null, - alignment: Alignment = .Left, + alignment: Alignment = .Right, fill: u8 = ' ', }; @@ -631,26 +631,22 @@ pub fn formatBuf( writer: anytype, ) !void { const width = options.width orelse buf.len; - var padding = if (width > buf.len) (width - buf.len) else 0; - const pad_byte = [1]u8{options.fill}; + const padding = if (width > buf.len) (width - buf.len) else 0; + switch (options.alignment) { .Left => { try writer.writeAll(buf); - while (padding > 0) : (padding -= 1) { - try writer.writeAll(&pad_byte); - } + try writer.writeByteNTimes(options.fill, padding); }, .Center => { - const padl = padding / 2; - var i: usize = 0; - while (i < padl) : (i += 1) try writer.writeAll(&pad_byte); + const left_padding = padding / 2; + const right_padding = (padding + 1) / 2; + try writer.writeByteNTimes(options.fill, left_padding); try writer.writeAll(buf); - while (i < padding) : (i += 1) try writer.writeAll(&pad_byte); + try writer.writeByteNTimes(options.fill, right_padding); }, .Right => { - while (padding > 0) : (padding -= 1) { - try writer.writeAll(&pad_byte); - } + try writer.writeByteNTimes(options.fill, padding); try writer.writeAll(buf); }, } @@ -941,61 +937,27 @@ pub fn formatInt( options: FormatOptions, writer: anytype, ) !void { + assert(base >= 2); + const int_value = if (@TypeOf(value) == comptime_int) blk: { const Int = math.IntFittingRange(value, value); break :blk @as(Int, value); } else value; - if (@typeInfo(@TypeOf(int_value)).Int.is_signed) { - return formatIntSigned(int_value, base, uppercase, options, writer); - } else { - return formatIntUnsigned(int_value, base, uppercase, options, writer); - } -} + const value_info = @typeInfo(@TypeOf(int_value)).Int; -fn formatIntSigned( - value: anytype, - base: u8, - uppercase: bool, - options: FormatOptions, - writer: anytype, -) !void { - const new_options = FormatOptions{ - .width = if (options.width) |w| (if (w == 0) 0 else w - 1) else null, - .precision = options.precision, - .fill = options.fill, - }; - const bit_count = @typeInfo(@TypeOf(value)).Int.bits; - const Uint = std.meta.Int(false, bit_count); - if (value < 0) { - try writer.writeAll("-"); - const new_value = math.absCast(value); - return formatIntUnsigned(new_value, base, uppercase, new_options, writer); - } else if (options.width == null or options.width.? == 0) { - return formatIntUnsigned(@intCast(Uint, value), base, uppercase, options, writer); - } else { - try writer.writeAll("+"); - const new_value = @intCast(Uint, value); - return formatIntUnsigned(new_value, base, uppercase, new_options, writer); - } -} + // The type must have the same size as `base` or be wider in order for the + // division to work + const min_int_bits = comptime math.max(value_info.bits, 8); + const MinInt = std.meta.Int(false, min_int_bits); -fn formatIntUnsigned( - value: anytype, - base: u8, - uppercase: bool, - options: FormatOptions, - writer: anytype, -) !void { - assert(base >= 2); - const value_info = @typeInfo(@TypeOf(value)).Int; - var buf: [math.max(value_info.bits, 1)]u8 = undefined; - const min_int_bits = comptime math.max(value_info.bits, @typeInfo(@TypeOf(base)).Int.bits); - const MinInt = std.meta.Int(value_info.is_signed, min_int_bits); - var a: MinInt = value; + const abs_value = math.absCast(int_value); + // The worst case in terms of space needed is base 2, plus 1 for the sign + var buf: [1 + math.max(value_info.bits, 1)]u8 = undefined; + + var a: MinInt = abs_value; var index: usize = buf.len; - while (true) { const digit = a % base; index -= 1; @@ -1004,25 +966,21 @@ fn formatIntUnsigned( if (a == 0) break; } - const digits_buf = buf[index..]; - const width = options.width orelse 0; - const padding = if (width > digits_buf.len) (width - digits_buf.len) else 0; - - if (padding > index) { - const zero_byte: u8 = options.fill; - var leftover_padding = padding - index; - while (true) { - try writer.writeAll(@as(*const [1]u8, &zero_byte)[0..]); - leftover_padding -= 1; - if (leftover_padding == 0) break; + if (value_info.is_signed) { + if (value < 0) { + // Negative integer + index -= 1; + buf[index] = '-'; + } else if (options.width == null or options.width.? == 0) { + // Positive integer, omit the plus sign + } else { + // Positive integer + index -= 1; + buf[index] = '+'; } - mem.set(u8, buf[0..index], options.fill); - return writer.writeAll(&buf); - } else { - const padded_buf = buf[index - padding ..]; - mem.set(u8, padded_buf[0..padding], options.fill); - return writer.writeAll(padded_buf); } + + return formatBuf(buf[index..], options, writer); } pub fn formatIntBuf(out_buf: []u8, value: anytype, base: u8, uppercase: bool, options: FormatOptions) usize { @@ -1287,7 +1245,17 @@ test "int.specifier" { test "int.padded" { try testFmt("u8: ' 1'", "u8: '{:4}'", .{@as(u8, 1)}); - try testFmt("u8: 'xxx1'", "u8: '{:x<4}'", .{@as(u8, 1)}); + try testFmt("u8: '1000'", "u8: '{:0<4}'", .{@as(u8, 1)}); + try testFmt("u8: '0001'", "u8: '{:0>4}'", .{@as(u8, 1)}); + try testFmt("u8: '0100'", "u8: '{:0^4}'", .{@as(u8, 1)}); + try testFmt("i8: '-1 '", "i8: '{:<4}'", .{@as(i8, -1)}); + try testFmt("i8: ' -1'", "i8: '{:>4}'", .{@as(i8, -1)}); + try testFmt("i8: ' -1 '", "i8: '{:^4}'", .{@as(i8, -1)}); + try testFmt("i16: '-1234'", "i16: '{:4}'", .{@as(i16, -1234)}); + try testFmt("i16: '+1234'", "i16: '{:4}'", .{@as(i16, 1234)}); + try testFmt("i16: '-12345'", "i16: '{:4}'", .{@as(i16, -12345)}); + try testFmt("i16: '+12345'", "i16: '{:4}'", .{@as(i16, 12345)}); + try testFmt("u16: '12345'", "u16: '{:4}'", .{@as(u16, 12345)}); } test "buffer" { @@ -1333,7 +1301,7 @@ test "slice" { try testFmt("slice: []const u8@deadbeef\n", "slice: {}\n", .{value}); } - try testFmt("buf: Test \n", "buf: {s:5}\n", .{"Test"}); + try testFmt("buf: Test\n", "buf: {s:5}\n", .{"Test"}); try testFmt("buf: Test\n Other text", "buf: {s}\n Other text", .{"Test"}); } @@ -1366,7 +1334,7 @@ test "cstr" { .{@ptrCast([*c]const u8, "Test C")}, ); try testFmt( - "cstr: Test C \n", + "cstr: Test C\n", "cstr: {s:10}\n", .{@ptrCast([*c]const u8, "Test C")}, ); @@ -1809,7 +1777,7 @@ test "vector" { try testFmt("{ true, false, true, false }", "{}", .{vbool}); try testFmt("{ -2, -1, 0, 1 }", "{}", .{vi64}); - try testFmt("{ - 2, - 1, + 0, + 1 }", "{d:5}", .{vi64}); + try testFmt("{ -2, -1, +0, +1 }", "{d:5}", .{vi64}); try testFmt("{ 1000, 2000, 3000, 4000 }", "{}", .{vu64}); try testFmt("{ 3e8, 7d0, bb8, fa0 }", "{x}", .{vu64}); try testFmt("{ 1kB, 2kB, 3kB, 4kB }", "{B}", .{vu64}); @@ -1822,15 +1790,16 @@ test "enum-literal" { test "padding" { try testFmt("Simple", "{}", .{"Simple"}); - try testFmt("true ", "{:10}", .{true}); + try testFmt(" true", "{:10}", .{true}); try testFmt(" true", "{:>10}", .{true}); try testFmt("======true", "{:=>10}", .{true}); try testFmt("true======", "{:=<10}", .{true}); try testFmt(" true ", "{:^10}", .{true}); try testFmt("===true===", "{:=^10}", .{true}); - try testFmt("Minimum width", "{:18} width", .{"Minimum"}); + try testFmt(" Minimum width", "{:18} width", .{"Minimum"}); try testFmt("==================Filled", "{:=>24}", .{"Filled"}); try testFmt(" Centered ", "{:^24}", .{"Centered"}); + try testFmt("-", "{:-^1}", .{""}); } test "decimal float padding" { From b7f9f779afa6de38f7599d8ffd9937fb48b9284c Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 17 Sep 2020 09:27:24 +0200 Subject: [PATCH 3/3] translate-c: Fix formatting of non-printable chars The two octets in hex notation must be aligned to the right and padded on the left, not the other way around. --- src-self-hosted/translate_c.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index 1cdb5099e1..a5619d56fe 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -2032,7 +2032,7 @@ fn escapeChar(c: u8, char_buf: *[4]u8) []const u8 { // Handle the remaining escapes Zig doesn't support by turning them // into their respective hex representation else => if (std.ascii.isCntrl(c)) - std.fmt.bufPrint(char_buf, "\\x{x:0<2}", .{c}) catch unreachable + std.fmt.bufPrint(char_buf, "\\x{x:0>2}", .{c}) catch unreachable else std.fmt.bufPrint(char_buf, "{c}", .{c}) catch unreachable, };