zig compiler - IR serde · compiler development

Hi! I'm starting on function specialization. I'd like to create a test that has the correct input data in it. Since I'm at the veeery beginning of learning, can someone give me an example how I could build up an IR in zig manually (with module Env all that)? Just for example this code:

increment = |num| num + 1

main = || {
    x = 2
    increment(x)
}

Sam Mohr (Feb 16 2025 at 09:25):

Sam Mohr (Feb 16 2025 at 09:29):

We could manually create nodes, but that's probably more trouble than it's worth

Sam Mohr (Feb 16 2025 at 09:33):

If you really want to go that way, let me know and I can make a branch with an example

Norbert Hajagos (Feb 16 2025 at 13:39):

I agree a serializable format would be better. I guess there's no need for me to rush straight to tackling specialization, if there are things that would make it easier.

Brendan Hansknecht (Feb 16 2025 at 16:10):

Brendan Hansknecht (Feb 16 2025 at 16:11):

Norbert Hajagos (Feb 16 2025 at 16:13):

From a quick look, yes, they seem pretty similar. At least from what I have seen, in function lift and function solve IR. But after reading Ayas' document, they should be the same as they are just more and more simplified versions of one-another.

Brendan Hansknecht (Feb 16 2025 at 16:17):

I more meant in datastructure form such that a tiny bit of zig comptime could deal with any of them for printing and parsing. And I know they all should have similar datastrtutures. Just not sure if they are close enough in practice.

Norbert Hajagos (Feb 16 2025 at 16:23):

You mean creating a general IRPrinter that can take any of the IR's and spit out the correct strings? (Same for the parse)

Brendan Hansknecht (Feb 16 2025 at 16:23):

Brendan Hansknecht (Feb 16 2025 at 16:25):

Norbert Hajagos (Feb 16 2025 at 16:29):

I see. Would be cool and should strive for it I think! I just started to work on a printer. I'm still in the Zig phase of "What.. a file is a struct?", so I don't know when that will be usable. Right now my plan is to create 'serialize' functions on all the base.zig types. They would take a std.io.AnyWriter as argument. Once I can print those, I can widen my horizon.

Brendan Hansknecht (Feb 16 2025 at 16:48):

Joshua Warner (Feb 16 2025 at 17:45):

My thinking was to create a generic sexpr parser, and have each IR be constructable from that sexpr representation. Similarly for printing - irs only need to know how to output sexpr nodes.

Norbert Hajagos (Feb 16 2025 at 17:55):

Do I understand it correctly that Sexpr would be an Intermediate representation for all the intermadiate representations during serialization / deserialization :big_smile: ? So we would go from {check_IR, lift_IR, ...}->Sexpr->String and similarly backwards

Notification Bot (Feb 16 2025 at 17:58):

Joshua Warner (Feb 16 2025 at 18:01):

Norbert Hajagos (Feb 16 2025 at 18:01):

Joshua Warner (Feb 16 2025 at 18:01):

Isaac Van Doren (Feb 16 2025 at 19:09):

How do you think a common printer and parser would be implemented in Zig? I don’t have a clear idea how that would work

Brendan Hansknecht (Feb 16 2025 at 19:13):

Sounds like they were thinking just making a generic intermediate IR. Then having everything go to/from that data structure.

Brendan Hansknecht (Feb 16 2025 at 19:15):

Otherwise, you can do interfaces by doing what allocators do with essentially a vtable. Could make one that every ir implements that is used for printing/parsing

Brendan Hansknecht (Feb 16 2025 at 19:15):

Brendan Hansknecht (Feb 16 2025 at 19:57):

When we are spinning this up, if anyone needs help with zig interfaces and what not, let me know. I think if you design the interface as you see fit ignoring zig, I can help translate to zig.

Norbert Hajagos (Feb 16 2025 at 20:03):

Norbert Hajagos (Feb 16 2025 at 20:25):

I already have a zig question :big_smile:
The type of base.Literal.String is of collections.interner.StringLiteral
I wanted to experiment with printing each kind of literal, but the StringLiteral type doesn't seem to hold any value. It's only a namespace for the Idx and the Interner type. I undestand that with the combination of an interner and an index, i can query the string value, but does the current representation of base.Literal.String doesn't have either, so it holds 0 information.

Should the base.Literal.String be of type collections.interner.StringLiteral.Idx, or am I missing something?

Luke Boswell (Feb 16 2025 at 20:30):

The IRs in those later build stages are mostly a placeholder. It's possible it just doesn't make sense. Im afk rn but can help poke at it in a couple of hours.

Norbert Hajagos (Feb 16 2025 at 20:33):

Thanks, but in a couple of hours, I'll already be sleeping :smile:. Think I'll just change it to Idx, see how things will puzzle out.

Sam Mohr (Feb 16 2025 at 20:34):

Sam Mohr (Feb 16 2025 at 20:35):

The lazy compilation thing has been pretty tricky for starting this project, though I'm sure it'll not be as annoying in a couple weeks

Norbert Hajagos (Feb 16 2025 at 20:37):

Sam Mohr (Feb 16 2025 at 20:40):

Norbert Hajagos (Feb 16 2025 at 22:41):

As I was exploring, I stumbled upon a memory leak in the string interner. Fixed it on my branch (testing allocator is so good, just running zig build test does the job).
But... it just feels wrong to loop through strings and free them one-by-one. After all these times of hearing about them, I'm pretty sure this is where I'd need an arena. I tried to wrap the interner's allocator in one, but it kept leaking anyways, so I'm asking if any one of you could guide me here.

Faild attempt at using arena:

pub const Interner = struct {
    strings: std.ArrayList([]u8),
    arena: std.heap.ArenaAllocator,

    pub fn init(allocator: std.mem.Allocator) Interner {
        var arena = std.heap.ArenaAllocator.init(allocator);
        return Interner{
            .strings = std.ArrayList([]u8).init(arena.allocator()),
            .arena = arena,
        };
    }

    pub fn deinit(self: *Interner) void {
        self.arena.deinit();
    }

    pub fn insert(self: *Interner, string: []u8) Idx {
        const len = self.strings.items.len;
        const copied_string = self.arena.allocator().alloc(u8, string.len) catch exitOnOom();
        std.mem.copyForwards(u8, copied_string, string);

        self.strings.append(copied_string) catch exitOnOom();

        return @enumFromInt(@as(u32, @intCast(len)));
    }

    pub fn get(self: *Interner, idx: Idx) []u8 {
        return self.strings.items[@as(usize, @intFromEnum(idx))];
    }
};

