Hooking up tests and fixing various Zig compile errors · compiler development

Stream: compiler development

Topic: Hooking up tests and fixing various Zig compile errors

Jared Ramirez (Feb 11 2025 at 18:41):

Yesterday, I started looking into implementing the function solving stage of the compiler. When I created a stub of a test in build/solve_functions.zig, it resulted in a smattering of zig compiler errors for the code paths now used.

For some reason though, this test wasn't picked up by root esting.refAllDeclsRecursive(@import("main.zig")) in test.zig, even though solve_functions was part of the dependency graph. In the PR, @Brendan Hansknecht suggested that it might be helpful to manually add all stagings of the compiler, base, and collections to test.zig to get ahead of any other errors that other folks might run into. So this morning, I did that and have been working through this next set of errors. But before I get too far, I wanted to double check:

That other folks think this is worth doing
That touching lots of these shared files won't conflict with other folks' work

Jared Ramirez (Feb 11 2025 at 18:43):

The errors range from trivial (typos like Id -> Idx) to more complicated (like needing read/understand functions like base.Package.addDependencyToPackage to fix type mismatches)

Brendan Hansknecht (Feb 11 2025 at 18:49):

@Sam Mohr may already be looking into this?

Sam Mohr (Feb 11 2025 at 18:53):

No, I was busy yesterday. I was planning on doing it today, but if @Jared Ramirez I'm okay with that!

Sam Mohr (Feb 11 2025 at 19:00):

Anyway, this is the time to do this! Break away

Jared Ramirez (Feb 11 2025 at 23:08):

@Sam Mohr can you help me understand what exposing_modules is doing in the Ident.Store?

Jared Ramirez (Feb 11 2025 at 23:09):

From how it's used, it seems like it used to create a connection between an Ident and a Module (namely, which module exposes it), but since the type is Astd.ArrayList(base.Module.Idx) I don't see exactly how.

Anthony Bullard (Feb 11 2025 at 23:10):

I fixed a bunch of things in safe_list.zig

Jared Ramirez (Feb 11 2025 at 23:10):

Based on these functions:

    pub fn getExposingModule(self: *Store, idx: Idx) base.Module.Idx {
        return self.exposing_modules.items[@as(usize, @intFromEnum(idx))];
    }

    /// Set the module that exposes this ident.
    ///
    /// NOTE: This should be called as soon as an ident is encountered during
    /// canonicalization to make sure that we don't
    pub fn setExposingModule(self: *Store, idx: Idx, exposing_module: base.Module.Idx) void {
        self.exposing_modules.items[@as(usize, @intFromEnum(idx))] = exposing_module;
    }

It seems like the ident is the index of the array? It almost seems like this could be a map instead of an array?

Anthony Bullard (Feb 11 2025 at 23:11):

I think a number of us learned some lessons about when and when not the Zig compiler does semantic analysis on dead code

Jared Ramirez (Feb 11 2025 at 23:12):

Yeah, I've been working through lots of errors all over haha

Jared Ramirez (Feb 11 2025 at 23:13):

I fixed a bunch of things in safe_list.zig

I did too, along with a bunch of other files. When will your changes go in? I'll be sure to pull in the latest and resolve conflicts before PRing

Jared Ramirez (Feb 11 2025 at 23:15):

It almost seems like this could be a map instead of an array?

Maybe it's an array for perf reasons?

Anyways, I figured it out. Idx is not an enum:

    pub fn setExposingModule(self: *Store, idx: Idx, exposing_module: base.Module.Idx) void {
        self.exposing_modules.items[@as(usize, idx.id)] = exposing_module;
    }

Sam Mohr (Feb 11 2025 at 23:28):

I can explain the intent of exposing_modules in like 2 hours properly

Sam Mohr (Feb 11 2025 at 23:28):

In short, it's just a way to say "this foo ident was imported from something called "Bar", which may or may not exist"

Sam Mohr (Feb 11 2025 at 23:29):

And then during your phase of the compiler, you'll need to say "okay, when this module uses an Ident.Idx that points to foo and is from module "Bar", which module in my list is actually called "Bar"

Sam Mohr (Feb 11 2025 at 23:46):

And you'll need to be conscious that:

multiple packages can have the same named module "Foo", so you have to be package-aware
a package shorthand, e.g. cli.Foo might be the same in two different packages, so you need to handle "cli" per-package

Brendan Hansknecht (Feb 11 2025 at 23:51):

@Andrew Kelley just curious if there is a blessed zig way to do this? Should we just expect dead code to bit root and break in zig?

Currently we are just testing.redAllDecsRecursive on a ton of file to get full type checking. I'm guessing a large part of this pain is simply that the compiler isn't actually fully wired together yet.

Jared Ramirez (Feb 12 2025 at 00:03):

I have a PR enabling the tests for more files here, along with the fixes for various compile errors.

Full details in the PR, but after these changes, it seems like test pass, but not transitive tests:

[Running: zig build test --summary all]
test
└─ run test failure
error: while executing test 'test.test_0', the following command terminated with signal 11 (expected exited with code 0):
/.../roc/.zig-cache/o/7efe31480f3801691496ea52b63aabcb/test --listen=-
Build Summary: 3/5 steps succeeded; 1 failed; 7/7 tests passed (disable with --summary none)
test transitive failure
└─ run test failure
   └─ zig test Debug native-native-musl success 1s MaxRSS:243M
      └─ run gencat (gencat.bin.z) cached
         └─ zig build-exe gencat Debug native cached 3ms MaxRSS:50M
