diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index fce65f159b..7e62e09b46 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -8901,14 +8901,28 @@ pub const Interpreter = struct { // Provide closure context for capture lookup try self.active_closures.append(func_val); - // Bind parameters + // Bind parameters using pattern matching to handle destructuring for (params, 0..) |param, idx| { - try self.bindings.append(.{ - .pattern_idx = param, - .value = arg_values[idx], - .expr_idx = @enumFromInt(0), - .source_env = self.env, - }); + // Get the runtime type for this parameter + const param_rt_var = if (ci.arg_rt_vars_to_free) |vars| + (if (idx < vars.len) vars[idx] else try self.translateTypeVar(self.env, can.ModuleEnv.varFrom(param))) + else + try self.translateTypeVar(self.env, can.ModuleEnv.varFrom(param)); + + // Use patternMatchesBind to properly handle complex patterns (e.g., list destructuring) + // patternMatchesBind borrows the value and creates copies for bindings, so we need to + // decref the original arg_value after successful binding + if (!try self.patternMatchesBind(param, arg_values[idx], param_rt_var, roc_ops, &self.bindings, @enumFromInt(0))) { + // Pattern match failed - cleanup and error + self.env = saved_env; + _ = self.active_closures.pop(); + func_val.decref(&self.runtime_layout_store, roc_ops); + for (arg_values) |arg| arg.decref(&self.runtime_layout_store, roc_ops); + if (ci.arg_rt_vars_to_free) |vars| self.allocator.free(vars); + return error.TypeMismatch; + } + // Decref the original argument value since patternMatchesBind made copies + arg_values[idx].decref(&self.runtime_layout_store, roc_ops); } // Push cleanup continuation, then evaluate body @@ -8942,15 +8956,6 @@ pub const Interpreter = struct { // Body triggered early return - use that value self.early_return_value = null; - // Decref parameter bindings - var k = cleanup.param_count; - while (k > 0) { - k -= 1; - if (self.bindings.pop()) |binding| { - binding.value.decref(&self.runtime_layout_store, roc_ops); - } - } - // Pop active closure if needed if (cleanup.has_active_closure) { if (self.active_closures.pop()) |closure_val| { @@ -8964,9 +8969,11 @@ pub const Interpreter = struct { self.rigid_subst = saved; } - // Restore environment and free arg_rt_vars + // Restore environment and cleanup bindings + // Use trimBindingList to properly decref all bindings created by pattern matching + // (which may be more than param_count due to destructuring) self.env = cleanup.saved_env; - self.bindings.shrinkRetainingCapacity(cleanup.saved_bindings_len); + self.trimBindingList(&self.bindings, cleanup.saved_bindings_len, roc_ops); if (cleanup.arg_rt_vars_to_free) |vars| self.allocator.free(vars); try value_stack.push(return_val); @@ -8976,15 +8983,6 @@ pub const Interpreter = struct { // Normal return - result is on value stack const result = value_stack.pop() orelse return error.Crash; - // Decref parameter bindings - var k = cleanup.param_count; - while (k > 0) { - k -= 1; - if (self.bindings.pop()) |binding| { - binding.value.decref(&self.runtime_layout_store, roc_ops); - } - } - // Pop active closure if needed if (cleanup.has_active_closure) { if (self.active_closures.pop()) |closure_val| { @@ -8998,9 +8996,11 @@ pub const Interpreter = struct { self.rigid_subst = saved; } - // Restore environment and free arg_rt_vars + // Restore environment and cleanup bindings + // Use trimBindingList to properly decref all bindings created by pattern matching + // (which may be more than param_count due to destructuring) self.env = cleanup.saved_env; - self.bindings.shrinkRetainingCapacity(cleanup.saved_bindings_len); + self.trimBindingList(&self.bindings, cleanup.saved_bindings_len, roc_ops); if (cleanup.arg_rt_vars_to_free) |vars| self.allocator.free(vars); try value_stack.push(result); @@ -9644,6 +9644,10 @@ pub const Interpreter = struct { list_value.decref(&self.runtime_layout_store, roc_ops); return error.TypeMismatch; } + // Decref the element after successful pattern matching. + // patternMatchesBind creates copies via pushCopy which increfs, so the original + // incref we did above is now an extra reference that needs to be released. + elem_value.decref(&self.runtime_layout_store, roc_ops); // Push body_done continuation try work_stack.push(.{ .apply_continuation = .{ .for_loop_body_done = .{ @@ -9716,6 +9720,10 @@ pub const Interpreter = struct { fl.list_value.decref(&self.runtime_layout_store, roc_ops); return error.TypeMismatch; } + // Decref the element after successful pattern matching. + // patternMatchesBind creates copies via pushCopy which increfs, so the original + // incref we did above is now an extra reference that needs to be released. + elem_value.decref(&self.runtime_layout_store, roc_ops); // Push body_done continuation for next iteration try work_stack.push(.{ .apply_continuation = .{ .for_loop_body_done = .{ diff --git a/src/eval/render_helpers.zig b/src/eval/render_helpers.zig index 2d6e222dd9..9df01da2dd 100644 --- a/src/eval/render_helpers.zig +++ b/src/eval/render_helpers.zig @@ -292,37 +292,68 @@ pub fn renderValueRocWithType(ctx: *RenderCtx, value: StackValue, rt_var: types. } }, .record => |rec| { - const ext_resolved = ctx.runtime_types.resolveVar(rec.ext); - const use_placeholder = switch (ext_resolved.desc.content) { - .structure => |st| st != .empty_record, - else => true, - }; - if (use_placeholder) { - return try gpa.dupe(u8, ""); + // Gather all record fields by following the extension chain + var all_fields = std.array_list.AlignedManaged(types.RecordField, null).init(gpa); + defer all_fields.deinit(); + + // Add fields from the initial record + const initial_fields = ctx.runtime_types.getRecordFieldsSlice(rec.fields); + for (initial_fields.items(.name), initial_fields.items(.var_)) |name, var_| { + try all_fields.append(.{ .name = name, .var_ = var_ }); } - var out = std.array_list.AlignedManaged(u8, null).init(gpa); - errdefer out.deinit(); - try out.appendSlice("{ "); - var acc = try value.asRecord(ctx.layout_store); - const fields = ctx.runtime_types.getRecordFieldsSlice(rec.fields); - var i: usize = 0; - while (i < fields.len) : (i += 1) { - const f = fields.get(i); - const name_text = ctx.env.getIdent(f.name); - try out.appendSlice(name_text); - try out.appendSlice(": "); - if (acc.findFieldIndex(f.name)) |idx| { - const field_val = try acc.getFieldByIndex(idx); - const rendered = try renderValueRoc(ctx, field_val); - defer gpa.free(rendered); - try out.appendSlice(rendered); - } else { - try out.appendSlice(""); + + // Follow the extension chain to gather all fields + var ext = rec.ext; + var is_valid = true; + while (is_valid) { + const ext_resolved = ctx.runtime_types.resolveVar(ext); + switch (ext_resolved.desc.content) { + .structure => |flat_type| switch (flat_type) { + .record => |ext_record| { + const ext_fields = ctx.runtime_types.getRecordFieldsSlice(ext_record.fields); + for (ext_fields.items(.name), ext_fields.items(.var_)) |name, var_| { + try all_fields.append(.{ .name = name, .var_ = var_ }); + } + ext = ext_record.ext; + }, + .empty_record => break, // Reached the end of the extension chain + else => { + is_valid = false; + }, + }, + .alias => |alias| { + // Follow alias to its backing type + ext = ctx.runtime_types.getAliasBackingVar(alias); + }, + else => { + is_valid = false; + }, } - if (i + 1 < fields.len) try out.appendSlice(", "); } - try out.appendSlice(" }"); - return out.toOwnedSlice(); + + if (is_valid and all_fields.items.len > 0) { + var out = std.array_list.AlignedManaged(u8, null).init(gpa); + errdefer out.deinit(); + try out.appendSlice("{ "); + var acc = try value.asRecord(ctx.layout_store); + for (all_fields.items, 0..) |f, i| { + const name_text = ctx.env.getIdent(f.name); + try out.appendSlice(name_text); + try out.appendSlice(": "); + if (acc.findFieldIndex(f.name)) |idx| { + const field_val = try acc.getFieldByIndex(idx); + const rendered = try renderValueRocWithType(ctx, field_val, f.var_); + defer gpa.free(rendered); + try out.appendSlice(rendered); + } else { + try out.appendSlice(""); + } + if (i + 1 < all_fields.items.len) try out.appendSlice(", "); + } + try out.appendSlice(" }"); + return out.toOwnedSlice(); + } + // Fall through to renderValueRoc which can use layout info }, else => {}, }; diff --git a/src/eval/test/eval_test.zig b/src/eval/test/eval_test.zig index 3361fba32f..7bc5bab8a1 100644 --- a/src/eval/test/eval_test.zig +++ b/src/eval/test/eval_test.zig @@ -27,6 +27,8 @@ const runExpectInt = helpers.runExpectInt; const runExpectBool = helpers.runExpectBool; const runExpectError = helpers.runExpectError; const runExpectStr = helpers.runExpectStr; +const runExpectRecord = helpers.runExpectRecord; +const ExpectedField = helpers.ExpectedField; const TraceWriterState = struct { buffer: [256]u8 = undefined, @@ -973,3 +975,275 @@ test "nominal type equality - nested structures with Bool" { try runExpectBool("{ outer: { inner: Bool.True } } == { outer: { inner: Bool.False } }", false, .no_trace); try runExpectBool("((Bool.True, Bool.False), Bool.True) == ((Bool.True, Bool.False), Bool.True)", true, .no_trace); } + +// Tests for List.fold with record accumulators +// This exercises record state management within fold operations + +test "List.fold with record accumulator - sum and count" { + // Test folding a list while accumulating sum and count in a record + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 6 }, + .{ .name = "count", .value = 3 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3], {sum: 0, count: 0}, |acc, item| {sum: acc.sum + item, count: acc.count + 1})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - empty list" { + // Folding an empty list should return the initial record unchanged + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 0 }, + .{ .name = "count", .value = 0 }, + }; + try runExpectRecord( + "List.fold([], {sum: 0, count: 0}, |acc, item| {sum: acc.sum + item, count: acc.count + 1})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - single field" { + // Test with a single-field record accumulator + const expected_fields = [_]ExpectedField{ + .{ .name = "total", .value = 10 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3, 4], {total: 0}, |acc, item| {total: acc.total + item})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - record update syntax" { + // Test using record update syntax { ..acc, field: newValue } + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 6 }, + .{ .name = "count", .value = 3 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3], {sum: 0, count: 0}, |acc, item| {..acc, sum: acc.sum + item, count: acc.count + 1})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - partial update" { + // Test updating only one field while keeping others + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 10 }, + .{ .name = "multiplier", .value = 2 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3, 4], {sum: 0, multiplier: 2}, |acc, item| {..acc, sum: acc.sum + item})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - nested field access" { + // Test accessing nested record fields in accumulator + const expected_fields = [_]ExpectedField{ + .{ .name = "value", .value = 6 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3], {value: 0}, |acc, item| {value: acc.value + item})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - three fields" { + // Test with more fields to exercise record layout handling + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 10 }, + .{ .name = "count", .value = 4 }, + .{ .name = "product", .value = 24 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3, 4], {sum: 0, count: 0, product: 1}, |acc, item| {sum: acc.sum + item, count: acc.count + 1, product: acc.product * item})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - conditional update" { + // Test conditional logic inside the fold with record accumulator + const expected_fields = [_]ExpectedField{ + .{ .name = "evens", .value = 6 }, + .{ .name = "odds", .value = 4 }, + }; + try runExpectRecord( + "List.fold([1, 2, 3, 4], {evens: 0, odds: 0}, |acc, item| if item % 2 == 0 {evens: acc.evens + item, odds: acc.odds} else {evens: acc.evens, odds: acc.odds + item})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - string list" { + // Test folding over strings with a record accumulator (count only) + const expected_fields = [_]ExpectedField{ + .{ .name = "count", .value = 3 }, + }; + try runExpectRecord( + "List.fold([\"a\", \"bb\", \"ccc\"], {count: 0}, |acc, _| {count: acc.count + 1})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - record equality comparison" { + // Test comparing the result of a fold that produces a record + // This exercises the record equality code path + try runExpectBool( + "List.fold([1, 2, 3], {sum: 0}, |acc, item| {sum: acc.sum + item}) == {sum: 6}", + true, + .no_trace, + ); +} + +test "List.fold with record accumulator - record inequality comparison" { + // Test that fold result can be compared for inequality + try runExpectBool( + "List.fold([1, 2, 3], {sum: 0}, |acc, item| {sum: acc.sum + item}) == {sum: 5}", + false, + .no_trace, + ); +} + +test "List.fold with record accumulator - multi-field record equality" { + // Test equality comparison with multi-field record result + try runExpectBool( + "List.fold([1, 2], {a: 0, b: 10}, |acc, item| {a: acc.a + item, b: acc.b - item}) == {a: 3, b: 7}", + true, + .no_trace, + ); +} + +// Tests for List.fold with record accumulators and list/record destructuring +// This exercises pattern matching within fold operations + +test "List.fold with record accumulator - record destructuring in lambda" { + // Test folding over a list of records, destructuring each record in the lambda + const expected_fields = [_]ExpectedField{ + .{ .name = "total_x", .value = 6 }, + .{ .name = "total_y", .value = 15 }, + }; + try runExpectRecord( + "List.fold([{x: 1, y: 2}, {x: 2, y: 5}, {x: 3, y: 8}], {total_x: 0, total_y: 0}, |acc, {x, y}| {total_x: acc.total_x + x, total_y: acc.total_y + y})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - partial record destructuring" { + // Test destructuring only some fields from records + const expected_fields = [_]ExpectedField{ + .{ .name = "sum", .value = 6 }, + }; + try runExpectRecord( + "List.fold([{a: 1, b: 100}, {a: 2, b: 200}, {a: 3, b: 300}], {sum: 0}, |acc, {a}| {sum: acc.sum + a})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - single field record destructuring" { + // Test destructuring single-field records + const expected_fields = [_]ExpectedField{ + .{ .name = "total", .value = 10 }, + }; + try runExpectRecord( + "List.fold([{val: 1}, {val: 2}, {val: 3}, {val: 4}], {total: 0}, |acc, {val}| {total: acc.total + val})", + &expected_fields, + .no_trace, + ); +} + +// List destructuring tests in lambda params - these previously leaked memory +// Fixed by adding decref after successful patternMatchesBind in for_loop_iterate + +test "List.fold with list destructuring - simple first element" { + // Simplest case: just extract the first element + try runExpectInt( + "List.fold([[10], [20], [30]], 0, |acc, [x]| acc + x)", + 60, + .no_trace, + ); +} + +test "List.fold with list destructuring - two element exact match" { + // Extract exactly two elements + try runExpectInt( + "List.fold([[1, 2], [3, 4]], 0, |acc, [a, b]| acc + a + b)", + 10, + .no_trace, + ); +} + +// Test that list destructuring works in match (not in lambda params) - this should work +test "match with list destructuring - baseline" { + // This tests list destructuring in a match context, not lambda params + try runExpectInt( + "match [1, 2, 3] { [a, b, c] => a + b + c, _ => 0 }", + 6, + .no_trace, + ); +} + +// List destructuring tests with record accumulators + +test "List.fold with record accumulator - list destructuring in lambda" { + // Test folding over a list of lists, destructuring each inner list + // [1, 2], [3, 4], [5, 6] -> first elements are 1, 3, 5 -> sum is 9 + const expected_fields = [_]ExpectedField{ + .{ .name = "first_sum", .value = 9 }, + .{ .name = "count", .value = 3 }, + }; + try runExpectRecord( + "List.fold([[1, 2], [3, 4], [5, 6]], {first_sum: 0, count: 0}, |acc, [first, ..]| {first_sum: acc.first_sum + first, count: acc.count + 1})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - destructure two elements" { + // Test destructuring first two elements from each inner list + const expected_fields = [_]ExpectedField{ + .{ .name = "sum_firsts", .value = 9 }, + .{ .name = "sum_seconds", .value = 12 }, + }; + try runExpectRecord( + "List.fold([[1, 2, 100], [3, 4, 200], [5, 6, 300]], {sum_firsts: 0, sum_seconds: 0}, |acc, [a, b, ..]| {sum_firsts: acc.sum_firsts + a, sum_seconds: acc.sum_seconds + b})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - exact list pattern" { + // Test exact list pattern matching (no rest pattern) + const expected_fields = [_]ExpectedField{ + .{ .name = "total", .value = 21 }, + }; + try runExpectRecord( + "List.fold([[1, 2], [3, 4], [5, 6]], {total: 0}, |acc, [a, b]| {total: acc.total + a + b})", + &expected_fields, + .no_trace, + ); +} + +test "List.fold with record accumulator - nested list and record" { + // Test combining list destructuring with record accumulator updates + // Using ".. as tail" syntax for the rest pattern + const expected_fields = [_]ExpectedField{ + .{ .name = "head_sum", .value = 6 }, + .{ .name = "tail_count", .value = 6 }, + }; + try runExpectRecord( + "List.fold([[1, 10, 20], [2, 30, 40], [3, 50, 60]], {head_sum: 0, tail_count: 0}, |acc, [head, .. as tail]| {head_sum: acc.head_sum + head, tail_count: acc.tail_count + List.len(tail)})", + &expected_fields, + .no_trace, + ); +} diff --git a/src/repl/eval.zig b/src/repl/eval.zig index cd7d63493c..db2cb42c3b 100644 --- a/src/repl/eval.zig +++ b/src/repl/eval.zig @@ -680,10 +680,6 @@ pub const Repl = struct { }; result.decref(&interpreter.runtime_layout_store, self.roc_ops); - if (result.layout.tag == .record) { - self.allocator.free(output); - return try self.allocator.dupe(u8, ""); - } return output; } @@ -864,10 +860,6 @@ pub const Repl = struct { }; result.decref(&interpreter.runtime_layout_store, self.roc_ops); - if (result.layout.tag == .record) { - self.allocator.free(output); - return .{ .expression = try self.allocator.dupe(u8, "") }; - } return .{ .expression = output }; } }; diff --git a/test/snapshots/repl/deeply_nested_polymorphic_functions.md b/test/snapshots/repl/deeply_nested_polymorphic_functions.md index 808f7cd078..7951552b16 100644 --- a/test/snapshots/repl/deeply_nested_polymorphic_functions.md +++ b/test/snapshots/repl/deeply_nested_polymorphic_functions.md @@ -8,6 +8,6 @@ type=repl » (|twice, identity| { a: twice(identity, 42), b: twice(|x| x + 1, 100) })(|f, val| f(f(val)), |x| x) ~~~ # OUTPUT - +{ a: 42, b: 102 } # PROBLEMS NIL diff --git a/test/snapshots/repl/list_fold_record_accumulator.md b/test/snapshots/repl/list_fold_record_accumulator.md new file mode 100644 index 0000000000..77f1e9123c --- /dev/null +++ b/test/snapshots/repl/list_fold_record_accumulator.md @@ -0,0 +1,13 @@ +# META +~~~ini +description=List.fold with record accumulator - tests record state in fold +type=repl +~~~ +# SOURCE +~~~roc +» List.fold([1, 2, 3], {sum: 0, count: 0}, |acc, item| {sum: acc.sum + item, count: acc.count + 1}) +~~~ +# OUTPUT +{ count: 3, sum: 6 } +# PROBLEMS +NIL diff --git a/test/snapshots/repl/nested_polymorphic_functions.md b/test/snapshots/repl/nested_polymorphic_functions.md index 86f1a0d1f4..a05b44695c 100644 --- a/test/snapshots/repl/nested_polymorphic_functions.md +++ b/test/snapshots/repl/nested_polymorphic_functions.md @@ -8,6 +8,6 @@ type=repl » (|identity| { a: identity(10), b: identity(20), c: identity(30) })(|x| x) ~~~ # OUTPUT - +{ a: 10, b: 20, c: 30 } # PROBLEMS NIL diff --git a/test/snapshots/repl/str_from_utf8.md b/test/snapshots/repl/str_from_utf8.md index 13492b4d8b..56a26c505a 100644 --- a/test/snapshots/repl/str_from_utf8.md +++ b/test/snapshots/repl/str_from_utf8.md @@ -35,6 +35,6 @@ True --- "fallback" --- -Err(BadUtf8({ index: 0, problem: 3 })) +Err(BadUtf8({ index: 0, problem: InvalidStartByte })) # PROBLEMS NIL