Richard Feldman (Feb 16 2025 at 22:48):

yeah I think we need arenas for everything, including string interning, so that we can serialize them :big_smile:

Richard Feldman (Feb 16 2025 at 22:48):

I guess except like...scratch allocations that aren't going to end up being serialized maybe

Sam Mohr (Feb 16 2025 at 22:48):

Sam Mohr (Feb 16 2025 at 22:49):

I think having some naming pattern around scratch_arena vs. just arena could help there

Sam Mohr (Feb 16 2025 at 22:49):

Since Zig seems to lean into the gpa meaning "use a general-purpose allocator here" naming convention

Richard Feldman (Feb 16 2025 at 22:50):

@Andrew Kelley pointed out that bump arenas are not conducive to arraylists that need to resize because they're super unlikely to be able to resize in-place

Richard Feldman (Feb 16 2025 at 22:50):

Richard Feldman (Feb 16 2025 at 22:51):

Brendan Hansknecht (Feb 16 2025 at 23:38):

Brendan Hansknecht (Feb 16 2025 at 23:39):

For arraylists, the amortization of growing by 1.5 to 2x and reasonable capacity defaults will make it cheap

Brendan Hansknecht (Feb 16 2025 at 23:40):

Brendan Hansknecht (Feb 16 2025 at 23:42):

Yeah, and I think we should have a pattern of all allocators being named arena or gpa. I guess we could get more fine grain, but that is likely enough by context of the datastructure. Like the parsing arena won't out live the parsing IR.

Richard Feldman (Feb 16 2025 at 23:44):

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

There will be one arena for that which CanIR will own, and it'll get provided to each module's ModuleEnv as their allocator. We should call the allocator for ModuleEnv "arena" to make that more obvious

Sam Mohr (Feb 16 2025 at 23:47):

And by CanIR owning the arena, then the serialization work just falls out of that for free

Brendan Hansknecht (Feb 16 2025 at 23:48):

Sam Mohr (Feb 16 2025 at 23:48):

So parse will intern data into the CanIR arena but put its IR into a different allocator's memory, canonicalize will put its intern data (e.g. generated idents created during desugaring) into the arena

Sam Mohr (Feb 16 2025 at 23:49):

Brendan Hansknecht (Feb 16 2025 at 23:49):

Sam Mohr (Feb 16 2025 at 23:50):

So if there's a single data structure, CanIR, in the arena, which includes the ModuleEnv, then we can just treat that arena as binary data that goes into/out of the cache

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

The ir needs to be in a gpa or you will hit big pains on growth. On top of that, if your arena has tons of gaps, you don't want to serialize it.

Sam Mohr (Feb 16 2025 at 23:52):

Brendan Hansknecht (Feb 16 2025 at 23:52):

I think it is better to think of serialization as writing out a struct containing each of the core arrays of data. Each array only uses indices and not pointers and thus can just be copied to disk.

Brendan Hansknecht (Feb 16 2025 at 23:52):

I would not consider serializing arenas. They will have tons of gaps and garbage data from intermediates.

Brendan Hansknecht (Feb 16 2025 at 23:53):

Also, references to data in an arena are still pointers (thus not safe to just serialize)

Sam Mohr (Feb 16 2025 at 23:53):

Sam Mohr (Feb 16 2025 at 23:54):

All intermediates besides arraylist's backing arrays should be created within other allocators

Sam Mohr (Feb 16 2025 at 23:54):

Sam Mohr (Feb 16 2025 at 23:55):

Which would be faster: creating an mmaped arena that we don't have to serialize, or using a gpa to get a more compact memory backing for the serialized data and then manually copying to/from the cache?

Sam Mohr (Feb 16 2025 at 23:55):

Sam Mohr (Feb 16 2025 at 23:56):

But if it's similar perf, then maybe the arena-based approach isn't worth the effort

Brendan Hansknecht (Feb 16 2025 at 23:56):

Sam Mohr (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:59):

Sam Mohr (Feb 17 2025 at 00:00):

I've been presuming Richard's way must be faster for us to consider it, but that may very well be wrong

Richard Feldman (Feb 17 2025 at 00:02):

so briefly, the whole goal of the serialization is that reading a cache into memory is 2 syscalls: open the file, and then read it into memory. That's it, there's no parsing step whatsoever - once it's in memory, it is already ready to use normally

Brendan Hansknecht (Feb 17 2025 at 00:03):

Richard Feldman (Feb 17 2025 at 00:04):

Brendan Hansknecht (Feb 17 2025 at 00:05):

Well, you may want to do some minor range processing to get zig slices instead of raw offsets for some things, but that is derived from the raw mmap

Richard Feldman (Feb 17 2025 at 00:06):

the thing is, in bigger projects (where compile times are noticeable) reading from cache is going to be overwhelmingly how things get loaded into memory

Brendan Hansknecht (Feb 17 2025 at 00:06):

Brendan Hansknecht (Feb 17 2025 at 00:07):

Richard Feldman (Feb 17 2025 at 00:07):

that reallocate and copy if necessary (like an arraylist does), which is fine if everything is indices

Brendan Hansknecht (Feb 17 2025 at 00:08):

I guess what I'm trying to say is separate initial construction from cache form.

Brendan Hansknecht (Feb 17 2025 at 00:08):

Initial construction will be fastest and easiest to do with large gpa backed arraylists

Brendan Hansknecht (Feb 17 2025 at 00:09):

Richard Feldman (Feb 17 2025 at 00:09):

a critical part of the "2 syscalls" idea is that the very beginning of the allocation (as in, index zero) has to be the header which holds all the other arraylists (and then from there everything can be indices into those)

Richard Feldman (Feb 17 2025 at 00:09):

Brendan Hansknecht (Feb 17 2025 at 00:10):

Yes, when serializing, you do so flat (header of slice info) then all those slices constructed above with the gpa.

You serialize flat into a static sized file. This can be loaded with the two syscall approach

Richard Feldman (Feb 17 2025 at 00:10):

ok separate question though: if we're running this persistently, and we've loaded something from cache, we explicitly don't want to dealloc everything individually

Richard Feldman (Feb 17 2025 at 00:10):

Richard Feldman (Feb 17 2025 at 00:11):

Brendan Hansknecht (Feb 17 2025 at 00:11):

You can dealloc at the module level. Each module will have its own mmap or allocators

Richard Feldman (Feb 17 2025 at 00:11):

Richard Feldman (Feb 17 2025 at 00:12):