error: the following build command failed with exit code 1:
/.../roc/.zig-cache/o/adcce5f042dbee170734c34f9f0af3e9/build /nix/store/5yk32f31879lfsnyv0yhl0af0v2dz9dz-zig-0.13.0/bin/zig /.../roc /home/jared/dev/github/jaredramirez/roc/.zig-cache /home/jared/.cache/zig --seed 0xb18323b4 -Zf6c0cef7264c1068 test
[Command exited with 1]

Still looking into it

Jared Ramirez (Feb 12 2025 at 01:06):

Figured out the issue, details are in PR

Andrew Kelley (Feb 14 2025 at 04:58):

Brendan Hansknecht said:

Andrew Kelley just curious if there is a blessed zig way to do this? Should we just expect dead code to bit root and break in zig?

Currently we are just testing.redAllDecsRecursive on a ton of file to get full type checking. I'm guessing a large part of this pain is simply that the compiler isn't actually fully wired together yet.

Personally I never use that function because it's obviously a hack to workaround a deficiency in the language, and my code of conduct requires me to embrace the full consequences of my own actions. Plus it's ugly!

Jokes aside, consider that any generic function is also not being checked unless you test it. So you can think of normal functions and generic functions in the same way.

For packages where you want to maintain a compatible API, the lack of analysis can be an issue, but for an application, I question the validity of wanting an extra check on all those declarations. If you're not using them, what's the point? If you're using them then they'll be compiled. Presumably when you were writing the code you needed to verify the behavior right? So they should be covered by some kind of testing, which would ensure they are analyzed

Sam Mohr (Feb 14 2025 at 05:00):

At the start of writing out the compiler, to enable parallel development, I started by sketching out the rough IRs for at least most of the compiler stages

Sam Mohr (Feb 14 2025 at 05:00):

So there's a lot of skeleton code that lets individual contributors work within their own lanes

Sam Mohr (Feb 14 2025 at 05:00):

But its not getting called yet, since no one has written the coordinating logic that ties everything together

Sam Mohr (Feb 14 2025 at 05:01):

So until at least the coordination of compilation stages is wired up, that's a hole that's worth the "dead code"

Andrew Kelley (Feb 14 2025 at 05:01):

Jared Ramirez said:

I have a PR enabling the tests for more files here, along with the fixes for various compile errors.

Full details in the PR, but after these changes, it seems like test pass, but not transitive tests:

[Running: zig build test --summary all]
test
└─ run test failure
error: while executing test 'test.test_0', the following command terminated with signal 11 (expected exited with code 0):
/.../roc/.zig-cache/o/7efe31480f3801691496ea52b63aabcb/test --listen=-
Build Summary: 3/5 steps succeeded; 1 failed; 7/7 tests passed (disable with --summary none)
test transitive failure
└─ run test failure
   └─ zig test Debug native-native-musl success 1s MaxRSS:243M
      └─ run gencat (gencat.bin.z) cached
         └─ zig build-exe gencat Debug native cached 3ms MaxRSS:50M
error: the following build command failed with exit code 1:
/.../roc/.zig-cache/o/adcce5f042dbee170734c34f9f0af3e9/build /nix/store/5yk32f31879lfsnyv0yhl0af0v2dz9dz-zig-0.13.0/bin/zig /.../roc /home/jared/dev/github/jaredramirez/roc/.zig-cache /home/jared/.cache/zig --seed 0xb18323b4 -Zf6c0cef7264c1068 test
[Command exited with 1]

Still looking into it

Since it didn't print a stack trace, my guess is stack overflow. related

you can use a debugger on the printed test executable to be sure.

Andrew Kelley (Feb 14 2025 at 05:01):

Then, lastly, this PR adds watchexec to the nix shell to easily re-run test on file change.

not sure what watchexec is doing but based on the name alone, you might want to give zig build --watch a try :smile:

Brendan Hansknecht (Feb 14 2025 at 05:20):

Sam Mohr said:

So until at least the coordination of compilation stages is wired up, that's a hole that's worth the "dead code"

yeah, I think it's a growing pain with how we decided to start writing roc

Brendan Hansknecht (Feb 14 2025 at 05:21):

Likely we want to switch to something normal once we have the pieces of the compiler connected

Brendan Hansknecht (Feb 14 2025 at 05:22):

@Sam Mohr how hard would it be to make a minimal coordinate that just calls everything linearly? I guess it is more complex than that due to imports?

Sam Mohr (Feb 14 2025 at 05:23):

I have two entire days free this weekend to try it. Yes imports make it a little complex, but just stubbing out some functions and properly implementing the overall shape should be doable

Brendan Hansknecht (Feb 14 2025 at 05:25):

Also, to be clear, now that we have the recursive import hack, this isn't too important, but clearly will be needed at some point

Richard Feldman (Feb 14 2025 at 05:29):

Sam Mohr said:

I have two entire days free this weekend

this has happened to me once in the ~2.5 years since I had a kid. I remember it vividly. I got so much done!

Sam Mohr (Feb 14 2025 at 05:29):

My goal is to finish the rewrite before you get any more free time so I can rename it to Sam's Roc

Sam Mohr (Feb 14 2025 at 05:30):

I've been clearing these next two months because I really think we can have the core of things working by then

Sam Mohr (Feb 14 2025 at 05:30):

Everything else is incremental

Last updated: Jul 06 2025 at 12:14 UTC