fix: disallow leading zeroes in int literals and int types

[wooster0]

This makes 0123 and u0123 etc. illegal.

I'm now confident that this is a good change because
I actually caught two C header translation mistakes in haiku.zig with this.
Clearly, 0123 being octal in C (TIL) can cause confusion, and we avoid ambiguity by
requiring 0o as the prefix and now also disallowing leading zeroes in integers.

For consistency and because it looks weird, we disallow it for integer types too (e.g. u0123).

Fixes #11963
Fixes #12417

[tiehuis]

As an aside, the current implementation of parseInt and parseFloat in std accept leading zeroes. May want to do a follow-up to this to modify their behavior if we are wanting consistency.

[marler8997]

This brings me back. I'll share the story for those interested. At my old job we were sometimes getting weird file permissions. It was a very large code base (millions of LOC in hundreds of thousands of files) and was hard to reproduce. Various people had spent days trying to reproduce/debug this issue but it's difficult when there's so much code to sift through. I thought it could be some sort of uninitialized value issue but I was able to determine that each time it happened the permission bits were consistent. After looking at the bit pattern of the permissions, it dawned on me that a user probably entered a hex integer literal when it was meant to be octal (i.e. they did something like 0x755 instead of 0755). I took the bit pattern and decoded it into a hex integer literal and used grep to search our repositories (probably took 5 or 10 minutes) and sure enough, I got a hit for a set of permission bits being passed to an open call! Changed that 0x to an 0 and problem solved.

[wooster0]

Wow, thanks for sharing! That really sounds like a pain. So much time spent for such a simple fix. Luckily with Zig this should never happen again!

[motiejus]

I had exact this issue myself a couple of weeks ago in my zig codebase, and thought about suggesting this just a few days ago. @r00ster91 you got it , thanks.

[wooster0]

@tiehuis good point! I opened #12446.

[Vexu]

a.zig:1:11: error: expected expression, found 'invalid bytes'
const a = 00123;
          ^~~~~
a.zig:1:16: note: invalid byte: ';'
const a = 00123;
               ^

.. is a terrible compile error. This either needs a custom error in Ast.zig or, preferably an error in AstGen.zig since this doesn't prevent us from parsing properly.

[wooster0]

I agree. I thought it was very confusing too but then I noticed that we actually already have this problem with many other, similar cases too:

pub fn main() void {
    _ = 1_;
}

x.zig:4:9: error: expected expression, found 'invalid bytes'
    _ = 1_;
        ^~
x.zig:4:11: note: invalid byte: ';'
    _ = 1_;
          ^

pub fn main() void {
    _ = 1__1;
}

x.zig:4:9: error: expected expression, found 'invalid bytes'
    _ = 1__1;
        ^~
x.zig:4:11: note: invalid byte: '_'
    _ = 1__1;
          ^

I think it is very much part of the lump you will find when you search for "underscore_placement" on https://github.com/ziglang/zig/find/master

As you can see, they all expect the // :2:21: error: expected expression, found 'invalid bytes' error too, which I also find could probably be improved.

I think one simple thing we could do is use a more specific result.tag than .invalid? Currently we use .invalid for everything but we could use more specific ones. Is that an option?
I feel like this might be better suited as a follow-up PR.

[wooster0]

Besides, do all these "invalid_underscore_placement_in_"-named tests even belong in zig/test/cases/compile_errors? These tests seem like very simple tests that should just be part of tests in tokenizer.zig (and probably most if not all of them already are).

[Vexu]

I agree. I thought it was very confusing too but then I noticed that we actually already have this problem with many other, similar cases too:

Yes, and I'm about to open an issue for this.

I think one simple thing we could do is use a more specific result.tag than .invalid? Currently we use .invalid for everything but we could use more specific ones. Is that an option?

Most uses of .invalid are related to literals, it would be better to just validate them at a later stage than the tokenizer.

I feel like this might be better suited as a follow-up PR.

Fixing the previous things sure, but I'd prefer we start this new error off properly.

[nektro]

this broke translate-c

[wooster0]

How did it break? Any test case? Can you elaborate?

[nektro]

/run/media/meghan/dev/mount.ufs/zig-cache/o/9aeca74be3315ca7b4accb89a5db24de/cimport.zig:1648:33: error: integer literal '00' has leading zero
pub const O_RDONLY = @as(c_int, 00);
                                ^~

[wooster0]

What is the C version of that?

[nektro]

https://github.com/ziglang/zig/blob/master/lib/libc/musl/include/fcntl.h#L44

#define O_RDONLY  00