separately, I think we'll want to do some giant virtual alloc up front and then read all the cache entries into that

Richard Feldman (Feb 17 2025 at 00:13):

Brendan Hansknecht (Feb 17 2025 at 00:21):

Brendan Hansknecht (Feb 17 2025 at 00:23):

MMaps are lazily loaded and page fault to load data. Force loading them into one big allocation would force being everything to be copied an extra time.

Brendan Hansknecht (Feb 17 2025 at 00:23):

It might be beneficial to load everything into CPU cache early, but if so, we can just force non-lazy loading of the mmaps

Brendan Hansknecht (Feb 17 2025 at 00:25):

And due to everything being handled at the page level anyway, doesn't really matter if we have "scattered" mmap pages or "linear" mmap pages. Both are totally up to the os virtual to physical page map to where they will actually be in memory

Brendan Hansknecht (Feb 17 2025 at 00:31):

As a note, the string intern probably needs to be two gpa backed arraylists. One of starts and lengths for the interned strings. And one flat U8 arraylists of string content. That will make serialization easy.

Aside, does string interning do deduplication if so, it probably also needs an hashmap of string to index it is inserted at. No need to serialize the hashmap cause the string intern will be finalized when serialized to disk.

Richard Feldman (Feb 17 2025 at 00:38):

Brendan Hansknecht (Feb 17 2025 at 01:06):

Oh, I thought the point of making a flat serializable datastructure was to enable mmaping them

Brendan Hansknecht (Feb 17 2025 at 01:06):

Brendan Hansknecht (Feb 17 2025 at 01:07):

We would either need to serialize the interner or would need to reference the original source to reload the strings

Sam Mohr (Feb 17 2025 at 01:08):

I definitely figured we'd be serializing the interner, which would "own" the strings within the arena instead of referencing the source file or something

Brendan Hansknecht (Feb 17 2025 at 01:09):

Richard Feldman (Feb 17 2025 at 01:11):

btw this can be even cheaper than that! writev can let you skip the memcpys, and there's an equivalent on windows

Richard Feldman (Feb 17 2025 at 01:12):

Brendan Hansknecht (Feb 17 2025 at 01:12):

Yeah, that sounds fine too (can test both and see which is faster, minor detail on how the data is loaded)

Brendan Hansknecht (Feb 17 2025 at 01:13):

Yeah, I said memcpy, but I really meant a copy to disk. Which would be writeev or whatever system call.

Richard Feldman (Feb 17 2025 at 01:13):

mmap on anything not in a tempdir always makes me nervous that some other process is going to delete the file while the mmap is active and cause chaos

Richard Feldman (Feb 17 2025 at 01:13):

although wait, didn't you say there's a setting that can prevent that? :thinking:

Brendan Hansknecht (Feb 17 2025 at 01:14):

Richard Feldman (Feb 17 2025 at 01:14):

Brendan Hansknecht (Feb 17 2025 at 01:15):

Yeah, if so, reading is fine. Then less chance for failure at the cost of eagerly loading everything (which could actually be a perf gain, really depends on usage pattern). Could be worse for memory use, but maybe not in any sort of meaningful way.

Brendan Hansknecht (Feb 17 2025 at 01:16):

But yeah, read or mmap or etc all sound fine and we can figure out the tradeoffs later.

Brendan Hansknecht (Feb 17 2025 at 01:19):

Sounds like mmap counts as an extra reference count on a file and it won't actually be deleted until the mmaps is closed (on Linux)

Brendan Hansknecht (Feb 17 2025 at 01:19):

Brendan Hansknecht (Feb 17 2025 at 01:20):

So deleting it will remove it from the file tree, but the lnode wont be deleted until the file is until it is safe

Brendan Hansknecht (Feb 17 2025 at 01:20):

Would definitely want to test that behaviour to make sure or look it up in docs somewhere

Brendan Hansknecht (Feb 17 2025 at 01:22):

Oh, though mutation on an mmaped file still may affect things, but if a user is mutating cache files, that is their own problem

Richard Feldman (Feb 17 2025 at 01:23):

Richard Feldman (Feb 17 2025 at 01:24):

Richard Feldman (Feb 17 2025 at 01:25):

in that design the 2 syscalls are open and mmap, and then afterwards we can munmap if we're a language server and want to selectively clean up

Richard Feldman (Feb 17 2025 at 01:25):

Brendan Hansknecht (Feb 17 2025 at 01:28):

Andrew Kelley (Feb 17 2025 at 03:29):

ArrayList didn't grow this debug feature yet but some of the data structures such as hash maps have "pointer locks", so if you want to keep element pointers alive for a nontrivial amount of function call graph, you can do something like this:

map.lockPointers();
defer map.unlockPointers();

and it will crash if you called any of the non "assume capacity" methods. i.e. you get a crash instead of potential Use-After-Free

Andrew Kelley (Feb 17 2025 at 03:31):

Brendan Hansknecht (Feb 17 2025 at 03:39):

Norbert Hajagos (Feb 17 2025 at 10:15):

/// As opposed to the [IdentName.Interner], this interner does not
/// deduplicate its values because they are expected to be almost all unique,
/// and also larger, meaning more expensive to do equality checking on.

I also thought of something similar, since the strings won't be changing in the buffer. How do you feel about a design like this?

// just after inicialization
expect interner.buff == ""
expect interner.start_positions == [0]

//inserting 2 strings
const hi_idx = interner.insert("hi"); // 0
const there_idx = interner.insert("there"); // 1

//results with
expect interner.buff == "hithere"
expect interner.start_positions == [0, 2, 7]

// no need to store start and length, since the next value in start_positions holds where the string ends
expect interner.get(there_idx) == interner.buff[interner.start_positions[there_idx]..interner.start_positions[there_idx +1]]

Would be happy to implement it. What type should the start_positions arraylist hold? If only string literals from source code will be in the interner, u32 would be enough. Source files with > 4Gb str literals aren't realistic imo. However, if we stored embedded files in these as well (think import "some-file" as some_str : Str) then I'd use a usize just in case. I'm easily convinced it's an overkill though.

Andrew Kelley (Feb 17 2025 at 10:48):

null termination is viable as well. with short strings the cpu cache savings can be worth it

Joshua Warner (Feb 17 2025 at 11:18):

Or a previous strategy I’ve seen, of prepending the string with a varint length, encoded backwards, and placed before the offset you return.

Norbert Hajagos (Feb 17 2025 at 12:23):

Cool! Could do either. @Joshua Warner do you think storing the length that way would be beneficial over having null terminated strings?

