ziglang / zig

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

std.fmt.Parser.peek() potential problem and fix therefore #20500

Open ephaeton opened 1 week ago

ephaeton commented 1 week ago

Zig Version

0.14.0-dev.144+a31fe8aa3

Steps to Reproduce and Observed Behavior

Use a fresh project output by zig init and replace src/main.zig with:

const std = @import("std");

pub fn main() !void {
    var buf: [64]u8 = undefined;
    const buf_slice = try std.fmt.bufPrint(&buf, "{s}", .{".dot"});
    const view = try std.unicode.Utf8View.init(buf_slice);
    var parser = std.fmt.Parser{
        .buf = buf_slice,
        .iter = view.iterator(),
    };
    if (parser.maybe('.')) {
        std.debug.print("leading dot!\n", .{});
    }
}

When I try and compile this (with 0.12.0, 0.13.0, above cited 0.14.0 version), I get the following error:

zig build
install
└─ install x
   └─ zig build-exe x Debug native 1 errors
/home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/lib/std/fmt.zig:377:13: error: variable of type 'comptime_int' must be const or comptime
        var i = 0;
            ^
/home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/lib/std/fmt.zig:377:13: note: to modify this variable at runtime, it must be given an explicit fixed-size number type
referenced by:
    maybe: /home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/lib/std/fmt.zig:348:17
    format__anon_3615: /home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/lib/std/fmt.zig:358:17
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
error: the following command failed with 1 compilation errors:
/home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/zig build-exe -ODebug -Mroot=/tmp/x/src/main.zig --cache-dir /tmp/x/.zig-cache --global-cache-dir /home/phaeton/.cache/zig --name x --listen=- 
Build Summary: 2/5 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install x transitive failure
   └─ zig build-exe x Debug native 1 errors
error: the following build command failed with exit code 1:
/tmp/x/.zig-cache/o/f3f1c19b82a7c635149145fba3613e07/build /home/phaeton/.local/share/ziege/pkg/zig/0.14.0-dev.144+a31fe8aa3/zig /tmp/x /tmp/x/.zig-cache /home/phaeton/.cache/zig --seed 0xc5d044ce -Z7886857364005c98

You can see I'm using ziege, but that shouldn't matter, should it? I don't understand how this issue hasn't surfaced before, which makes me think it's a problem on my end. Then again, the code (see below) is the same in the repo as in my ziege managed installation. If you agree it's a generally applicable problem, fret not, the fix is trivial.

Expected Behavior

The expectation is for this to compile (and work). We can track it down to the type information at https://github.com/ziglang/zig/blob/768b17755e7735b328b92212de2dd7018f78fb4b/lib/std/fmt.zig#L377 just as the compiler says - make i not a comptime_int and the project compiles.

Does a 7-byte change merit a PR?

--- fmt_old.zig 2024-07-04 13:51:02.206760285 +0200
+++ fmt.zig 2024-07-04 13:51:11.250041745 +0200
@@ -374,7 +374,7 @@
         const original_i = self.iter.i;
         defer self.iter.i = original_i;

-        var i = 0;
+        var i: usize = 0;
         var code_point: ?u21 = null;
         while (i <= n) : (i += 1) {
             code_point = self.iter.nextCodepoint();

I don't understand the logic behind it, though; observationally I can tell you that fixes "the" issue.

scottredig commented 1 week ago

The breaking change was https://github.com/ziglang/zig/pull/18533

This parser is for use in decoding formatting strings. Since this is usually done at comptime, the usage of comptime_int usually works fine. If you're not working with formatting strings, you should probably be using your own parsing code. If you are working with format strings, you should be parsing them at comptime so you can validate them, if possible.

That said, there doesn't appear to be a reason for this code to not work at runtime.