Skip to content

Conversation

@UnusualEgg
Copy link
Contributor

uses var a = [ .foo = bar ]; syntax. Nested structs give a segfault for some reason though. #39

@UnusualEgg UnusualEgg marked this pull request as draft January 13, 2026 20:22
@UnusualEgg UnusualEgg marked this pull request as ready for review January 13, 2026 20:43
@ikskuh
Copy link
Owner

ikskuh commented Jan 24, 2026

Wow, not bad!

Nested structs give a segfault for some reason though.

That might be because the load/store sequence is broken, but i'm not sure.

a.b.c = 10;

must perform a struct_load, struct_load, push 10, struct store, struct store, write local which might break.

I'm fine with this, we can just say it's a problem and we're fine with it for now.


var iter = self.contents.iterator();
while (iter.next()) |entry| {
arr.contents.putAssumeCapacity(try arr.allocator.dupe(u8, entry.key_ptr.*), try entry.value_ptr.clone());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential memory leak here when arr.allocator.dupe fails to clone the key.

You gotta construct both locally first and errdefer-free them, then put the key/value pair

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putAssumeCapacity can't fail. You would just need to errdefer duped_key.deinit() before adding the key and value to arr

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even then, it's a bug right now :)

i'd just use two errdefers, and make it so that future expansions dont have to think about the cleanup.

the compiler won't emit the second errdefer anyways until there's an error


var iter = self.contents.iterator();
while (iter.next()) |entry| {
arr.contents.putAssumeCapacity(try arr.allocator.dupe(u8, entry.key_ptr.*), try entry.value_ptr.clone());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
arr.contents.putAssumeCapacity(try arr.allocator.dupe(u8, entry.key_ptr.*), try entry.value_ptr.clone());
const duped_key = try arr.allocator.dupe(u8, entry.key_ptr.*);
errdefer arr.allocator.free(duped_key);
arr.contents.putAssumeCapacity(duped_key, try entry.value_ptr.clone());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants