language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-12-23 08:48:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Merge pull request #8679 from roc-lang/fix-issue-8667

Browse source 
Fix type inference bug in low-level lambda calls (#8667)
This commit is contained in:
Richard Feldman 2025-12-14 16:01:26 -05:00 • committed by GitHub
commit 3bc2fdd2cc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 115 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

59
src/eval/interpreter.zig
View file

@ -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) {

19
src/eval/test/eval_test.zig
View file

@ -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);
}

56
src/eval/test/helpers.zig
View file

@ -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