From 38acce6f73d60205c198f12e3926a2e2bac5f9be Mon Sep 17 00:00:00 2001 From: Matthieu Pizenberg Date: Thu, 4 Dec 2025 18:39:02 +0100 Subject: [PATCH 1/3] Fix closure capture for if expressions Signed-off-by: Matthieu Pizenberg --- src/canonicalize/Can.zig | 73 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/src/canonicalize/Can.zig b/src/canonicalize/Can.zig index 0eb87727b9..02e640d152 100644 --- a/src/canonicalize/Can.zig +++ b/src/canonicalize/Can.zig @@ -5321,6 +5321,11 @@ pub fn canonicalizeExpr( .if_then_else => |e| { const region = self.parse_ir.tokenizedRegionToRegion(e.region); + // Use scratch_captures as intermediate buffer for collecting free vars + // This avoids capturing intermediate data from nested block canonicalization + const captures_top = self.scratch_captures.top(); + defer self.scratch_captures.clearFrom(captures_top); + const free_vars_start = self.scratch_free_vars.top(); // Start collecting if-branches @@ -5341,6 +5346,14 @@ pub fn canonicalizeExpr( return CanonicalizedExpr{ .idx = malformed_idx, .free_vars = DataSpan.empty() }; }; + // Collect free variables from the condition into scratch_captures + const cond_free_vars_slice = self.scratch_free_vars.sliceFromSpan(can_cond.free_vars); + for (cond_free_vars_slice) |fv| { + if (!self.scratch_captures.contains(fv)) { + try self.scratch_captures.append(fv); + } + } + const can_then = try self.canonicalizeExpr(current_if.then) orelse { const ast_then = self.parse_ir.store.getExpr(current_if.then); const then_region = self.parse_ir.tokenizedRegionToRegion(ast_then.to_tokenized_region()); @@ -5350,6 +5363,14 @@ pub fn canonicalizeExpr( return CanonicalizedExpr{ .idx = malformed_idx, .free_vars = DataSpan.empty() }; }; + // Collect free variables from the then-branch into scratch_captures + const then_free_vars_slice = self.scratch_free_vars.sliceFromSpan(can_then.free_vars); + for (then_free_vars_slice) |fv| { + if (!self.scratch_captures.contains(fv)) { + try self.scratch_captures.append(fv); + } + } + // Add this condition/then pair as an if-branch const if_branch = Expr.IfBranch{ .cond = can_cond.idx, @@ -5371,6 +5392,15 @@ pub fn canonicalizeExpr( }); return CanonicalizedExpr{ .idx = malformed_idx, .free_vars = DataSpan.empty() }; }; + + // Collect free variables from the else-branch into scratch_captures + const else_free_vars_slice = self.scratch_free_vars.sliceFromSpan(can_else.free_vars); + for (else_free_vars_slice) |fv| { + if (!self.scratch_captures.contains(fv)) { + try self.scratch_captures.append(fv); + } + } + final_else = can_else.idx; break; } @@ -5390,7 +5420,16 @@ pub fn canonicalizeExpr( }, }, region); - const free_vars_span = self.scratch_free_vars.spanFrom(free_vars_start); + // Clear intermediate data from scratch_free_vars + self.scratch_free_vars.clearFrom(free_vars_start); + + // Copy collected free vars from scratch_captures to scratch_free_vars + const if_free_vars_start = self.scratch_free_vars.top(); + const captures_slice = self.scratch_captures.sliceFromStart(captures_top); + for (captures_slice) |fv| { + try self.scratch_free_vars.append(fv); + } + const free_vars_span = self.scratch_free_vars.spanFrom(if_free_vars_start); return CanonicalizedExpr{ .idx = expr_idx, .free_vars = free_vars_span }; }, .if_without_else => |e| { @@ -5409,6 +5448,11 @@ pub fn canonicalizeExpr( // Desugar to if-then-else with empty record {} as the final else // Type checking will ensure the then-branch also has type {} + // Use scratch_captures as intermediate buffer for collecting free vars + // This avoids capturing intermediate data from nested block canonicalization + const captures_top = self.scratch_captures.top(); + defer self.scratch_captures.clearFrom(captures_top); + const free_vars_start = self.scratch_free_vars.top(); // Canonicalize condition @@ -5421,6 +5465,14 @@ pub fn canonicalizeExpr( return CanonicalizedExpr{ .idx = malformed_idx, .free_vars = DataSpan.empty() }; }; + // Collect free variables from the condition into scratch_captures + const cond_free_vars_slice = self.scratch_free_vars.sliceFromSpan(can_cond.free_vars); + for (cond_free_vars_slice) |fv| { + if (!self.scratch_captures.contains(fv)) { + try self.scratch_captures.append(fv); + } + } + // Canonicalize then branch const can_then = try self.canonicalizeExpr(e.then) orelse { const ast_then = self.parse_ir.store.getExpr(e.then); @@ -5431,6 +5483,14 @@ pub fn canonicalizeExpr( return CanonicalizedExpr{ .idx = malformed_idx, .free_vars = DataSpan.empty() }; }; + // Collect free variables from the then-branch into scratch_captures + const then_free_vars_slice = self.scratch_free_vars.sliceFromSpan(can_then.free_vars); + for (then_free_vars_slice) |fv| { + if (!self.scratch_captures.contains(fv)) { + try self.scratch_captures.append(fv); + } + } + // Create an empty record {} as the implicit else const empty_record_idx = try self.env.addExpr(CIR.Expr{ .e_empty_record = .{} }, region); @@ -5452,7 +5512,16 @@ pub fn canonicalizeExpr( }, }, region); - const free_vars_span = self.scratch_free_vars.spanFrom(free_vars_start); + // Clear intermediate data from scratch_free_vars + self.scratch_free_vars.clearFrom(free_vars_start); + + // Copy collected free vars from scratch_captures to scratch_free_vars + const if_free_vars_start = self.scratch_free_vars.top(); + const captures_slice = self.scratch_captures.sliceFromStart(captures_top); + for (captures_slice) |fv| { + try self.scratch_free_vars.append(fv); + } + const free_vars_span = self.scratch_free_vars.spanFrom(if_free_vars_start); return CanonicalizedExpr{ .idx = expr_idx, .free_vars = free_vars_span }; }, .match => |m| { From 6611e2f2f8d71a2c10782800f3a5f91afe2ed337 Mon Sep 17 00:00:00 2001 From: Matthieu Pizenberg Date: Thu, 4 Dec 2025 18:41:48 +0100 Subject: [PATCH 2/3] Add test/fx file Signed-off-by: Matthieu Pizenberg --- test/fx/if-closure-capture.roc | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 test/fx/if-closure-capture.roc diff --git a/test/fx/if-closure-capture.roc b/test/fx/if-closure-capture.roc new file mode 100644 index 0000000000..218d598037 --- /dev/null +++ b/test/fx/if-closure-capture.roc @@ -0,0 +1,8 @@ +app [main!] { pf: platform "./platform/main.roc" } + +main! = || { + if True { + x = 0 + _y = x + } +} From 27efb60b26acdde53fc97e3414f5ee3835fad89d Mon Sep 17 00:00:00 2001 From: Matthieu Pizenberg Date: Thu, 4 Dec 2025 18:55:50 +0100 Subject: [PATCH 3/3] Reference the new platform test Signed-off-by: Matthieu Pizenberg --- src/cli/test/fx_platform_test.zig | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/cli/test/fx_platform_test.zig b/src/cli/test/fx_platform_test.zig index bc4cf8274b..980cf7ee4d 100644 --- a/src/cli/test/fx_platform_test.zig +++ b/src/cli/test/fx_platform_test.zig @@ -1538,3 +1538,16 @@ test "fx platform method inspect on string" { // Should show MISSING METHOD error try testing.expect(std.mem.indexOf(u8, run_result.stderr, "MISSING METHOD") != null); } + +test "fx platform if-expression closure capture regression" { + // Regression test: Variables bound inside an if-expression's block were + // incorrectly being captured as free variables by the enclosing lambda, + // causing a crash with "e_closure: failed to resolve capture value". + const allocator = testing.allocator; + + const run_result = try runRoc(allocator, "test/fx/if-closure-capture.roc", .{}); + defer allocator.free(run_result.stdout); + defer allocator.free(run_result.stderr); + + try checkSuccess(run_result); +}