ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.84k stars 2.55k forks source link

std.fmt.formatInt produces wrong strings depending on signedness of integers #14436

Open Daimanta opened 1 year ago

Daimanta commented 1 year ago

Zig Version

0.10.1

Steps to Reproduce and Observed Behavior

Depending on the signedness of the integertype, an integer formatted with a minimal width will include a '+' sign in an incorrect place. Disabling this '+' is not possible. Example code for comparison:

     const std2 = @import("std");
     var buffer: [20]u8 = undefined;
     const len = std2.fmt.formatIntBuf(&buffer, @as(i64, 3), 10, .lower, .{.fill = '0', .width = 4});
     std2.debug.print("{s}\n", .{buffer[0..len]});

     const len2 = std2.fmt.formatIntBuf(&buffer, @as(u64, 3), 10, .lower, .{.fill = '0', .width = 4});
     std2.debug.print("{s}\n", .{buffer[0..len2]});

Expected Behavior

In both cases I would expect a printed string "0003". However, in the case of the value 3 as typed i64, we get the string "00+3". This is obviously a wrong string. In my opinion there are two issues with the "00+3" output. First of all, if a plus sign is included it should be at the start, i.e. "+003". Second of all, the '+' sign should be an optional character. The c formatting specification has an option for an explicit '+' sign for numerical values. This should be the case for the Zig stdlib as well. In most cases, the '+' is not desired behavior, as we do not want to explicitly indicate that we are talking about a positive number. Therefore, '+" should be removed from default options and added only if explicitly indicated.

matu3ba commented 1 year ago
pub fn main() void {
    const std2 = @import("std");
    var buffer: [20]u8 = undefined;
    const len = std2.fmt.formatIntBuf(&buffer, @as(i64, 3), 10, .lower, .{ .fill = '0', .width = 4 });
    std2.debug.print("{s}\n", .{buffer[0..len]});

    const len2 = std2.fmt.formatIntBuf(&buffer, @as(u64, 3), 10, .lower, .{ .fill = '0', .width = 4 });
    std2.debug.print("{s}\n", .{buffer[0..len2]});
}

out:

$ zig run printfail.zig
00+3
0003
chrboesch commented 1 year ago

I can add an option to print the sign at positive integers just like in C printf.

achan1989 commented 1 month ago

It's difficult to know what the correct behaviour should be in the general case.

In C printf the only fill options are space or zero. With space, the sign is kept next to the value, e.g. | -3|. With zero, the sign is separated from the value, e.g. |-003|.

Zig currently keeps the sign next to the value in all cases. We could modify it to replicate the printf behaviour when .fill = '0', which probably matches what most people want from int formatting.

But what should we do with an arbitrary fill character? The user may intend either behaviour. Currently there is no way to specify which behaviour to use (and is it even worth cluttering the API with such a niche option?). We could assume one behaviour for now.

Options for going forward:

  1. Modify formatInt. If the fill character is not default_fill_char then separate the sign from the value. Otherwise keep the sign next to the value.
  2. Modify formatInt. If the fill character is '0' then separate the sign from the value. Otherwise keep the sign next to the value.
  3. Do nothing until formatting design is overhauled (#20152, #19488 etc).