Unstable zig package for tooling

[sno2]

It would be great to have an unstable zig package available. A tokenizer/parser and semantic analyzing API would make tooling (e.g. lsp, formatter, documentation generator) fairly straightforward to implement. I would be willing to add a PR for this.

[fubark]

That would be nice to have! I would appreciate a PR even if it's just a barebones api over the zon package (just reading about it now). I'm not sure what kind of endpoints would be needed for an LSP but I'd be happy to add them when the time comes.

To get started, there is currently https://github.com/fubark/cyber/blob/master/src/capi.zig. It's not comprehensive but it can be used as a reference.

[sno2]

I was more thinking of exposing cyber.zig, or a more minimal Zig file with tokenizer/parser/sema exposed, as a Zig package and marking the API as non-SemVer. I integrated the tokenizer in the C API because I have not yet played around with Zig bindings generation. However, working the entire parser API along with sema.zig's functionality seems like it would require a lot of work.

[fubark]

Yea that's fine too. A lot of the data types do not map to C structs so that would be easier to expose. The only issue is it would require a particular Zig version when writing an extension but it's better than nothing.

[sno2]

Now that 0.13 is released, we can cleanly use the Zig build system to depend on the upstream sources for mimalloc and tcc.

@fubark Do you know what Git commit the vendored mimalloc is on? I was able to get tcc working, but I don't see mimalloc's hash mentioned anywhere.

[fubark]

I forgot to record the hash so I ended up upgrading mimalloc to latest. Also, I'm not sure 0.13.0 is stable for Cyber. I saw tests fail from what looked like packed struct miscompilations. (Although it's probably fixed on master)

Edit: Mimalloc is now at 03020fbf81541651e24289d2f7033a772a50f480

[sno2]

Thanks for upgrading mimalloc, and I see what you mean by the failing tests. Here is my branch, although I didn't commit the tcc module migration.

https://github.com/sno2/cyber/tree/upgrade-zig

Definitely some UB happening in the tests, so I'll probably wait until 0.14 releases and try again then.

[sno2]

I got cyber building on 0.14, but we are still getting the same error on the ARC test.

https://github.com/sno2/cyber/tree/upgrade-zig-0.14

What steps did you take to figure out that this was most likely a packed struct miscompilation? I wonder if explicitly stating the size of the packed struct like packed struct(...) could help.

[fubark]

I got more tests to pass when I removed the packed modifier so I figured it might be related to packed structs. But I also didn't try removing all of them, so I probably need to look at it again.

[fubark]

Here is a MRE:

const std = @import("std");

pub const Foo = packed struct {
    a: u31,
    b: bool,

    pub fn init() Foo {
        return .{ .a = undefined, .b = true };
    }
};

pub fn foo(a: Foo) void {
    std.debug.print("{} {b}\n", .{a.b, @as(u32, @bitCast(a))});
    // Expected "true", and a non zero value
    // Found "false", 0
}

pub fn main() !void {
    const a = Foo.init();
    foo(a);
}

The issue seems to be passing a packed struct where the first field is initialized as undefined.

[sno2]

Nice find! I have also been looking into this issue this morning although I hadn't been able to deduce that. It seems like using i32 as the backing integer works around the issue:

pub const Foo = packed struct(i32) {
    a: u31,
    b: bool,

    pub fn init() Foo {
        return .{ .a = undefined, .b = true };
    }
};

u32 has the same problems as the unspecified version.

[fubark]

Neither works for me on 0.14.0-dev.839+a931bfada or 0.13.0. I'm on arm64 so perhaps it's also architecture specific issue.

[sno2]

Ah, sorry seems to be non-deterministic. I should've ran more than 3 times. (Zig 0.13, Windows)

$ zig run foo.zig
true 10000001010110101010010010100000
$ zig run foo.zig
true 10110001000000111010010010100000
$ zig run foo.zig
true 10101111010011011010010010100000
$ zig run foo.zig
true 11101011000100111010010010100000
$ zig run foo.zig
false 110100111001010010010100000