Skip to content

Commit

Permalink
support iterating over maps
Browse files Browse the repository at this point in the history
  • Loading branch information
Vexu committed Jul 6, 2020
1 parent 3c14114 commit 34d729f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
21 changes: 19 additions & 2 deletions src/value.zig
Expand Up @@ -95,7 +95,24 @@ pub const Value = union(Type) {
.str = str[iter.index - cp_len .. iter.index],
};
},
.map => {},
.map => |*map| {
if (iter.index == map.entries.items.len) {
res.* = &Value.None;
return;
}

if (res.* == null)
res.* = try vm.gc.alloc();
if (iter.index == 0) {
res.*.?.* = .{ .tuple = try vm.gc.gpa.alloc(*Value, 2) };
}
const e = &map.entries.items[iter.index];
const t = res.*.?.tuple;
// removing `const` on `Map` causes dependency loop??
t[0] = @intToPtr(*Value, @ptrToInt(e.key));
t[1] = e.value;
iter.index += 1;
},
.range => @panic("TODO: range iterator"),
else => unreachable,
}
Expand Down Expand Up @@ -123,7 +140,7 @@ pub const Value = union(Type) {
switch (value.*) {
.int, .num, .none, .bool, .native, .tagged, .err, .range, .iterator => {},
.tuple => |t| allocator.free(t),
// I really, really hate `deinit` taking a mutable pointer.
// I really, really hate `deinit` taking a mutable pointer.

This comment has been minimized.

Copy link
@andrewrk

andrewrk Jul 6, 2020

you could do this pattern:

var moved = const_object;
moved.deinit();

But here's an argument for deinit taking a mutable pointer:

  • init() takes a mutable pointer to undefined memory and initializes state
  • deinit() takes a mutable pointer to initialized state and makes it undefined memory again

On the other hand, a lot of people agree with you and it's probably worth making a proposal

This comment has been minimized.

Copy link
@Vexu

Vexu Jul 7, 2020

Author Owner

I dislike how it makes forces you to remove constness like in Vexu/zuri@a309b5a or do some copying like you suggested, but I ended up discovering a double free thanks to it almost immediately after writing this so I think I'm ready to embrace the change.

Also it seems like value isn't even const in this version so I'm not sure why I was having problems with this.

.map => |*m| m.deinit(allocator),
.list => |*l| l.deinit(allocator),
.str => {
Expand Down
13 changes: 13 additions & 0 deletions tests/behavior.zig
@@ -1,3 +1,16 @@
test "map iterator" {
expectOutput(
\\const map = {1: 2, 3: 4, 5: 6}
\\let sum = 0
\\for (let (k, v) in map)
\\ sum += k
\\ sum *= v
\\return sum
,
\\150
);
}

test "list comprehension" {
expectOutput(
\\return for (const c in "hello") c
Expand Down

0 comments on commit 34d729f

Please sign in to comment.