diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index 6aa69ced46..74b4364672 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -2229,25 +2229,43 @@ pub const Interpreter = struct { // Determine if elements are refcounted const elements_refcounted = elem_layout.isRefcounted(); - // TODO: Proper refcounting for list elements - // For now, we use no-op functions even for refcounted elements. - // This works correctly because listConcat will handle the list-level refcounting, - // but element-level refcounting may need manual handling in complex cases. - const inc_fn = builtins.list.rcNone; - const dec_fn = builtins.list.rcNone; - - // Call listConcat - it consumes both input lists + // Call listConcat with NO element refcounting. + // We use the post-processing approach: tell listConcat that elements + // aren't refcounted (avoiding the need for C-callable function pointers + // with context), then manually fix element refcounts afterward using + // StackValue.incref() which already handles all layout types. + // This has the same performance characteristics but avoids threadlocals. const result_list = builtins.list.listConcat( list_a.*, list_b.*, elem_alignment_u32, elem_size, - elements_refcounted, - inc_fn, - dec_fn, + false, // Tell listConcat elements aren't refcounted + &builtins.list.rcNone, // No-op inc + &builtins.list.rcNone, // No-op dec roc_ops, ); + // Post-process: Fix element refcounts if needed. + // listConcat has already copied all elements via memcpy, so we need to + // increment their refcounts since they're now shared between the result + // and the original source data (before the input lists are decremented). + if (elements_refcounted) { + if (result_list.bytes) |buffer| { + var i: usize = 0; + while (i < result_list.len()) : (i += 1) { + const elem_ptr = buffer + i * elem_size; + const elem_value = StackValue{ + .layout = elem_layout, + .ptr = @ptrCast(elem_ptr), + .is_initialized = true, + }; + // This handles all nested refcounting (strings, lists, etc.) + elem_value.incref(); + } + } + } + // Allocate space for the result list const result_layout = list_a_arg.layout; // Same layout as input var out = try self.pushRaw(result_layout, 0); diff --git a/src/eval/test/low_level_interp_test.zig b/src/eval/test/low_level_interp_test.zig index 94102f4ab1..3d8a06b3da 100644 --- a/src/eval/test/low_level_interp_test.zig +++ b/src/eval/test/low_level_interp_test.zig @@ -273,3 +273,75 @@ test "e_low_level_lambda - List.concat preserves order" { const tag_name = result.module_env.getIdent(first_expr.e_tag.ident); try testing.expectEqualStrings("Ok", tag_name); } + +test "e_low_level_lambda - List.concat with strings (refcounted elements)" { + const src = + \\x = List.concat(["hello", "world"], ["foo", "bar"]) + \\len = List.len(x) + ; + + var result = try parseCheckAndEvalModule(src); + defer cleanupEvalModule(&result); + + const summary = try result.evaluator.evalAll(); + + // Should evaluate 2 declarations with 0 crashes + try testing.expectEqual(@as(u32, 2), summary.evaluated); + try testing.expectEqual(@as(u32, 0), summary.crashed); + + // Verify the length is 4 + const defs = result.module_env.store.sliceDefs(result.module_env.all_defs); + const len_def = result.module_env.store.getDef(defs[1]); + const len_expr = result.module_env.store.getExpr(len_def.expr); + + try testing.expect(len_expr == .e_num); + try testing.expectEqual(@as(u64, 4), len_expr.e_num.value.int); +} + +test "e_low_level_lambda - List.concat with nested lists (refcounted elements)" { + const src = + \\x = List.concat([[1, 2], [3]], [[4, 5, 6]]) + \\len = List.len(x) + ; + + var result = try parseCheckAndEvalModule(src); + defer cleanupEvalModule(&result); + + const summary = try result.evaluator.evalAll(); + + // Should evaluate 2 declarations with 0 crashes + try testing.expectEqual(@as(u32, 2), summary.evaluated); + try testing.expectEqual(@as(u32, 0), summary.crashed); + + // Verify the length is 3 (outer list has 3 elements) + const defs = result.module_env.store.sliceDefs(result.module_env.all_defs); + const len_def = result.module_env.store.getDef(defs[1]); + const len_expr = result.module_env.store.getExpr(len_def.expr); + + try testing.expect(len_expr == .e_num); + try testing.expectEqual(@as(u64, 3), len_expr.e_num.value.int); +} + +test "e_low_level_lambda - List.concat with empty string list" { + const src = + \\x = List.concat([], ["a", "b", "c"]) + \\len = List.len(x) + ; + + var result = try parseCheckAndEvalModule(src); + defer cleanupEvalModule(&result); + + const summary = try result.evaluator.evalAll(); + + // Should evaluate 2 declarations with 0 crashes + try testing.expectEqual(@as(u32, 2), summary.evaluated); + try testing.expectEqual(@as(u32, 0), summary.crashed); + + // Verify the length is 3 + const defs = result.module_env.store.sliceDefs(result.module_env.all_defs); + const len_def = result.module_env.store.getDef(defs[1]); + const len_expr = result.module_env.store.getExpr(len_def.expr); + + try testing.expect(len_expr == .e_num); + try testing.expectEqual(@as(u64, 3), len_expr.e_num.value.int); +}