Skip to content

Commit 036b069

Browse files
rtfeldmanrtfeldman
authored
Merge pull request #8434 from roc-lang/fix-roc-run2
Fix generalization bug
2 parents 9e509c5 + 76cd242 commit 036b069

22 files changed

+428
-374
lines changed

‎build.zig‎

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,24 +93,59 @@ const MiniCiStep = struct {
9393
fn make(step: *Step, options: Step.MakeOptions) !void {
9494
_ = options;
9595

96-
const b = step.owner;
97-
9896
// Run the sequence of `zig build` commands that make up the
9997
// mini CI pipeline.
100-
try runSubBuild(b, "fmt", "zig build fmt");
101-
try runSubBuild(b, null, "zig build");
102-
try runSubBuild(b, "snapshot", "zig build snapshot");
103-
try runSubBuild(b, "test", "zig build test");
104-
try runSubBuild(b, "test-playground", "zig build test-playground");
105-
try runSubBuild(b, "test-serialization-sizes", "zig build test-serialization-sizes");
106-
try runSubBuild(b, "test-cli", "zig build test-cli");
98+
try runSubBuild(step, "fmt", "zig build fmt");
99+
try runSubBuild(step, null, "zig build");
100+
try runSubBuild(step, "snapshot", "zig build snapshot");
101+
try checkSnapshotChanges(step);
102+
try runSubBuild(step, "test", "zig build test");
103+
try runSubBuild(step, "test-playground", "zig build test-playground");
104+
try runSubBuild(step, "test-serialization-sizes", "zig build test-serialization-sizes");
105+
try runSubBuild(step, "test-cli", "zig build test-cli");
106+
}
107+
108+
fn checkSnapshotChanges(step: *Step) !void {
109+
const b = step.owner;
110+
std.debug.print("---- minici: checking for snapshot changes ----\n", .{});
111+
112+
var child_argv = std.ArrayList([]const u8).empty;
113+
defer child_argv.deinit(b.allocator);
114+
115+
try child_argv.append(b.allocator, "git");
116+
try child_argv.append(b.allocator, "diff");
117+
try child_argv.append(b.allocator, "--exit-code");
118+
try child_argv.append(b.allocator, "test/snapshots");
119+
120+
var child = std.process.Child.init(child_argv.items, b.allocator);
121+
child.stdin_behavior = .Inherit;
122+
child.stdout_behavior = .Inherit;
123+
child.stderr_behavior = .Inherit;
124+
125+
const term = try child.spawnAndWait();
126+
127+
switch (term) {
128+
.Exited => |code| {
129+
if (code != 0) {
130+
return step.fail(
131+
"Snapshots in 'test/snapshots' have changed. " ++
132+
"Run 'zig build snapshot' locally, review the updates, and commit the changes.",
133+
.{},
134+
);
135+
}
136+
},
137+
else => {
138+
return step.fail("git diff terminated abnormally", .{});
139+
},
140+
}
107141
}
108142

109143
fn runSubBuild(
110-
b: *std.Build,
144+
step: *Step,
111145
step_name: ?[]const u8,
112146
display: []const u8,
113147
) !void {
148+
const b = step.owner;
114149
std.debug.print("---- minici: running `{s}` ----\n", .{display});
115150

116151
var child_argv = std.ArrayList([]const u8).empty;
@@ -134,16 +169,11 @@ const MiniCiStep = struct {
134169
switch (term) {
135170
.Exited => |code| {
136171
if (code != 0) {
137-
std.debug.print(
138-
"minici: `{s}` failed with exit code {d}\n",
139-
.{ display, code },
140-
);
141-
return error.MakeFailed;
172+
return step.fail("`{s}` failed with exit code {d}", .{ display, code });
142173
}
143174
},
144175
else => {
145-
std.debug.print("minici: `{s}` terminated abnormally\n", .{display});
146-
return error.MakeFailed;
176+
return step.fail("`{s}` terminated abnormally", .{display});
147177
},
148178
}
149179
}

‎src/canonicalize/Can.zig‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7803,6 +7803,7 @@ fn canonicalizeBlock(self: *Self, e: AST.Block) std.mem.Allocator.Error!Canonica
78037803
const cir_stmt = self.env.store.getStatement(canonicailzed_stmt.idx);
78047804
switch (cir_stmt) {
78057805
.s_decl => |decl| try self.collectBoundVars(decl.pattern, &bound_vars),
7806+
.s_decl_gen => |decl| try self.collectBoundVars(decl.pattern, &bound_vars),
78067807
.s_var => |var_stmt| try self.collectBoundVars(var_stmt.pattern_idx, &bound_vars),
78077808
else => {},
78087809
}

‎src/cli/test/fx_platform_test.zig‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,38 @@ test "fx platform expect with numeric literal" {
264264
try testing.expectEqualStrings("", run_result.stdout);
265265
try testing.expectEqualStrings("", run_result.stderr);
266266
}
267+
268+
test "fx platform match with wildcard" {
269+
const allocator = testing.allocator;
270+
271+
try ensureRocBinary(allocator);
272+
273+
// Run an app that uses a match expression with a wildcard pattern
274+
// This tests that wildcard patterns in match expressions work correctly
275+
const run_result = try std.process.Child.run(.{
276+
.allocator = allocator,
277+
.argv = &[_][]const u8{
278+
"./zig-out/bin/roc",
279+
"test/fx/match_with_wildcard.roc",
280+
},
281+
});
282+
defer allocator.free(run_result.stdout);
283+
defer allocator.free(run_result.stderr);
284+
285+
switch (run_result.term) {
286+
.Exited => |code| {
287+
if (code != 0) {
288+
std.debug.print("Run failed with exit code {}\n", .{code});
289+
std.debug.print("STDOUT: {s}\n", .{run_result.stdout});
290+
std.debug.print("STDERR: {s}\n", .{run_result.stderr});
291+
return error.RunFailed;
292+
}
293+
},
294+
else => {
295+
std.debug.print("Run terminated abnormally: {}\n", .{run_result.term});
296+
std.debug.print("STDOUT: {s}\n", .{run_result.stdout});
297+
std.debug.print("STDERR: {s}\n", .{run_result.stderr});
298+
return error.RunFailed;
299+
},
300+
}
301+
}

‎src/eval/interpreter.zig‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,16 @@ pub const Interpreter = struct {
420420
}
421421

422422
const header: *const layout.Closure = @ptrCast(@alignCast(func_val.ptr.?));
423+
424+
// Switch to the closure's source module for correct expression evaluation.
425+
// This is critical because pattern indices and expression indices in the closure
426+
// are relative to the source module where the closure was defined, not the
427+
// current module. Without this switch, bindings created in the closure body
428+
// would have the wrong source_env and lookups would fail.
429+
const saved_env = self.env;
430+
self.env = @constCast(header.source_env);
431+
defer self.env = saved_env;
432+
423433
const params = self.env.store.slicePatterns(header.params);
424434

425435
try self.active_closures.append(func_val);

‎src/interpreter_shim/main.zig‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,13 @@ const ShimError = error{
6161
/// Expected format in shared memory: [u64 parent_address][u32 entry_count][ModuleEnv data][u32[] def_indices]
6262
export fn roc_entrypoint(entry_idx: u32, ops: *builtins.host_abi.RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) callconv(.c) void {
6363
evaluateFromSharedMemory(entry_idx, ops, ret_ptr, arg_ptr) catch |err| {
64-
var buf: [256]u8 = undefined;
65-
const msg2 = std.fmt.bufPrint(&buf, "Error evaluating from shared memory: {s}", .{@errorName(err)}) catch "Error evaluating from shared memory";
66-
ops.crash(msg2);
64+
// Only show this generic error if we haven't already crashed with a more specific message
65+
// (errors like Crash already triggered roc_crashed with details)
66+
if (err != error.Crash) {
67+
var buf: [256]u8 = undefined;
68+
const msg2 = std.fmt.bufPrint(&buf, "Error evaluating from shared memory: {s}", .{@errorName(err)}) catch "Error evaluating from shared memory";
69+
ops.crash(msg2);
70+
}
6771
};
6872
}
6973

‎test/fx/match_with_wildcard.roc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
app [main!] { pf: platform "./platform/main.roc" }
2+
3+
import pf.Stdout
4+
5+
main! = || {
6+
zero : U8
7+
zero = 0
8+
zero_hex = u4_to_hex(zero)
9+
Stdout.line!(zero_hex)
10+
}
11+
12+
u4_to_hex : U8 -> Str
13+
u4_to_hex = |n| {
14+
match n {
15+
_ => "0"
16+
}
17+
}

‎test/snapshots/can_basic_scoping.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ outerFunc = |_| {
120120
(p-assign (ident "outerFunc"))
121121
(e-closure
122122
(captures
123-
(capture (ident "x"))
124123
(capture (ident "y")))
125124
(e-lambda
126125
(args

‎test/snapshots/can_var_scoping_regular_var.md‎

Lines changed: 46 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -129,59 +129,56 @@ NO CHANGE
129129
(can-ir
130130
(d-let
131131
(p-assign (ident "processItems"))
132-
(e-closure
133-
(captures
134-
(capture (ident "nestedFunc")))
135-
(e-lambda
136-
(args
137-
(p-assign (ident "items")))
138-
(e-block
139-
(s-var
140-
(p-assign (ident "count_"))
141-
(e-num (value "0")))
142-
(s-var
143-
(p-assign (ident "total_"))
144-
(e-num (value "0")))
145-
(s-reassign
146-
(p-assign (ident "count_"))
147-
(e-binop (op "add")
148-
(e-lookup-local
149-
(p-assign (ident "count_")))
150-
(e-num (value "1"))))
151-
(s-reassign
152-
(p-assign (ident "total_"))
153-
(e-binop (op "add")
154-
(e-lookup-local
155-
(p-assign (ident "total_")))
156-
(e-num (value "10"))))
157-
(s-let
158-
(p-assign (ident "nestedFunc"))
159-
(e-closure
160-
(captures
161-
(capture (ident "count_")))
162-
(e-lambda
163-
(args
164-
(p-underscore))
165-
(e-block
166-
(s-reassign
167-
(p-assign (ident "count_"))
168-
(e-runtime-error (tag "var_across_function_boundary")))
169-
(s-reassign
170-
(p-assign (ident "total_"))
171-
(e-runtime-error (tag "var_across_function_boundary")))
172-
(e-lookup-local
173-
(p-assign (ident "count_")))))))
174-
(s-let
175-
(p-assign (ident "result"))
176-
(e-call
177-
(e-lookup-local
178-
(p-assign (ident "nestedFunc")))
179-
(e-empty_record)))
132+
(e-lambda
133+
(args
134+
(p-assign (ident "items")))
135+
(e-block
136+
(s-var
137+
(p-assign (ident "count_"))
138+
(e-num (value "0")))
139+
(s-var
140+
(p-assign (ident "total_"))
141+
(e-num (value "0")))
142+
(s-reassign
143+
(p-assign (ident "count_"))
144+
(e-binop (op "add")
145+
(e-lookup-local
146+
(p-assign (ident "count_")))
147+
(e-num (value "1"))))
148+
(s-reassign
149+
(p-assign (ident "total_"))
180150
(e-binop (op "add")
181151
(e-lookup-local
182152
(p-assign (ident "total_")))
153+
(e-num (value "10"))))
154+
(s-let
155+
(p-assign (ident "nestedFunc"))
156+
(e-closure
157+
(captures
158+
(capture (ident "count_")))
159+
(e-lambda
160+
(args
161+
(p-underscore))
162+
(e-block
163+
(s-reassign
164+
(p-assign (ident "count_"))
165+
(e-runtime-error (tag "var_across_function_boundary")))
166+
(s-reassign
167+
(p-assign (ident "total_"))
168+
(e-runtime-error (tag "var_across_function_boundary")))
169+
(e-lookup-local
170+
(p-assign (ident "count_")))))))
171+
(s-let
172+
(p-assign (ident "result"))
173+
(e-call
183174
(e-lookup-local
184-
(p-assign (ident "result")))))))))
175+
(p-assign (ident "nestedFunc")))
176+
(e-empty_record)))
177+
(e-binop (op "add")
178+
(e-lookup-local
179+
(p-assign (ident "total_")))
180+
(e-lookup-local
181+
(p-assign (ident "result"))))))))
185182
~~~
186183
# TYPES
187184
~~~clojure

‎test/snapshots/comprehensive/Container.md‎

Lines changed: 41 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -947,51 +947,48 @@ main = {
947947
(ty-rigid-var-lookup (ty-rigid-var (name "c"))))))))))
948948
(d-let
949949
(p-assign (ident "process_with_method"))
950-
(e-closure
951-
(captures
952-
(capture (ident "id")))
953-
(e-lambda
954-
(args
955-
(p-assign (ident "container"))
956-
(p-assign (ident "value")))
957-
(e-block
958-
(s-let
959-
(p-assign (ident "id"))
960-
(e-lambda
961-
(args
962-
(p-assign (ident "x")))
963-
(e-lookup-local
964-
(p-assign (ident "x")))))
965-
(s-let
966-
(p-assign (ident "_test1"))
967-
(e-call
968-
(e-lookup-local
969-
(p-assign (ident "id")))
970-
(e-num (value "42"))))
971-
(s-let
972-
(p-assign (ident "_test2"))
973-
(e-call
950+
(e-lambda
951+
(args
952+
(p-assign (ident "container"))
953+
(p-assign (ident "value")))
954+
(e-block
955+
(s-let
956+
(p-assign (ident "id"))
957+
(e-lambda
958+
(args
959+
(p-assign (ident "x")))
960+
(e-lookup-local
961+
(p-assign (ident "x")))))
962+
(s-let
963+
(p-assign (ident "_test1"))
964+
(e-call
965+
(e-lookup-local
966+
(p-assign (ident "id")))
967+
(e-num (value "42"))))
968+
(s-let
969+
(p-assign (ident "_test2"))
970+
(e-call
971+
(e-lookup-local
972+
(p-assign (ident "id")))
973+
(e-string
974+
(e-literal (string "test")))))
975+
(s-let
976+
(p-assign (ident "result"))
977+
(e-dot-access (field "map")
978+
(receiver
974979
(e-lookup-local
975-
(p-assign (ident "id")))
976-
(e-string
977-
(e-literal (string "test")))))
978-
(s-let
979-
(p-assign (ident "result"))
980-
(e-dot-access (field "map")
981-
(receiver
982-
(e-lookup-local
983-
(p-assign (ident "container"))))
984-
(args
985-
(e-closure
986-
(captures
987-
(capture (ident "value")))
988-
(e-lambda
989-
(args
990-
(p-underscore))
991-
(e-lookup-local
992-
(p-assign (ident "value"))))))))
993-
(e-lookup-local
994-
(p-assign (ident "result"))))))
980+
(p-assign (ident "container"))))
981+
(args
982+
(e-closure
983+
(captures
984+
(capture (ident "value")))
985+
(e-lambda
986+
(args
987+
(p-underscore))
988+
(e-lookup-local
989+
(p-assign (ident "value"))))))))
990+
(e-lookup-local
991+
(p-assign (ident "result")))))
995992
(annotation
996993
(ty-fn (effectful false)
997994
(ty-rigid-var (name "a"))

0 commit comments

Comments
 (0)