Joshua Warner (Feb 17 2025 at 15:40):

The advantage would be in making computing the length of strings >= ~64 bytes (a cache line) faster

Joshua Warner (Feb 17 2025 at 15:41):

(that said, I think it's unlikely we'll have many identifiers >= 64 bytes, so :shrug: )

Brendan Hansknecht (Feb 17 2025 at 15:52):

Brendan Hansknecht (Feb 17 2025 at 16:02):

I think we should handle imported sources special and just never put them in interners. That would be a waste. And since the interners are per module, the strings from a single module can never be over 4GB.

Richard Feldman (Feb 17 2025 at 17:35):

should literals be interned at all? I don't see the benefit honestly :sweat_smile:

Brendan Hansknecht (Feb 17 2025 at 17:36):

Brendan Hansknecht (Feb 17 2025 at 17:37):

Brendan Hansknecht (Feb 17 2025 at 17:38):

But yeah, I think the only real gain is if it saves a lot of memory in large roc projects due to being able to free files.

Joshua Warner (Feb 17 2025 at 18:14):

Presumably we would want to serialize that data with the can IR (or whatever else we're caching)

Joshua Warner (Feb 17 2025 at 18:15):

That way we only have to read the can IR and not the original file (e.g. supposing we're confident it hasn't changed - and assuming we didn't need to hash it to figure that out)

Richard Feldman (Feb 17 2025 at 18:17):

we already have to read the original file into memory in order to hash it, because the hash is the key for the cached can IR

Richard Feldman (Feb 17 2025 at 18:17):

and the can IR is a hash of the source bytes, so we know all the offsets into the source file will still be valid! :smiley:

Joshua Warner (Feb 17 2025 at 18:17):

Joshua Warner (Feb 17 2025 at 18:18):

(this is exactly what git does - it doesn't rehash all files in your git dir every time you commit - only the ones that have a different stat)

Joshua Warner (Feb 17 2025 at 18:20):

If we can only read one file (the can ir), and not seek the disk to read the original file again, that's a substantial speed boost

Brendan Hansknecht (Feb 17 2025 at 19:19):

Brendan Hansknecht (Feb 17 2025 at 19:20):

Richard Feldman (Feb 17 2025 at 19:41):

Richard Feldman (Feb 17 2025 at 19:42):

like if the CLI receives an instruction to check main.roc, how would it know what cached IR to load without reading and hashing the source bytes?

Richard Feldman (Feb 17 2025 at 19:42):

(it's different if it's a persistent process that's watching the filesystem for changes, of course)

Brendan Hansknecht (Feb 17 2025 at 20:09):

Andrew Kelley (Feb 18 2025 at 02:19):

Andrew Kelley (Feb 18 2025 at 02:20):

In zig we do it because it helps simplify our memory model, particularly with serialization

Andrew Kelley (Feb 18 2025 at 02:21):

Richard Feldman (Feb 18 2025 at 02:24):

if the literal doesn't have any escapes in it, you can just store the region in the original source file
because those are the exact bytes you want anyway

Andrew Kelley (Feb 18 2025 at 02:24):

Have a cache file per source file path, and store the size, mtime, inode. If any of those change, cache miss. We do this in zig for source -> ZIR

Andrew Kelley (Feb 18 2025 at 02:25):

Oh, we don't have source files, tokens, or ast nodes loaded at all in the happy path

Richard Feldman (Feb 18 2025 at 02:26):

we could do the size/mtime/inode/etc as an extra layer, but if any of those change we still have to redo the hash computation

Richard Feldman (Feb 18 2025 at 02:28):

in the cache miss scenario, we might discover that we need to re-hash sooner...but then the first thing we do in a cache miss is hash anyway, to know what to name the new cache file :big_smile:

Richard Feldman (Feb 18 2025 at 02:28):

so I guess another way to say it is that we re-hash whether there's going to be a cache miss or cache hit, so might as well just always do it, yeah?

Andrew Kelley (Feb 18 2025 at 02:29):

Andrew Kelley (Feb 18 2025 at 02:30):

In our case it would be extremely wasteful since we don't even need access to the file contents on a cache hit

Andrew Kelley (Feb 18 2025 at 02:31):

Maybe I'm not understanding what you mean when you say "hash". Are you talking about the file contents?

Andrew Kelley (Feb 18 2025 at 02:38):

I vaguely recall something about assigning a unique integer per identifier across all source files or something like that. I'm guessing that's what you mean by "hash"

Richard Feldman (Feb 18 2025 at 02:40):

by hash I specifically mean a BLAKE3 hash of the entire contents of the .roc source file

Richard Feldman (Feb 18 2025 at 02:41):

Andrew Kelley (Feb 18 2025 at 02:54):

(mtime, inode, size) is generally considered to be reliable enough. As a bonus you can try to detect mtime granularity and avoid false positives in case the file system has high granularity (i.e. NFS, which nobody should be using)

Andrew Kelley (Feb 18 2025 at 02:55):

fstat cache hits are going to be a lot faster than hashing cache hits. Especially if you don't need to load the file contents on hit

Sam Mohr (Feb 18 2025 at 02:57):

If we intern string literals, then I'm pretty sure we wouldn't need anything from the source file, just the cache contents

Sam Mohr (Feb 18 2025 at 02:57):

Unless there are compiler errors and we need to show diagnostics based on regions in the source, of course

Richard Feldman (Feb 18 2025 at 02:58):

gotcha, yeah that's a reasonable argument for layering a (mtime, inode, size) cache on top of the BLAKE3 hash cache, to avoid reading the files and BLAKE3-ing them unnecessarily in the happy path case :thumbs_up:

Richard Feldman (Feb 18 2025 at 02:59):

I don't think we need to do any special design work for that now - can add that on sometime after 0.1.0

Richard Feldman (Feb 18 2025 at 02:59):

Richard Feldman (Feb 18 2025 at 03:00):

Sam Mohr (Feb 18 2025 at 03:00):

Richard Feldman (Feb 18 2025 at 03:02):

Sam Mohr (Feb 18 2025 at 03:10):

Joshua Warner (Feb 18 2025 at 03:42):

We can differentiate between interning (ensuring we have one unique copy of each) and just copying them

Joshua Warner (Feb 18 2025 at 03:42):

Joshua Warner (Feb 18 2025 at 03:43):

The former is potentially a small binary size optimization, but this is probably the wrong layer to do that at

Joshua Warner (Feb 18 2025 at 03:43):

We need to ensure symbols are unique, but I don't see any particular reason we need to guarantee strings are

Sam Mohr (Feb 18 2025 at 03:57):

Brendan Hansknecht (Feb 18 2025 at 04:43):

Brendan Hansknecht (Feb 18 2025 at 04:44):

Luke Boswell (Feb 22 2025 at 05:14):

test "example s-expr" {
    const source =
        \\module []
        \\
        \\foo = "bar"
    ;
    const expected =
        \\(header statement)
    ;

    try testHelper(testing.allocator, source, expected);
}

Luke Boswell (Feb 22 2025 at 05:14):

Intention will be to make it generate snapshots somehow... but while I'm developing the IR and wiring it up I'll probably use a unit test.

Luke Boswell (Feb 22 2025 at 23:38):

After going down this road a bit further... I'm thinking this may be the wrong approach.

Luke Boswell (Feb 22 2025 at 23:38):

Luke Boswell (Feb 22 2025 at 23:40):

That does mean we are providing a std.io.Writer, but I assume there is a way for that to not allocate, like maybe a heap allocated writer or similar.

Luke Boswell (Feb 23 2025 at 00:14):

I'm reasonably sure we just haven't got to this part yet, but wondering where the interned strings would be.

/// Represents a Pattern used in pattern matching.
pub const Pattern = union(enum) {
    ident: struct {
        ident_tok: TokenIdx, // <--- should this be an interned Ident.Idx instead?
        region: Region,
    },

pub const Token = struct {
    tag: Tag,
    offset: u32,
    extra: Extra,

    pub const Extra = union {
        length: u32,
        interned: base.Ident.Idx, <--- or should I be pulling it out from here??
    };

Luke Boswell (Feb 23 2025 at 00:15):

Or maybe using the offset and length and just indexing from the source bytes

Luke Boswell (Feb 23 2025 at 00:17):

Luke Boswell (Feb 23 2025 at 00:25):

Ok, yeah I'm pretty sure I should be grabbing them from the ModuleEnv using the Ident.Idx. So I'll need to take the env as an argument for the formatting helper.

Joshua Warner (Feb 23 2025 at 00:31):

Joshua Warner (Feb 23 2025 at 00:32):

Strings and numbers and such you’ll have to fetch from the source for now, but my plan is to also stash those somewhere, possibly on the ModuleEnv

Luke Boswell (Feb 23 2025 at 00:46):

Ok got something working. Just building out more of the structure for a minimal SExpr test

Sam Mohr (Feb 23 2025 at 01:19):

Brendan Hansknecht (Feb 23 2025 at 03:33):

Would be glad to chat about this design space at some point or help with implementation issues. Overall, the perf isn't too important. Though good perf will help with fuzzing

Luke Boswell (Feb 23 2025 at 03:43):

It was more a reflection on my SExpr "library" and how I had planned to map between the IR's and that to generate a string.

My new approach is to make a toStr helper for things that can be implemented at each level in the IR (but still has the global SoA context). Probably a terrible description -- but I'll get something minimal working soon and share my PR for comments/review.

Luke Boswell (Feb 23 2025 at 03:44):

Basically I'm experimenting with ways to do it as I haven't built something like this before.

Luke Boswell (Feb 23 2025 at 04:28):

test "example s-expr" {
    const source =
        \\module [foo, bar]
        \\
        \\foo = "hey"
        \\bar = "yo"
    ;
    const expected =
        \\(parse_ast (header "module" (exposes ("foo" "bar"))) (statements (decl (pattern (ident "foo")) (body ((expr "hey")))) (decl (pattern (ident "bar")) (body ((expr "yo"))))))
    ;

    try testSExprHelper(testing.allocator, source, expected);
}

Luke Boswell (Feb 23 2025 at 04:30):

Basic idea is to implement the following for various levels of the IR hierarchy and just call it recursively writing out the AST as it goes.

pub fn toStr(ir: *@This(), env: *base.ModuleEnv, writer: std.io.AnyWriter) !void { ... }

Brendan Hansknecht (Feb 23 2025 at 05:24):

Ah, but then you have to separately make a parser for every ir? Well, I guess the parser isn't strictly required. Just nice to have

Luke Boswell (Feb 23 2025 at 22:04):

This approach with toStr is my second attempt but taking a different direction.

I'm not sure how important it will be for parsing these IR's. That seems to be a tangental requirement in the nice to have category -- but also feels like a rabbit hole in itself.

I can appreciate the idea of build a list of tokens instead of writing directly to a buffer -- I was thinking this enables us to implement checks to ensure everything is well formed.

I am not opposed to returning to try the SExpr IR idea again, I think I would simplify it a littler further, only have the "node" itself a comptime type variable.

Brendan Hansknecht (Feb 23 2025 at 22:06):

Why does anything need comptime? (I think the comptime free IR I shared on the PR should be enough)

Brendan Hansknecht (Feb 23 2025 at 22:09):

I don't think we need to write any of the parsing right now. I think we can just write a basic node type, conversion logic from parse ir to that basic node type, and a printer.

Brendan Hansknecht (Feb 23 2025 at 22:09):

Brendan Hansknecht (Feb 23 2025 at 22:10):

Luke Boswell (Feb 23 2025 at 22:11):

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

All this said, I don't want to block anything, but I don't understand the issue with a super simple sexpr ir as an intermediate.

Luke Boswell (Feb 23 2025 at 22:11):

I added the comptime so you could pass in a "parser" function to take the []u8 and give you back the nodes you expect -- or an error if it didn't recognise the node string

Luke Boswell (Feb 23 2025 at 22:12):

Brendan Hansknecht (Feb 23 2025 at 22:12):

Oh, I would just separately parse to sexpr ir, then simply pattern match on sexpr ir to convert to the parse ir.

Brendan Hansknecht (Feb 23 2025 at 22:13):

so parsing sexpr ir would accept all node types and names. Only the final conversion would invalidate specific like node types

Luke Boswell (Feb 23 2025 at 22:13):

Jared Ramirez (Feb 24 2025 at 03:06):

to use sexprs to build IRs for later stages (anything after solve), then will it need to parse nodes that contain type information as well?

Brendan Hansknecht (Feb 24 2025 at 03:08):

Though we can make type info a child node or side band if necessary (as long as we make a way to print it)

Brendan Hansknecht (Feb 24 2025 at 03:09):

Or if may be the case that we don't support parsing later IRs and they simply print in a lossy form. Haven't fully thought through this

Luke Boswell (Feb 24 2025 at 04:00):

I've been thinking along these lines as one of the reasons why we may not want to "parse" the IR's.

Luke Boswell (Feb 24 2025 at 04:01):

The other thought is that these SExpr IR's aren't very nice to read. I'm starting to think it might be nicer just to make something more human readable that is unique for each IR that just includes the context relevant for that stage.

Jared Ramirez (Feb 24 2025 at 04:15):

I was thinking a similar thing as I’ve been hand-writing some IRs (while exploring the function solving stage). It seems like sexprs that contain ast _plus_ type info could get in unwieldy.

Jared Ramirez (Feb 24 2025 at 04:24):

And each stage has different data, so there’s one main parser, it would need to support a bunch of nodes that are maybe only applicable to a single stage

Joshua Warner (Feb 24 2025 at 04:39):

My thinking was to do sexprs with perhaps some tasteful extensions. Keep it simple to print and parse but perhaps slightly less verbose

Brendan Hansknecht (Feb 24 2025 at 04:46):

Brendan Hansknecht (Feb 24 2025 at 04:47):

Goal one is definitely snapshot tests (which requires printing and some reliability, but doesn't need to be perfect).

Brendan Hansknecht (Feb 24 2025 at 04:47):

Goal two is parsing for better fuzzing (probably also requires verification, which may actually not be reasonable).

Brendan Hansknecht (Feb 24 2025 at 04:49):

I think we could minorly expand expressions to support types and that would be reasonable (something to worry about later)

Brendan Hansknecht (Feb 24 2025 at 04:50):

Brendan Hansknecht (Feb 24 2025 at 04:51):

That said, the simplicity of bespoke printing and no parsing is also reasonable as long as we try to be consistent with printing.

Brendan Hansknecht (Feb 24 2025 at 04:51):

Luke Boswell (Feb 24 2025 at 08:35):

I've got something simple that is working ok. I'm going to continue with converting it into some small snapshots and trying a simple heuristic for pretty printing.

I'm busy all day tomorrow, but should be able to make some progress later in the week.

Luke Boswell (Feb 28 2025 at 05:00):

I've updated my PR and would like to merge what I have to prevent it going stale again.

I haven't quite figured out how to get the string parts back to working yet, but the rest is working well.

I also added an option to give either compact or pretty formatted strings. So now our unit test looks like this.

The next step is to generate actual snapshot files instead of having a unit test. But I'm pretty happy now with the API I think.

@Brendan Hansknecht can you cast your eye over my implementation - particularly the children: std.ArrayListUnmanaged(Node) part... :pray:

test "example s-expr" {
    const source =
        \\module [foo, bar]
        \\
        \\foo = "hey"
        \\bar = "yo"
    ;

    const expected =
        \\(file
        \\    (header
        \\        'foo'
        \\        'bar')
        \\    (decl
        \\        (ident
        \\            'foo')
        \\        (string_part))
        \\    (decl
        \\        (ident
        \\            'bar')
        \\        (string_part)))
    ;

    try testSExprHelper(source, expected);
}

Luke Boswell (Mar 01 2025 at 01:34):

This PR adds a snapshot tool that iterates through all the snapshot text files in src/snapshots/*.txt and re-generates the AST/IR's for each compiler stage.

$ zig build snapshot
info: processed 2 snapshots in 1ms.
$ zig build snapshot -- --verbose
info: src/snapshots/002.txt
info: src/snapshots/001.txt
info: processed 2 snapshots in 2ms.

~~~META
description=Basic example to develop the snapshot methodology
~~~SOURCE
module [foo, bar]
foo = "one"
bar = "two"
~~~PARSE
(file
    (header
        'foo'
        'bar')
    (decl
        (ident
            'foo')
        (string_part))
    (decl
        (ident
            'bar')
        (string_part)))

Luke Boswell (Mar 01 2025 at 02:40):

@Isaac Van Doren -- how would you feel about me adding FORMAT as a section in these snapshot tests, instead of having a unit tests that write out to another file?

Luke Boswell (Mar 01 2025 at 02:41):

If we think that's a good idea, I'm down to give it a try. We can land the format cli PR, I can rebase my snapshot PR and try it out

Luke Boswell (Mar 01 2025 at 02:41):

I guess I'm thinking of the "snapshot" tool as like our integration test suite...

Brendan Hansknecht (Mar 01 2025 at 02:43):

Would also be great to make the format section optional. If the section is missing, it will then be required that the source section formats to itself.

Luke Boswell (Mar 01 2025 at 02:44):

So like, we put un-formatted code in the SOURCE section... it formats it and adds to the FORMATED section.

Luke Boswell (Mar 01 2025 at 02:44):

Luke Boswell (Mar 01 2025 at 02:45):

Did you have another idea in mind @Brendan Hansknecht ? why would we want it optional?

Luke Boswell (Mar 01 2025 at 02:47):

Or maybe if the formatted code is identical we don't bother writing out the section... but I wonder if it's cheaper just to dump the bytes out than compare the strings.

Brendan Hansknecht (Mar 01 2025 at 02:47):

I guess to work with git diff, if only a SOURCE section. The process is read source, format it, and write it back to SOURCE (only source means it is already formatted correctly). This avoids printing the same chunk of code twice in a row if the formatting is already good.

If a FORMATTED section exist, we expect the SOURCE to change when formatted. So we need to write out both sections and they aren't redundant.

Luke Boswell (Mar 01 2025 at 02:48):

Ahk that's a good idea. That way we will know if the formatting changes, but not duplicate things unnecessarily.

Brendan Hansknecht (Mar 01 2025 at 02:48):

Maybe that isn't needed, but I feel like for long examples, it will be nice to not print the source twice

Luke Boswell (Mar 01 2025 at 02:50):

FYI @Isaac Van Doren I merged your PR -- I'll adapt your integration tests into the snapshot tool as above.

Luke Boswell (Mar 01 2025 at 02:51):

@Brendan Hansknecht -- I assumed we want to build the snapshot tool by default so you can run the integration tests easily

Luke Boswell (Mar 01 2025 at 02:51):

Luke Boswell (Mar 01 2025 at 02:52):

I guess it should probably just sit behind zig build test also? or might we want to keep that for unit tests only?

Isaac Van Doren (Mar 01 2025 at 03:04):

Isaac Van Doren (Mar 01 2025 at 03:06):

But it won't be testing the same behavior as the test I wrote so I think we should keep it. That test wasn't meant to really check anything about the actual formatting itself, just that roc format correctly reads in the specified file and writes back out the formatted version.

Isaac Van Doren (Mar 01 2025 at 03:08):

Originally that test was e2e (it actually built the roc executable and invoked it). I think it would be good to stand up a separate e2e suite for the CLI which would be a good place for tests like that to live

Brendan Hansknecht (Mar 01 2025 at 03:14):

Just thinking if someone only wants to build roc, they can just zig build and that can be roc and no other wasted time

Isaac Van Doren (Mar 01 2025 at 03:18):

Brendan Hansknecht (Mar 01 2025 at 03:23):

I feel like it would be pretty redundant with all of the snapshot tests. Not bad to add a few of, but it is probably preferable to avoid too many scattered tests

Brendan Hansknecht (Mar 01 2025 at 03:25):

If we put it behind, zig build test we just need to make it sure it is fast enough. Might hurt the dev loop, in which case, probably should be zig build test -Dsnapshot. I think really the separation is for ensure fast feedback.

I assume eventually we will have enough snapshot tests we will want to separate it out, but probably fine to start unified and only separate out if it gets too slow.

Luke Boswell (Mar 01 2025 at 03:34):

Isaac Van Doren (Mar 01 2025 at 03:47):

My intention would be to focus on focusing on the CLI/orchestration logic that wouldn't be tested by the snapshots at all, so I don't think there would be much overlap there. I wouldn't try to make it exhaustive, just for some confidence that changes to the CLI are correct without having to manually test everything.

Brendan Hansknecht (Mar 01 2025 at 03:47):

Ah, I thought you meant specifically formatting tests. Like another suite of tests but just for formatting.

Brendan Hansknecht (Mar 01 2025 at 03:48):

But like tests that format examples/stdlib (folder vs file vs invalid) or exercise various cli args sound great

Isaac Van Doren (Mar 01 2025 at 03:49):

Luke Boswell (Mar 01 2025 at 23:25):

Anyone looking for something to help out with, we should set up our snapshot tool to run in CI with the other zig things.

Luke Boswell (Mar 01 2025 at 23:26):

I think the next thing I'd like to tackle is parsing just an expression (not a full file), and translate some of our syntax snapshot tests across to work on improving coverage there.

Anton (Mar 03 2025 at 09:06):

Luke Boswell (Mar 03 2025 at 10:11):

Anton (Mar 03 2025 at 10:17):

Isaac Van Doren (Mar 03 2025 at 13:18):

zig build snapshot updates the golden values right? Why do we want to run it in CI?

Brendan Hansknecht (Mar 03 2025 at 16:40):

You run it, then a few extra git commands to ensure that none of the snapshot files changed

Brendan Hansknecht (Mar 03 2025 at 16:41):

Isaac Van Doren (Mar 03 2025 at 16:47):

Ah I see. I was imagining that there would be a command that would explicitly check that the snapshots matched the current output without requiring you to manually diff them

Joshua Warner (Mar 04 2025 at 15:19):

Luke Boswell (Mar 06 2025 at 08:04):

For example in crates/compiler/test_syntax/tests/snapshots/pass/add_var_with_spaces.expr.roc we have a single expression

I was thinking of migrating these across one by one as snapshots and using them to help add parser functionality.

Luke Boswell (Mar 06 2025 at 08:15):

Or perhaps we would want something like this instead... so turn them into a full module.

~~~META
description=Add a variable with spaces
~~~SOURCE
module [add2]

add2 = x + 2
~~~PARSE
(file
    (header 'add2')
    (decl
        (ident 'add2')
        (binop
            '+'
            (ident '' 'x')
            (int '2'))))

Luke Boswell (Mar 06 2025 at 08:18):

Also -- minor tangent while I'm looking at it... would we prefer (binop '+' <lhs> <rhs>) ... or switch on the token and make it more like (plus <lhs> <rhs>)

Sam Mohr (Mar 06 2025 at 08:21):

I'd vote for the former (AKA (binop ...)) since it's more consistent with how the rest of the S-expr stuff seems to work, and it gets desugared immediately in the next stage, so it's minor anyway

Anthony Bullard (Mar 06 2025 at 10:31):

Definitely want to be able to parse single syntax nodes (exprs, statements, header, etc)

Joshua Warner (Mar 06 2025 at 16:12):

Luke Boswell (Mar 07 2025 at 03:04):

~~~META
description=Various type annotations
~~~SOURCE
module []

foo : U64
bar : Thing(a, b, _)
baz : (a, b, c)
add_one : (U8, U16 -> U32)
main! : List(String) -> Result({}, _)
~~~PARSE
(file
    (header)
    (type_anno
        'foo'
        (tag 'U64'))
    (type_anno
        'bar'
        (tag
            'Thing'
            (ty_var 'a')
            (ty_var 'b')
            (_)))
    (type_anno
        'baz'
        (tuple
            (ty_var 'a')
            (ty_var 'b')
            (ty_var 'c')))
    (type_anno
        'add_one'
        (fn
            (tag 'U32')
            (tag 'U8')
            (tag 'U16')))
    (type_anno
        'main!'
        (fn
            (tag
                'Result'
                (record)
                (_))
            (tag
                'List'
                (tag 'String')))))

Luke Boswell (Mar 07 2025 at 03:06):

Some simplifications I'm debating rn (just for the snapshots, not renaming tag unions in the zig source or anything)

There's a lot of design space here. It's nice that this is all one way and so easy to update the snapshots.

Anton (Mar 08 2025 at 09:39):

type_anno and ty_var are a lot clearer, I wouldn't change them if we don't experience noticeable problems due to verbosity.

Luke Boswell (Mar 08 2025 at 22:35):

I'd just like to say in case anyone is wondering Re the current state of snapshots, because they look somewhat strange.

I've just been dumping things in there to help figure out syntax issues with the parser and seed the fuzzer etc.

The current snapshots are pretty expendable and we will probably blow them away soon or change them a lot when we start having a usable Can etc.

Anthony Bullard (Mar 10 2025 at 13:43):

I think it's this output is really nice, but I _personally_ would prefer double quotes for strings. Are we using singles just to avoid having to worry about escaping?

Luke Boswell (Mar 11 2025 at 03:36):

Just wanted to show off a little... checkout this snapshot. I'm pretty happy with how this is starting to look. It's really easy to see at a glance where the issues are. You can feed inputs from fuzz crashes and dissect what the compiler is doing pretty quickly -- which has helped me find and fix a number of issues.

~~~META
description=fuzz crash
~~~SOURCE
H{o,
    ]

foo =

    "on        (string 'onmo %')))
~~~PROBLEMS
TOKENIZE: (2:4-2:4) AsciiControl:

    ]
   ^
TOKENIZE: (1:6-1:6) Expected the correct closing brace here:

    ]
      ^

TOKENIZE: (6:7-6:36) UnclosedString:

    "on        (string 'onmo %')))
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
PARSER: missing_header
~~~TOKENS
UpperIdent,OpenCurly,LowerIdent,Comma,Newline,CloseCurly,Newline,LowerIdent,OpAssign,Newline,StringStart,StringPart,EndOfFile
~~~PARSE
(file
    (malformed_header 'missing_header')
    (record (field 'o'))
    (decl
        (ident 'foo')
        (string 'on        (string 'onmo %')))')))
~~~FORMATTED
{ o }

foo = "on        (string 'onmo %')))"
~~~END

Luke Boswell (Mar 11 2025 at 03:52):

Anthony Bullard (May 13 2025 at 22:41):

Hey @Luke Boswell I'm dealing with an issue with Zig where TLDR we want to move to using TAB as the canonical indentation character in the formatter but Zig multiline string literals do not support using TAB in them. So long story short I need to move all of the current fmt tests to snapshots (I don't have to per se, but I think it's the logical move).

In order to do that I am moving to make snapshots support different IR nodes like the old test_syntax snapshots did. Are you aligned with this? It means that I need to expose some extra methods on the Parse IR so that we can output the SExpr and format correctly.

Luke Boswell (May 14 2025 at 07:11):

Luke Boswell (May 14 2025 at 07:12):

Luke Boswell (May 14 2025 at 07:13):

That was my intention too, but I figured you wanted the kitchen sink in there because it was easier to work with while developing the Parser initial implementation.

Anthony Bullard (May 14 2025 at 10:15):

sorry i mean have it support snapshots of just exprs, statements, or headers instead of just modules

Stream: compiler development

Topic: zig compiler - IR serde

Norbert Hajagos (Feb 16 2025 at 09:08):

Sam Mohr (Feb 16 2025 at 09:25):

Sam Mohr (Feb 16 2025 at 09:29):

Sam Mohr (Feb 16 2025 at 09:33):

Norbert Hajagos (Feb 16 2025 at 13:39):

Brendan Hansknecht (Feb 16 2025 at 16:10):

Brendan Hansknecht (Feb 16 2025 at 16:11):

Norbert Hajagos (Feb 16 2025 at 16:13):

Brendan Hansknecht (Feb 16 2025 at 16:17):

Norbert Hajagos (Feb 16 2025 at 16:23):

Brendan Hansknecht (Feb 16 2025 at 16:23):

Brendan Hansknecht (Feb 16 2025 at 16:25):

Norbert Hajagos (Feb 16 2025 at 16:29):

Brendan Hansknecht (Feb 16 2025 at 16:48):

Brendan Hansknecht (Feb 16 2025 at 16:48):

Joshua Warner (Feb 16 2025 at 17:45):

Norbert Hajagos (Feb 16 2025 at 17:55):

Notification Bot (Feb 16 2025 at 17:58):

Joshua Warner (Feb 16 2025 at 18:01):

Norbert Hajagos (Feb 16 2025 at 18:01):

Joshua Warner (Feb 16 2025 at 18:01):

Isaac Van Doren (Feb 16 2025 at 19:09):

Brendan Hansknecht (Feb 16 2025 at 19:13):

Brendan Hansknecht (Feb 16 2025 at 19:15):

Brendan Hansknecht (Feb 16 2025 at 19:15):

Brendan Hansknecht (Feb 16 2025 at 19:57):

Norbert Hajagos (Feb 16 2025 at 20:03):

Norbert Hajagos (Feb 16 2025 at 20:25):

Luke Boswell (Feb 16 2025 at 20:30):

Norbert Hajagos (Feb 16 2025 at 20:33):

Sam Mohr (Feb 16 2025 at 20:34):

Sam Mohr (Feb 16 2025 at 20:35):

Norbert Hajagos (Feb 16 2025 at 20:37):

Sam Mohr (Feb 16 2025 at 20:40):

Norbert Hajagos (Feb 16 2025 at 22:41):

Richard Feldman (Feb 16 2025 at 22:48):

Richard Feldman (Feb 16 2025 at 22:48):

Sam Mohr (Feb 16 2025 at 22:48):

Sam Mohr (Feb 16 2025 at 22:49):

Sam Mohr (Feb 16 2025 at 22:49):

Richard Feldman (Feb 16 2025 at 22:50):

Richard Feldman (Feb 16 2025 at 22:50):

Richard Feldman (Feb 16 2025 at 22:50):

Richard Feldman (Feb 16 2025 at 22:51):

Brendan Hansknecht (Feb 16 2025 at 23:38):

Brendan Hansknecht (Feb 16 2025 at 23:39):

Brendan Hansknecht (Feb 16 2025 at 23:39):

Brendan Hansknecht (Feb 16 2025 at 23:40):

Brendan Hansknecht (Feb 16 2025 at 23:42):

Richard Feldman (Feb 16 2025 at 23:44):

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

Sam Mohr (Feb 16 2025 at 23:47):

Brendan Hansknecht (Feb 16 2025 at 23:48):

Sam Mohr (Feb 16 2025 at 23:48):

Sam Mohr (Feb 16 2025 at 23:49):

Brendan Hansknecht (Feb 16 2025 at 23:49):

Sam Mohr (Feb 16 2025 at 23:50):

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

Sam Mohr (Feb 16 2025 at 23:52):

Brendan Hansknecht (Feb 16 2025 at 23:52):

Brendan Hansknecht (Feb 16 2025 at 23:52):

Brendan Hansknecht (Feb 16 2025 at 23:53):

Sam Mohr (Feb 16 2025 at 23:53):

Sam Mohr (Feb 16 2025 at 23:54):

Sam Mohr (Feb 16 2025 at 23:54):

Sam Mohr (Feb 16 2025 at 23:55):

Sam Mohr (Feb 16 2025 at 23:55):

Sam Mohr (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:56):

Sam Mohr (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:56):

Brendan Hansknecht (Feb 16 2025 at 23:59):

Sam Mohr (Feb 17 2025 at 00:00):

Sam Mohr (Feb 17 2025 at 00:00):

Richard Feldman (Feb 17 2025 at 00:02):

Brendan Hansknecht (Feb 17 2025 at 00:03):

Richard Feldman (Feb 17 2025 at 00:04):