diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index 586a2b0fc4..aec42ff6c8 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -14158,14 +14158,57 @@ pub const Interpreter = struct { if (lambda_expr == .e_low_level_lambda) { const low_level = lambda_expr.e_low_level_lambda; - // Get the actual return type from the lambda's function type. - // This is needed because ci.call_ret_rt_var may be a polymorphic type - // that hasn't been unified with the actual return type (e.g., when - // passing a builtin like U64.from_str directly to a higher-order function). - const low_level_ct_var = can.ModuleEnv.varFrom(header.lambda_expr_idx); - const low_level_rt_var = try self.translateTypeVar(self.env, low_level_ct_var); - const resolved_func = self.runtime_types.resolveVar(low_level_rt_var); - const ret_rt_var = if (resolved_func.desc.content.unwrapFunc()) |func| func.ret else ci.call_ret_rt_var; + // Determine the return type for this low-level builtin call. + // + // There are two cases to consider: + // 1. Direct call with unified types (e.g., List.append(List.with_capacity(1), 1i64)) + // - ci.call_ret_rt_var has the correct unified type (List(I64)) + // - The lambda's function type has type parameters (List(item)) + // - We should use ci.call_ret_rt_var + // + // 2. Passing builtin to higher-order function (e.g., List.map(strs, U64.from_str)) + // - ci.call_ret_rt_var may be polymorphic (not properly unified) + // - The lambda's function type has the concrete return type + // - We should use func.ret + // + // Strategy: Check if ci.call_ret_rt_var contains unresolved type parameters. + // If it's concrete, use it. Otherwise, fall back to the lambda's return type. + const ret_rt_var = blk: { + const call_ret_resolved = self.runtime_types.resolveVar(ci.call_ret_rt_var); + // Check if the call return type is concrete (no unresolved flex/rigid parameters) + const is_concrete = switch (call_ret_resolved.desc.content) { + .structure => |st| switch (st) { + .nominal_type => |nom| is_concrete: { + // Check if any type args are unresolved flex/rigid + const type_args = self.runtime_types.sliceNominalArgs(nom); + for (type_args) |arg| { + const arg_resolved = self.runtime_types.resolveVar(arg); + switch (arg_resolved.desc.content) { + .flex => |flex| if (flex.constraints.count == 0) break :is_concrete false, + .rigid => |rigid| if (rigid.constraints.count == 0) break :is_concrete false, + else => {}, + } + } + break :is_concrete true; + }, + else => true, + }, + .flex => |flex| flex.constraints.count > 0, + .rigid => |rigid| rigid.constraints.count > 0, + else => true, + }; + + if (is_concrete) { + // Use the call site's return type - it has concrete type info + break :blk ci.call_ret_rt_var; + } else { + // Fall back to the lambda's function return type + const low_level_ct_var = can.ModuleEnv.varFrom(header.lambda_expr_idx); + const low_level_rt_var = try self.translateTypeVar(self.env, low_level_ct_var); + const resolved_func = self.runtime_types.resolveVar(low_level_rt_var); + break :blk if (resolved_func.desc.content.unwrapFunc()) |func| func.ret else ci.call_ret_rt_var; + } + }; // Special handling for list_sort_with which requires continuation-based evaluation if (low_level.op == .list_sort_with) { diff --git a/src/eval/test/eval_test.zig b/src/eval/test/eval_test.zig index 15a92811ac..dd6947463a 100644 --- a/src/eval/test/eval_test.zig +++ b/src/eval/test/eval_test.zig @@ -29,6 +29,7 @@ const runExpectError = helpers.runExpectError; const runExpectStr = helpers.runExpectStr; const runExpectRecord = helpers.runExpectRecord; const runExpectListI64 = helpers.runExpectListI64; +const runExpectListI64WithStrictLayout = helpers.runExpectListI64WithStrictLayout; const ExpectedField = helpers.ExpectedField; const TraceWriterState = struct { @@ -1478,16 +1479,12 @@ test "method calls on numeric variables with flex types - regression" { , "42.0", .no_trace); } -test "issue 8667: List.with_capacity in fold causes use-after-free" { - // First, test that List.append works with empty list - try runExpectListI64("List.append([], 1i64)", &[_]i64{1}, .no_trace); +test "issue 8667: List.with_capacity should be inferred as List(I64)" { + // When List.with_capacity is used with List.append(_, 1i64), the type checker should + // unify the list element type to I64. This means the layout should be .list (not .list_of_zst). + // If it's .list_of_zst, that indicates a type inference bug. + try runExpectListI64WithStrictLayout("List.append(List.with_capacity(1), 1i64)", &[_]i64{1}, .no_trace); - // Now test with List.with_capacity - try runExpectListI64("List.append(List.with_capacity(1), 1i64)", &[_]i64{1}, .no_trace); - - // Test fold with empty list - should work - try runExpectListI64("[1i64].fold([], List.append)", &[_]i64{1}, .no_trace); - - // Test fold with with_capacity - this is the bug from issue #8667 - try runExpectListI64("[1i64].fold(List.with_capacity(1), List.append)", &[_]i64{1}, .no_trace); + // Also test the fold case which is where the bug was originally reported + try runExpectListI64WithStrictLayout("[1i64].fold(List.with_capacity(1), List.append)", &[_]i64{1}, .no_trace); } diff --git a/src/eval/test/helpers.zig b/src/eval/test/helpers.zig index d006fc2ed1..9407cbf01e 100644 --- a/src/eval/test/helpers.zig +++ b/src/eval/test/helpers.zig @@ -475,6 +475,62 @@ pub fn runExpectListI64(src: []const u8, expected_elements: []const i64, should_ } } +/// Like runExpectListI64 but asserts the layout is .list (not .list_of_zst). +/// This is used to verify that type unification is working correctly - +/// when a list is used with a non-ZST element type, its layout should be .list. +pub fn runExpectListI64WithStrictLayout(src: []const u8, expected_elements: []const i64, should_trace: enum { trace, no_trace }) !void { + const resources = try parseAndCanonicalizeExpr(test_allocator, src); + defer cleanupParseAndCanonical(test_allocator, resources); + + var test_env_instance = TestEnv.init(test_allocator); + defer test_env_instance.deinit(); + + const builtin_types = BuiltinTypes.init(resources.builtin_indices, resources.builtin_module.env, resources.builtin_module.env, resources.builtin_module.env); + const imported_envs = [_]*const can.ModuleEnv{resources.builtin_module.env}; + var interpreter = try Interpreter.init(test_allocator, resources.module_env, builtin_types, resources.builtin_module.env, &imported_envs, &resources.checker.import_mapping, null); + defer interpreter.deinit(); + + const enable_trace = should_trace == .trace; + if (enable_trace) { + interpreter.startTrace(); + } + defer if (enable_trace) interpreter.endTrace(); + + const ops = test_env_instance.get_ops(); + const result = try interpreter.eval(resources.expr_idx, ops); + const layout_cache = &interpreter.runtime_layout_store; + defer result.decref(layout_cache, ops); + defer interpreter.cleanupBindings(ops); + + // STRICT: Verify we got a .list layout (NOT .list_of_zst) + // If this fails, it means type unification didn't properly infer the element type + if (result.layout.tag != .list) { + std.debug.print("\n[FAIL] Expected .list layout but got .{s}\n", .{@tagName(result.layout.tag)}); + std.debug.print("This indicates a type inference bug - List.with_capacity should be unified to List(I64)\n", .{}); + return error.TestExpectedEqual; + } + + // Get the element layout + const elem_layout_idx = result.layout.data.list; + const elem_layout = layout_cache.getLayout(elem_layout_idx); + + // Use the ListAccessor to safely access list elements + const list_accessor = try result.asList(layout_cache, elem_layout); + + try std.testing.expectEqual(expected_elements.len, list_accessor.len()); + + for (expected_elements, 0..) |expected_val, i| { + // Use the result's rt_var since we're accessing elements of the evaluated expression + const element = try list_accessor.getElement(i, result.rt_var); + + // Check if this is an integer + try std.testing.expect(element.layout.tag == .scalar); + try std.testing.expect(element.layout.data.scalar.tag == .int); + const int_val = element.asI128(); + try std.testing.expectEqual(@as(i128, expected_val), int_val); + } +} + /// Parse and canonicalize an expression. /// Rewrite deferred numeric literals to match their inferred types /// This is similar to what ComptimeEvaluator does but for test expressions