From 4dc548712d22015960a1ed12278ed2c5797c0d38 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 25 Nov 2025 09:36:20 -0500 Subject: [PATCH 1/6] Fix ZST copy bug --- src/cli/test/fx_platform_test.zig | 40 +++++++++++++++++++++++++++++++ src/eval/StackValue.zig | 9 +++++++ src/eval/interpreter.zig | 12 ++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/cli/test/fx_platform_test.zig b/src/cli/test/fx_platform_test.zig index c405a9456c..e6869640e1 100644 --- a/src/cli/test/fx_platform_test.zig +++ b/src/cli/test/fx_platform_test.zig @@ -264,3 +264,43 @@ test "fx platform expect with numeric literal" { try testing.expectEqualStrings("", run_result.stdout); try testing.expectEqualStrings("", run_result.stderr); } + +test "fx platform match returning string" { + const allocator = testing.allocator; + + try ensureRocBinary(allocator); + + // Run the app that has a match expression returning a string + // This tests that match expressions with string returns work correctly + const run_result = try std.process.Child.run(.{ + .allocator = allocator, + .argv = &[_][]const u8{ + "./zig-out/bin/roc", + "test/fx/match_str_return.roc", + }, + }); + defer allocator.free(run_result.stdout); + defer allocator.free(run_result.stderr); + + switch (run_result.term) { + .Exited => |code| { + if (code != 0) { + std.debug.print("Run failed with exit code {}\n", .{code}); + std.debug.print("STDOUT: {s}\n", .{run_result.stdout}); + std.debug.print("STDERR: {s}\n", .{run_result.stderr}); + return error.RunFailed; + } + }, + else => { + std.debug.print("Run terminated abnormally: {}\n", .{run_result.term}); + std.debug.print("STDOUT: {s}\n", .{run_result.stdout}); + std.debug.print("STDERR: {s}\n", .{run_result.stderr}); + return error.RunFailed; + }, + } + + // The app should run successfully and exit with code 0 + // It produces no output since it just returns a string that is discarded + try testing.expectEqualStrings("", run_result.stdout); + try testing.expectEqualStrings("", run_result.stderr); +} diff --git a/src/eval/StackValue.zig b/src/eval/StackValue.zig index 3e804c0aff..e62c89e2a5 100644 --- a/src/eval/StackValue.zig +++ b/src/eval/StackValue.zig @@ -59,6 +59,15 @@ pub fn copyToPtr(self: StackValue, layout_cache: *LayoutStore, dest_ptr: *anyopa .str => { // Clone the RocStr into the interpreter's heap std.debug.assert(self.ptr != null); + // Check if dest_ptr is properly aligned for RocStr + // If not, the caller doesn't expect a string (e.g., expected {} but got Str) + const dest_addr = @intFromPtr(dest_ptr); + const required_alignment = @alignOf(RocStr); + if (dest_addr % required_alignment != 0) { + // Destination not aligned for RocStr - skip copy + // This happens when return type is {} but evaluated result is Str + return; + } const src_str: *const RocStr = @ptrCast(@alignCast(self.ptr.?)); const dest_str: *RocStr = @ptrCast(@alignCast(dest_ptr)); dest_str.* = src_str.clone(ops); diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index d2277fd10f..df950d4c20 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -482,14 +482,22 @@ pub const Interpreter = struct { const result_value = try self.evalExprMinimal(header.body_idx, roc_ops, null); defer result_value.decref(&self.runtime_layout_store, roc_ops); - try result_value.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); + // Only copy result if it has non-zero size (skip for unit type {}) + const result_size = self.runtime_layout_store.layoutSize(result_value.layout); + if (result_size > 0) { + try result_value.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); + } return; } const result = try self.evalMinimal(expr_idx, roc_ops); defer result.decref(&self.runtime_layout_store, roc_ops); - try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); + // Only copy result if it has non-zero size (skip for unit type {}) + const result_size = self.runtime_layout_store.layoutSize(result.layout); + if (result_size > 0) { + try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); + } } fn evalExprMinimal( From 319b230294da81c12cdb837281a401963034355e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 25 Nov 2025 09:57:47 -0500 Subject: [PATCH 2/6] Skip invalid copies --- src/eval/StackValue.zig | 9 --------- src/eval/interpreter.zig | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/eval/StackValue.zig b/src/eval/StackValue.zig index e62c89e2a5..3e804c0aff 100644 --- a/src/eval/StackValue.zig +++ b/src/eval/StackValue.zig @@ -59,15 +59,6 @@ pub fn copyToPtr(self: StackValue, layout_cache: *LayoutStore, dest_ptr: *anyopa .str => { // Clone the RocStr into the interpreter's heap std.debug.assert(self.ptr != null); - // Check if dest_ptr is properly aligned for RocStr - // If not, the caller doesn't expect a string (e.g., expected {} but got Str) - const dest_addr = @intFromPtr(dest_ptr); - const required_alignment = @alignOf(RocStr); - if (dest_addr % required_alignment != 0) { - // Destination not aligned for RocStr - skip copy - // This happens when return type is {} but evaluated result is Str - return; - } const src_str: *const RocStr = @ptrCast(@alignCast(self.ptr.?)); const dest_str: *RocStr = @ptrCast(@alignCast(dest_ptr)); dest_str.* = src_str.clone(ops); diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index df950d4c20..18bcc2c6b3 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -482,9 +482,8 @@ pub const Interpreter = struct { const result_value = try self.evalExprMinimal(header.body_idx, roc_ops, null); defer result_value.decref(&self.runtime_layout_store, roc_ops); - // Only copy result if it has non-zero size (skip for unit type {}) - const result_size = self.runtime_layout_store.layoutSize(result_value.layout); - if (result_size > 0) { + // Only copy result if the result type is compatible with ret_ptr + if (self.shouldCopyResult(result_value, ret_ptr)) { try result_value.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); } return; @@ -493,13 +492,38 @@ pub const Interpreter = struct { const result = try self.evalMinimal(expr_idx, roc_ops); defer result.decref(&self.runtime_layout_store, roc_ops); - // Only copy result if it has non-zero size (skip for unit type {}) - const result_size = self.runtime_layout_store.layoutSize(result.layout); - if (result_size > 0) { + // Only copy result if the result type is compatible with ret_ptr + if (self.shouldCopyResult(result, ret_ptr)) { try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); } } + /// Check if we should copy the result to ret_ptr based on the result's layout. + /// Returns false if the result shouldn't be copied (e.g., when the evaluated result + /// type doesn't match what the caller expects based on pointer alignment). + fn shouldCopyResult(self: *Interpreter, result: StackValue, ret_ptr: *anyopaque) bool { + const result_size = self.runtime_layout_store.layoutSize(result.layout); + if (result_size == 0) { + // Zero-sized types don't need copying + return false; + } + + // Check if ret_ptr is properly aligned for the result type. + // This handles the case where the platform declares a function returning {} + // but the Roc code evaluates to a different type (e.g., Str). + // When the platform expects {}, the host provides a zero-sized or misaligned buffer. + // We detect this by checking alignment: if ret_ptr isn't aligned for the result type, + // the caller doesn't expect this type and we should skip the copy. + const required_alignment = result.layout.alignment(self.runtime_layout_store.targetUsize()); + const ret_addr = @intFromPtr(ret_ptr); + if (ret_addr % required_alignment.toByteUnits() != 0) { + // Destination not properly aligned for result type - skip copy + return false; + } + + return true; + } + fn evalExprMinimal( self: *Interpreter, expr_idx: can.CIR.Expr.Idx, From a401cf3c96f188d7cbfd38d78b7b0c81e729662e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 25 Nov 2025 11:20:22 -0500 Subject: [PATCH 3/6] Report mismatches with platform entrypoint types --- src/canonicalize/Can.zig | 49 ++++++ src/canonicalize/ModuleEnv.zig | 23 +++ src/check/Check.zig | 72 +++++++++ src/check/test/TestEnv.zig | 1 + src/cli/main.zig | 19 +++ src/compile/compile_build.zig | 119 +++++++++++++++ src/compile/compile_package.zig | 12 ++ src/compile/test/module_env_test.zig | 1 + src/eval/builtin_loading.zig | 1 + src/eval/interpreter.zig | 27 ++-- src/playground_wasm/main.zig | 1 + src/repl/repl_test.zig | 1 + test/fx/match_str_return.roc | 7 + test/fx/match_str_return_analysis.md | 217 +++++++++++++++++++++++++++ test/fx/test_explicit.roc | 6 + test/fx/test_type_mismatch.roc | 5 + 16 files changed, 547 insertions(+), 14 deletions(-) create mode 100644 test/fx/match_str_return.roc create mode 100644 test/fx/match_str_return_analysis.md create mode 100644 test/fx/test_explicit.roc create mode 100644 test/fx/test_type_mismatch.roc diff --git a/src/canonicalize/Can.zig b/src/canonicalize/Can.zig index 158a536256..e37d39efe4 100644 --- a/src/canonicalize/Can.zig +++ b/src/canonicalize/Can.zig @@ -1700,6 +1700,9 @@ pub fn canonicalizeFile( .platform => |h| { self.env.module_kind = .platform; try self.createExposedScope(h.exposes); + // Extract required type signatures for type checking + // This stores the types in env.requires_types without creating local definitions + try self.processRequiresSignatures(h.requires_signatures); }, .hosted => |h| { self.env.module_kind = .hosted; @@ -2589,6 +2592,52 @@ fn createExposedScope( } } +/// Process the requires_signatures from a platform header. +/// +/// This extracts the required type signatures (like `main! : () => {}`) from the platform +/// header and stores them in `env.requires_types`. These are used during app type checking +/// to ensure the app's provided values match the platform's expected types. +/// +/// Note: This does NOT create local definitions for the required identifiers. The platform +/// body can reference these identifiers as forward references that will be resolved to +/// the app's exports at runtime. +fn processRequiresSignatures(self: *Self, requires_signatures_idx: AST.TypeAnno.Idx) std.mem.Allocator.Error!void { + const requires_signatures = self.parse_ir.store.getTypeAnno(requires_signatures_idx); + + // The requires_signatures should be a record type like { main! : () => {} } + switch (requires_signatures) { + .record => |record| { + for (self.parse_ir.store.annoRecordFieldSlice(record.fields)) |field_idx| { + const field = self.parse_ir.store.getAnnoRecordField(field_idx) catch |err| switch (err) { + error.MalformedNode => { + // Skip malformed fields + continue; + }, + }; + + // Get the field name (e.g., "main!") + const field_name = self.parse_ir.tokens.resolveIdentifier(field.name) orelse continue; + const field_region = self.parse_ir.tokenizedRegionToRegion(field.region); + + // Canonicalize the type annotation for this required identifier + var type_anno_ctx = TypeAnnoCtx.init(.inline_anno); + const type_anno_idx = try self.canonicalizeTypeAnnoHelp(field.ty, &type_anno_ctx); + + // Store the required type in the module env + _ = try self.env.requires_types.append(self.env.gpa, .{ + .ident = field_name, + .type_anno = type_anno_idx, + .region = field_region, + }); + } + }, + else => { + // requires_signatures should always be a record type from parsing + // If it's not, just skip processing (parser would have reported an error) + }, + } +} + fn populateExports(self: *Self) std.mem.Allocator.Error!void { // Start a new scratch space for exports const scratch_exports_start = self.env.store.scratchDefTop(); diff --git a/src/canonicalize/ModuleEnv.zig b/src/canonicalize/ModuleEnv.zig index 5004e7de4c..e260ca8005 100644 --- a/src/canonicalize/ModuleEnv.zig +++ b/src/canonicalize/ModuleEnv.zig @@ -108,6 +108,10 @@ all_defs: CIR.Def.Span, all_statements: CIR.Statement.Span, /// Definitions that are exported by this module (populated by canonicalization) exports: CIR.Def.Span, +/// Required type signatures for platform modules (from `requires {} { main! : () => {} }`) +/// Maps identifier names to their expected type annotations. +/// Empty for non-platform modules. +requires_types: RequiredType.SafeList, /// All builtin stmts (temporary until module imports are working) builtin_statements: CIR.Statement.Span, /// All external declarations referenced in this module @@ -185,6 +189,19 @@ pub const DeferredNumericLiteral = struct { pub const SafeList = collections.SafeList(@This()); }; +/// Required type for platform modules - maps an identifier to its expected type annotation. +/// Used to enforce that apps provide values matching the platform's required types. +pub const RequiredType = struct { + /// The identifier name (e.g., "main!") + ident: Ident.Idx, + /// The canonicalized type annotation for this required value + type_anno: CIR.TypeAnno.Idx, + /// Region of the requirement for error reporting + region: Region, + + pub const SafeList = collections.SafeList(@This()); +}; + /// Relocate all pointers in the ModuleEnv by the given offset. /// This is used when loading a ModuleEnv from shared memory at a different address. pub fn relocate(self: *Self, offset: isize) void { @@ -192,6 +209,7 @@ pub fn relocate(self: *Self, offset: isize) void { self.common.relocate(offset); self.types.relocate(offset); self.external_decls.relocate(offset); + self.requires_types.relocate(offset); self.imports.relocate(offset); self.store.relocate(offset); self.deferred_numeric_literals.relocate(offset); @@ -276,6 +294,7 @@ pub fn init(gpa: std.mem.Allocator, source: []const u8) std.mem.Allocator.Error! .all_defs = .{ .span = .{ .start = 0, .len = 0 } }, .all_statements = .{ .span = .{ .start = 0, .len = 0 } }, .exports = .{ .span = .{ .start = 0, .len = 0 } }, + .requires_types = try RequiredType.SafeList.initCapacity(gpa, 4), .builtin_statements = .{ .span = .{ .start = 0, .len = 0 } }, .external_decls = try CIR.ExternalDecl.SafeList.initCapacity(gpa, 16), .imports = CIR.Import.Store.init(), @@ -312,6 +331,7 @@ pub fn deinit(self: *Self) void { self.common.deinit(self.gpa); self.types.deinit(); self.external_decls.deinit(self.gpa); + self.requires_types.deinit(self.gpa); self.imports.deinit(self.gpa); self.deferred_numeric_literals.deinit(self.gpa); // diagnostics are stored in the NodeStore, no need to free separately @@ -1713,6 +1733,7 @@ pub const Serialized = extern struct { all_defs: CIR.Def.Span, all_statements: CIR.Statement.Span, exports: CIR.Def.Span, + requires_types: RequiredType.SafeList.Serialized, builtin_statements: CIR.Statement.Span, external_decls: CIR.ExternalDecl.SafeList.Serialized, imports: CIR.Import.Store.Serialized, @@ -1760,6 +1781,7 @@ pub const Serialized = extern struct { self.exports = env.exports; self.builtin_statements = env.builtin_statements; + try self.requires_types.serialize(&env.requires_types, allocator, writer); try self.external_decls.serialize(&env.external_decls, allocator, writer); try self.imports.serialize(&env.imports, allocator, writer); @@ -1825,6 +1847,7 @@ pub const Serialized = extern struct { .all_defs = self.all_defs, .all_statements = self.all_statements, .exports = self.exports, + .requires_types = self.requires_types.deserialize(offset).*, .builtin_statements = self.builtin_statements, .external_decls = self.external_decls.deserialize(offset).*, .imports = (try self.imports.deserialize(offset, gpa)).*, diff --git a/src/check/Check.zig b/src/check/Check.zig index 1a3384f878..e7cc67b533 100644 --- a/src/check/Check.zig +++ b/src/check/Check.zig @@ -1062,6 +1062,78 @@ pub fn checkFile(self: *Self) std.mem.Allocator.Error!void { // Note that we can't use SCCs to determine the order to resolve defs // because anonymous static dispatch makes function order not knowable // before type inference + + // Process requires_types annotations for platforms + // This ensures the type store has the actual types for platform requirements + try self.processRequiresTypes(&env); +} + +/// Process the requires_types annotations for platform modules. +/// This generates the actual types from the type annotations stored in requires_types. +fn processRequiresTypes(self: *Self, env: *Env) std.mem.Allocator.Error!void { + const requires_types_slice = self.cir.requires_types.items.items; + for (requires_types_slice) |required_type| { + // Generate the type from the annotation + try self.generateAnnoTypeInPlace(required_type.type_anno, env, .annotation); + } +} + +/// Check that the app's exported values match the platform's required types. +/// This should be called after checkFile() to verify that app exports conform +/// to the platform's requirements. +pub fn checkPlatformRequirements(self: *Self, platform_env: *const ModuleEnv) std.mem.Allocator.Error!void { + const trace = tracy.trace(@src()); + defer trace.end(); + + // Create a solver env for type operations + var env = try self.env_pool.acquire(.generalized); + defer self.env_pool.release(env); + + // Iterate over the platform's required types + const requires_types_slice = platform_env.requires_types.items.items; + for (requires_types_slice) |required_type| { + // Get the identifier name for this required type + const required_ident = required_type.ident; + const required_ident_text = platform_env.getIdent(required_ident); + + // Find the matching export in the app + const app_exports_slice = self.cir.store.sliceDefs(self.cir.exports); + var found_export: ?CIR.Def.Idx = null; + + for (app_exports_slice) |def_idx| { + const def = self.cir.store.getDef(def_idx); + const pattern = self.cir.store.getPattern(def.pattern); + + if (pattern == .assign) { + const export_ident_text = self.cir.getIdent(pattern.assign.ident); + if (std.mem.eql(u8, export_ident_text, required_ident_text)) { + found_export = def_idx; + break; + } + } + } + + if (found_export) |export_def_idx| { + // Get the app export's type variable + const export_def = self.cir.store.getDef(export_def_idx); + const export_var = ModuleEnv.varFrom(export_def.pattern); + + // Copy the required type from the platform's type store into the app's type store + // First, convert the type annotation to a type variable in the platform's context + const required_type_var = ModuleEnv.varFrom(required_type.type_anno); + + // Copy the type from the platform's type store + const copied_required_var = try self.copyVar(required_type_var, platform_env, required_type.region); + + // Instantiate the copied variable before unifying (to avoid poisoning the cached copy) + const instantiated_required_var = try self.instantiateVar(copied_required_var, &env, .{ .explicit = required_type.region }); + + // Unify the app export's type with the platform's required type + // The platform type is the "expected" type, app export is "actual" + _ = try self.unifyFromAnno(instantiated_required_var, export_var, &env); + } + // Note: If the export is not found, the canonicalizer should have already reported an error + } } // repl // diff --git a/src/check/test/TestEnv.zig b/src/check/test/TestEnv.zig index b6757d10bf..0b21cbafcc 100644 --- a/src/check/test/TestEnv.zig +++ b/src/check/test/TestEnv.zig @@ -77,6 +77,7 @@ fn loadCompiledModule(gpa: std.mem.Allocator, bin_data: []const u8, module_name: .all_defs = serialized_ptr.all_defs, .all_statements = serialized_ptr.all_statements, .exports = serialized_ptr.exports, + .requires_types = serialized_ptr.requires_types.deserialize(@as(i64, @intCast(base_ptr))).*, .builtin_statements = serialized_ptr.builtin_statements, .external_decls = serialized_ptr.external_decls.deserialize(@as(i64, @intCast(base_ptr))).*, .imports = (try serialized_ptr.imports.deserialize(@as(i64, @intCast(base_ptr)), gpa)).*, diff --git a/src/cli/main.zig b/src/cli/main.zig index 24ff72b189..dcb4652605 100644 --- a/src/cli/main.zig +++ b/src/cli/main.zig @@ -1364,10 +1364,24 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons var exposed_modules = std.ArrayList([]const u8).empty; defer exposed_modules.deinit(allocs.gpa); + var has_platform = true; extractExposedModulesFromPlatform(allocs, platform_main_path, &exposed_modules) catch { // No platform found - that's fine, just continue with no platform modules + has_platform = false; }; + // Compile platform main.roc to get requires_types (if platform exists) + var platform_main_env: ?*ModuleEnv = null; + if (has_platform) { + platform_main_env = compileModuleToSharedMemory( + allocs, + platform_main_path, + "main.roc", + shm_allocator, + &builtin_modules, + ) catch null; + } + // Create header - use multi-module format const Header = struct { parent_base_addr: u64, @@ -1637,6 +1651,11 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons try app_checker.checkFile(); + // Check that app exports match platform requirements (if platform exists) + if (platform_main_env) |penv| { + try app_checker.checkPlatformRequirements(penv); + } + app_env_ptr.* = app_env; shm.updateHeader(); diff --git a/src/compile/compile_build.zig b/src/compile/compile_build.zig index 62b5c8f8b5..e7f5ee122c 100644 --- a/src/compile/compile_build.zig +++ b/src/compile/compile_build.zig @@ -16,12 +16,16 @@ const builtin = @import("builtin"); const build_options = @import("build_options"); const reporting = @import("reporting"); const eval = @import("eval"); +const check = @import("check"); const Report = reporting.Report; +const ReportBuilder = check.ReportBuilder; const BuiltinModules = eval.BuiltinModules; const Mode = @import("compile_package.zig").Mode; const Allocator = std.mem.Allocator; const ModuleEnv = can.ModuleEnv; +const Can = can.Can; +const Check = check.Check; const PackageEnv = @import("compile_package.zig").PackageEnv; const ModuleTimingInfo = @import("compile_package.zig").TimingInfo; const ImportResolver = @import("compile_package.zig").ImportResolver; @@ -544,10 +548,125 @@ pub const BuildEnv = struct { // Note: In single-threaded mode, buildRoot() runs synchronously and blocks // until all modules are complete, so no additional waiting is needed. + // Check platform requirements for app modules + try self.checkPlatformRequirements(); + // Deterministic emission: globally order reports by (min dependency depth from app, then module name) try self.emitDeterministic(); } + /// Check that app exports match platform requirements. + /// This is called after all modules are compiled and type-checked. + fn checkPlatformRequirements(self: *BuildEnv) !void { + // Find the app and platform packages + var app_pkg: ?[]const u8 = null; + var platform_pkg: ?[]const u8 = null; + + var pkg_it = self.packages.iterator(); + while (pkg_it.next()) |entry| { + const pkg = entry.value_ptr.*; + if (pkg.kind == .app) { + app_pkg = entry.key_ptr.*; + } else if (pkg.kind == .platform) { + platform_pkg = entry.key_ptr.*; + } + } + + // If we don't have both an app and a platform, nothing to check + const app_name = app_pkg orelse return; + const platform_name = platform_pkg orelse return; + + // Get the schedulers for both packages + const app_sched = self.schedulers.get(app_name) orelse return; + const platform_sched = self.schedulers.get(platform_name) orelse return; + + // Get the root module envs for both packages + const app_root_env = app_sched.getRootEnv() orelse return; + const platform_root_env = platform_sched.getRootEnv() orelse return; + + // If the platform has no requires_types, nothing to check + if (platform_root_env.requires_types.items.items.len == 0) { + return; + } + + // Get builtin indices and module + const builtin_indices = self.builtin_modules.builtin_indices; + const builtin_module_env = self.builtin_modules.builtin_module.env; + + // Build module_envs_map for type resolution + var module_envs_map = std.AutoHashMap(base.Ident.Idx, Can.AutoImportedType).init(self.gpa); + defer module_envs_map.deinit(); + + // Add builtin types (Bool, Try, Str) + if (app_root_env.common.findIdent("Bool")) |bi| { + try module_envs_map.put(bi, .{ + .env = builtin_module_env, + .statement_idx = builtin_indices.bool_type, + }); + } + if (app_root_env.common.findIdent("Try")) |ti| { + try module_envs_map.put(ti, .{ + .env = builtin_module_env, + .statement_idx = builtin_indices.try_type, + }); + } + if (app_root_env.common.findIdent("Str")) |si| { + try module_envs_map.put(si, .{ + .env = builtin_module_env, + .statement_idx = builtin_indices.str_type, + }); + } + + // Build common idents for the type checker + const common_idents = Check.CommonIdents{ + .module_name = app_root_env.module_name_idx, + .list = app_root_env.common.findIdent("List") orelse return, + .box = app_root_env.common.findIdent("Box") orelse return, + .@"try" = app_root_env.common.findIdent("Try") orelse return, + .bool_stmt = builtin_indices.bool_type, + .try_stmt = builtin_indices.try_type, + .str_stmt = builtin_indices.str_type, + .builtin_module = builtin_module_env, + }; + + // Create type checker for the app module + var checker = try Check.init( + self.gpa, + &app_root_env.types, + app_root_env, + &.{}, // No imported modules needed for checking exports + &module_envs_map, + &app_root_env.store.regions, + common_idents, + ); + defer checker.deinit(); + + // Check platform requirements against app exports + try checker.checkPlatformRequirements(platform_root_env); + + // If there are type problems, convert them to reports and emit via sink + if (checker.problems.problems.items.len > 0) { + const app_root_module = app_sched.getRootModule() orelse return; + + var rb = ReportBuilder.init( + self.gpa, + app_root_env, + app_root_env, + &checker.snapshots, + app_root_module.path, + &.{}, + &checker.import_mapping, + ); + defer rb.deinit(); + + for (checker.problems.problems.items) |prob| { + const rep = rb.build(prob) catch continue; + // Emit via sink with the module name (not path) to match other reports + self.sink.emitReport(app_name, app_root_module.name, rep); + } + } + } + // ------------------------ // Resolver implementation // ------------------------ diff --git a/src/compile/compile_package.zig b/src/compile/compile_package.zig index 759b2032ca..69b3d7f8ea 100644 --- a/src/compile/compile_package.zig +++ b/src/compile/compile_package.zig @@ -252,6 +252,18 @@ pub const PackageEnv = struct { self.emitted.deinit(self.gpa); } + /// Get the root module's env (first module added) + pub fn getRootEnv(self: *PackageEnv) ?*ModuleEnv { + if (self.modules.items.len == 0) return null; + return if (self.modules.items[0].env) |*env| env else null; + } + + /// Get the root module state (first module added) + pub fn getRootModule(self: *PackageEnv) ?*ModuleState { + if (self.modules.items.len == 0) return null; + return &self.modules.items[0]; + } + fn internModuleName(self: *PackageEnv, name: []const u8) !ModuleId { const gop = try self.module_names.getOrPut(self.gpa, name); if (!gop.found_existing) { diff --git a/src/compile/test/module_env_test.zig b/src/compile/test/module_env_test.zig index cd9dfee6f7..6feebae7f9 100644 --- a/src/compile/test/module_env_test.zig +++ b/src/compile/test/module_env_test.zig @@ -94,6 +94,7 @@ test "ModuleEnv.Serialized roundtrip" { .all_defs = deserialized_ptr.all_defs, .all_statements = deserialized_ptr.all_statements, .exports = deserialized_ptr.exports, + .requires_types = deserialized_ptr.requires_types.deserialize(@as(i64, @intCast(@intFromPtr(buffer.ptr)))).*, .builtin_statements = deserialized_ptr.builtin_statements, .external_decls = deserialized_ptr.external_decls.deserialize(@as(i64, @intCast(@intFromPtr(buffer.ptr)))).*, .imports = (try deserialized_ptr.imports.deserialize(@as(i64, @intCast(@intFromPtr(buffer.ptr))), deser_alloc)).*, diff --git a/src/eval/builtin_loading.zig b/src/eval/builtin_loading.zig index 36079ba82c..8709d296ec 100644 --- a/src/eval/builtin_loading.zig +++ b/src/eval/builtin_loading.zig @@ -69,6 +69,7 @@ pub fn loadCompiledModule(gpa: std.mem.Allocator, bin_data: []const u8, module_n .all_defs = serialized_ptr.all_defs, .all_statements = serialized_ptr.all_statements, .exports = serialized_ptr.exports, + .requires_types = serialized_ptr.requires_types.deserialize(@as(i64, @intCast(base_ptr))).*, .builtin_statements = serialized_ptr.builtin_statements, .external_decls = serialized_ptr.external_decls.deserialize(@as(i64, @intCast(base_ptr))).*, .imports = (try serialized_ptr.imports.deserialize(@as(i64, @intCast(base_ptr)), gpa)).*, diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index 18bcc2c6b3..8b09644863 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -483,7 +483,7 @@ pub const Interpreter = struct { defer result_value.decref(&self.runtime_layout_store, roc_ops); // Only copy result if the result type is compatible with ret_ptr - if (self.shouldCopyResult(result_value, ret_ptr)) { + if (try self.shouldCopyResult(result_value, ret_ptr)) { try result_value.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); } return; @@ -493,32 +493,31 @@ pub const Interpreter = struct { defer result.decref(&self.runtime_layout_store, roc_ops); // Only copy result if the result type is compatible with ret_ptr - if (self.shouldCopyResult(result, ret_ptr)) { + if (try self.shouldCopyResult(result, ret_ptr)) { try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); } } - /// Check if we should copy the result to ret_ptr based on the result's layout. - /// Returns false if the result shouldn't be copied (e.g., when the evaluated result - /// type doesn't match what the caller expects based on pointer alignment). - fn shouldCopyResult(self: *Interpreter, result: StackValue, ret_ptr: *anyopaque) bool { + /// Check if the result should be copied to ret_ptr based on the result's layout. + /// Returns false for zero-sized types (nothing to copy). + /// Validates that ret_ptr is properly aligned for the result type. + fn shouldCopyResult(self: *Interpreter, result: StackValue, ret_ptr: *anyopaque) !bool { const result_size = self.runtime_layout_store.layoutSize(result.layout); if (result_size == 0) { // Zero-sized types don't need copying return false; } - // Check if ret_ptr is properly aligned for the result type. - // This handles the case where the platform declares a function returning {} - // but the Roc code evaluates to a different type (e.g., Str). - // When the platform expects {}, the host provides a zero-sized or misaligned buffer. - // We detect this by checking alignment: if ret_ptr isn't aligned for the result type, - // the caller doesn't expect this type and we should skip the copy. + // Validate alignment: ret_ptr must be properly aligned for the result type. + // A mismatch here indicates a type error between what the platform expects + // and what the Roc code returns. This should have been caught at compile + // time, but if the type checking didn't enforce the constraint, we catch + // it here at runtime. const required_alignment = result.layout.alignment(self.runtime_layout_store.targetUsize()); const ret_addr = @intFromPtr(ret_ptr); if (ret_addr % required_alignment.toByteUnits() != 0) { - // Destination not properly aligned for result type - skip copy - return false; + // Type mismatch detected at runtime + return error.TypeMismatch; } return true; diff --git a/src/playground_wasm/main.zig b/src/playground_wasm/main.zig index cb590a28c8..3e223f8e8b 100644 --- a/src/playground_wasm/main.zig +++ b/src/playground_wasm/main.zig @@ -971,6 +971,7 @@ fn compileSource(source: []const u8) !CompilerStageData { .all_defs = serialized_ptr.all_defs, .all_statements = serialized_ptr.all_statements, .exports = serialized_ptr.exports, + .requires_types = serialized_ptr.requires_types.deserialize(@as(i64, @intCast(base_ptr))).*, .builtin_statements = serialized_ptr.builtin_statements, .external_decls = serialized_ptr.external_decls.deserialize(@as(i64, @intCast(base_ptr))).*, .imports = (try serialized_ptr.imports.deserialize(@as(i64, @intCast(base_ptr)), gpa)).*, diff --git a/src/repl/repl_test.zig b/src/repl/repl_test.zig index 35f26d157e..0584cf9b43 100644 --- a/src/repl/repl_test.zig +++ b/src/repl/repl_test.zig @@ -80,6 +80,7 @@ fn loadCompiledModule(gpa: std.mem.Allocator, bin_data: []const u8, module_name: .all_defs = serialized_ptr.all_defs, .all_statements = serialized_ptr.all_statements, .exports = serialized_ptr.exports, + .requires_types = serialized_ptr.requires_types.deserialize(@as(i64, @intCast(base_ptr))).*, .builtin_statements = serialized_ptr.builtin_statements, .external_decls = serialized_ptr.external_decls.deserialize(@as(i64, @intCast(base_ptr))).*, .imports = (try serialized_ptr.imports.deserialize(@as(i64, @intCast(base_ptr)), gpa)).*, diff --git a/test/fx/match_str_return.roc b/test/fx/match_str_return.roc new file mode 100644 index 0000000000..4b2cebb2f0 --- /dev/null +++ b/test/fx/match_str_return.roc @@ -0,0 +1,7 @@ +app [main!] { pf: platform "./platform/main.roc" } + +main! = || { + match 0 { + _ => "0" + } +} diff --git a/test/fx/match_str_return_analysis.md b/test/fx/match_str_return_analysis.md new file mode 100644 index 0000000000..9515940ea7 --- /dev/null +++ b/test/fx/match_str_return_analysis.md @@ -0,0 +1,217 @@ +# Analysis: Match Expression String Return Crash + +## Summary + +`roc check` succeeds but `roc run` crashes with "incorrect alignment" for the following code: + +```roc +app [main!] { pf: platform "./platform/main.roc" } + +main! = || { + match 0 { + _ => "0" + } +} +``` + +## Root Cause + +The crash occurs because the `ret_ptr` (return pointer) passed from the host to the Roc entrypoint is not properly aligned for `RocStr`, which requires 8-byte alignment on 64-bit systems. + +## Detailed Analysis + +### Pipeline Differences + +**`roc check`:** +- Parses, canonicalizes, and type-checks the code +- Does NOT evaluate or execute the code at runtime +- The `copyToPtr` function is never called with an actual pointer + +**`roc run`:** +1. Compiles modules to shared memory (`setupSharedMemoryWithModuleEnv`) +2. Links host library with interpreter shim to create executable +3. Launches the executable which reads from shared memory +4. Calls `roc_entrypoint` with host-provided `ret_ptr` +5. Interpreter evaluates the expression and calls `copyToPtr` to write result + +### Where the Crash Happens + +**File:** `src/eval/StackValue.zig:62-64` + +```zig +pub fn copyToPtr(self: StackValue, layout_cache: *LayoutStore, dest_ptr: *anyopaque, ops: *RocOps) !void { + // ... + if (self.layout.tag == .scalar) { + switch (self.layout.data.scalar.tag) { + .str => { + const src_str: *const RocStr = @ptrCast(@alignCast(self.ptr.?)); + const dest_str: *RocStr = @ptrCast(@alignCast(dest_ptr)); // <-- CRASH HERE + dest_str.* = src_str.clone(ops); + return; + }, + // ... + } + } + // ... +} +``` + +The `@alignCast(dest_ptr)` requires the pointer to be aligned to `RocStr`'s alignment (8 bytes on 64-bit). If `dest_ptr` is misaligned, Zig's runtime safety check triggers a panic. + +### The Call Chain + +1. **Host calls Roc:** `test/fx/platform/host.zig:201` + ```zig + var ret: [0]u8 = undefined; + roc__main_for_host(&roc_ops, @ptrCast(&ret), @ptrCast(&args)); + ``` + +2. **Roc entrypoint:** `src/interpreter_shim/main.zig:62` + ```zig + export fn roc_entrypoint(entry_idx: u32, ops: *RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) + ``` + +3. **Interpreter evaluates:** `src/eval/interpreter.zig:492` + ```zig + try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); + ``` + +4. **copyToPtr crashes** when casting `ret_ptr` to `*RocStr` + +### Why `main!` Returns a String + +The `main!` function in this test returns a `Str` (from the match expression), but the platform defines: + +```roc +main! : List(Str) => {} +``` + +The platform expects `main!` to return `{}` (unit), but the actual Roc code returns `"0"` (a string). However, `roc check` passes because: +- The match expression type-checks (it returns `Str`) +- The effectful block discards its result value + +But at runtime, the interpreter still evaluates the match expression and tries to copy the string result, even though the return type should be `{}`. + +## The Bug + +There appear to be two issues: + +### Issue 1: Host Provides Wrong Return Buffer + +The host in `test/fx/platform/host.zig:194` provides: +```zig +var ret: [0]u8 = undefined; // Zero-sized buffer for {} return type +``` + +But the interpreter tries to write a `RocStr` (24 bytes) to this location. + +### Issue 2: Discarded Expression Values Still Copied + +The interpreter evaluates the match expression and attempts to copy its result to `ret_ptr`, even when the result should be discarded (because `main!` returns `{}`). + +## Potential Fixes + +### Fix Option 1: Interpreter Should Not Copy Discarded Results + +In `evaluateExpression`, check if the return type is unit `{}` and skip the `copyToPtr` call: + +```zig +// If return type is unit (zero-sized), don't copy anything +const result_size = layout_cache.layoutSize(result.layout); +if (result_size > 0) { + try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); +} +``` + +### Fix Option 2: Ensure ret_ptr Alignment + +The host should always provide an aligned return buffer, or the interpreter should check alignment before casting: + +```zig +.str => { + const alignment = @alignOf(RocStr); + if (@intFromPtr(dest_ptr) % alignment != 0) { + return error.MisalignedPointer; + } + // ... proceed with cast +}, +``` + +### Fix Option 3: Handle Unit Return Type at Entry Point + +The interpreter shim should detect when `main!` returns `{}` and not attempt to write any result: + +```zig +// In evaluateExpression, check layout and skip copy for ZSTs +if (layout_cache.layoutSize(result.layout) == 0) { + return; // Nothing to copy for zero-sized types +} +try result.copyToPtr(...); +``` + +## Implemented Fix + +The fix was implemented in `src/eval/interpreter.zig` in the `evaluateExpression` function. + +We added a `shouldCopyResult` helper function that checks whether the result should be copied to `ret_ptr` based on: +1. Zero-sized types (size == 0) never need copying +2. If `ret_ptr` is not properly aligned for the result type, skip the copy + +```zig +/// Check if we should copy the result to ret_ptr based on the result's layout. +/// Returns false if the result shouldn't be copied (e.g., when the evaluated result +/// type doesn't match what the caller expects based on pointer alignment). +fn shouldCopyResult(self: *Interpreter, result: StackValue, ret_ptr: *anyopaque) bool { + const result_size = self.runtime_layout_store.layoutSize(result.layout); + if (result_size == 0) { + // Zero-sized types don't need copying + return false; + } + + // Check if ret_ptr is properly aligned for the result type. + // This handles the case where the platform declares a function returning {} + // but the Roc code evaluates to a different type (e.g., Str). + // When the platform expects {}, the host provides a zero-sized or misaligned buffer. + // We detect this by checking alignment: if ret_ptr isn't aligned for the result type, + // the caller doesn't expect this type and we should skip the copy. + const required_alignment = result.layout.alignment(self.runtime_layout_store.targetUsize()); + const ret_addr = @intFromPtr(ret_ptr); + if (ret_addr % required_alignment.toByteUnits() != 0) { + // Destination not properly aligned for result type - skip copy + return false; + } + + return true; +} +``` + +The `evaluateExpression` function now calls this helper before copying: + +```zig +// Only copy result if the result type is compatible with ret_ptr +if (self.shouldCopyResult(result, ret_ptr)) { + try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); +} +``` + +This fix works because: +1. When the host expects `{}`, it provides a pointer to a zero-sized buffer which may not be aligned +2. When we try to copy the result (e.g., `Str`), we first check if `ret_ptr` is aligned for the result type +3. If misaligned, we skip the copy since the caller doesn't expect this result type anyway +4. This avoids the Zig runtime "incorrect alignment" panic in `copyToPtr` + +### Why Not Use Type Annotations? + +We initially tried to use the function's type annotation to determine the expected return type, but this approach failed because: +1. Type unification merges declared and inferred types, so querying the pattern's type after type-checking returns the unified type (e.g., `Str`) not the declared type (`{}`) +2. The `def.annotation` field is `null` for definitions where the type annotation is on a separate line in the Roc source + +The alignment-based approach is robust because it relies on the host's behavior: when expecting `{}`, the host allocates a zero-sized or arbitrarily-aligned buffer. + +## Test Reproduction + +The test case is now in: +- **Roc file:** `test/fx/match_str_return.roc` +- **Test:** `src/cli/test/fx_platform_test.zig` - "fx platform match returning string" + +Run with: `zig build test` and look for the fx_platform_test failure. diff --git a/test/fx/test_explicit.roc b/test/fx/test_explicit.roc new file mode 100644 index 0000000000..6f41a0d37d --- /dev/null +++ b/test/fx/test_explicit.roc @@ -0,0 +1,6 @@ +app [main!] { pf: platform "./platform/main.roc" } + +main! : () => {} +main! = || { + "hello" +} diff --git a/test/fx/test_type_mismatch.roc b/test/fx/test_type_mismatch.roc new file mode 100644 index 0000000000..035c6e03e9 --- /dev/null +++ b/test/fx/test_type_mismatch.roc @@ -0,0 +1,5 @@ +app [main!] { pf: platform "./platform/main.roc" } + +main! = || { + "hello" +} From 963ddce26f7543a706929262b3c4de52b2721fdb Mon Sep 17 00:00:00 2001 From: Luke Boswell Date: Wed, 26 Nov 2025 09:39:26 +1100 Subject: [PATCH 4/6] remove stale builtin files --- src/builtins/roc/Bool.roc | 80 ----------------- src/builtins/roc/Try.roc | 177 -------------------------------------- src/builtins/roc/main.roc | 6 -- 3 files changed, 263 deletions(-) delete mode 100644 src/builtins/roc/Bool.roc delete mode 100644 src/builtins/roc/Try.roc delete mode 100644 src/builtins/roc/main.roc diff --git a/src/builtins/roc/Bool.roc b/src/builtins/roc/Bool.roc deleted file mode 100644 index 43c45295cb..0000000000 --- a/src/builtins/roc/Bool.roc +++ /dev/null @@ -1,80 +0,0 @@ -module [Bool, Eq, true, false, not, is_eq, is_not_eq] - -## Defines a type that can be compared for total equality. -## -## Total equality means that all values of the type can be compared to each -## other, and two values `a`, `b` are identical if and only if `is_eq(a, b)` is -## `Bool.true`. -## -## Not all types support total equality. For example, [`F32`](Num#F32) and [`F64`](Num#F64) can -## be a `NaN` ([Not a Number](https://en.wikipedia.org/wiki/NaN)), and the -## [IEEE-754](https://en.wikipedia.org/wiki/IEEE_754) floating point standard -## specifies that two `NaN`s are not equal. -Eq(a) : a - where - ## Returns `Bool.true` if the input values are equal. This is - ## equivalent to the logic - ## [XNOR](https://en.wikipedia.org/wiki/Logical_equality) gate. The infix - ## operator `==` can be used as shorthand for `Bool.is_eq`. - ## - ## **Note** that when `is_eq` is determined by the Roc compiler, values are - ## compared using structural equality. The rules for this are as follows: - ## - ## 1. Tags are equal if their name and also contents are equal. - ## 2. Records are equal if their fields are equal. - ## 3. The collections [Str], [List], [Dict], and [Set] are equal iff they - ## are the same length and their elements are equal. - ## 4. [Num] values are equal if their numbers are equal. However, if both - ## inputs are *NaN* then `is_eq` returns `Bool.false`. Refer to `Num.is_nan` - ## for more detail. - ## 5. Functions cannot be compared for structural equality, therefore Roc - ## cannot derive `is_eq` for types that contain functions. - a.is_eq(a) -> Bool, - -## Represents the boolean true and false using an nominal type. -## `Bool` implements the `Eq` ability. -Bool := [True, False] - -## The boolean true value. -true : Bool -true = Bool.True - -## The boolean false value. -false : Bool -false = Bool.False - -## Satisfies the interface of `Eq` -is_eq : Bool, Bool -> Bool -is_eq = |b1, b2| match (b1, b2) { - (Bool.True, Bool.True) => true - (Bool.False, Bool.False) => true - _ => false -} - -## Returns `Bool.false` when given `Bool.true`, and vice versa. This is -## equivalent to the logic [NOT](https://en.wikipedia.org/wiki/Negation) -## gate. The operator `!` can also be used as shorthand for `Bool.not`. -## ```roc -## expect Bool.not(Bool.false) == Bool.true -## expect Bool.false != Bool.true -## ``` -not : Bool -> Bool -not = |b| match b { - Bool.True => false - Bool.False => true -} - -## This will call the function `Bool.is_eq` on the inputs, and then `Bool.not` -## on the result. The is equivalent to the logic -## [XOR](https://en.wikipedia.org/wiki/Exclusive_or) gate. The infix operator -## `!=` can also be used as shorthand for `Bool.is_not_eq`. -## -## **Note** that `is_not_eq` does not accept arguments whose types contain -## functions. -## ```roc -## expect Bool.is_not_eq(Bool.false, Bool.true) == Bool.true -## expect (Bool.false != Bool.false) == Bool.false -## expect "Apples" != "Oranges" -## ``` -is_not_eq : a, a -> Bool where a.Eq -is_not_eq = |a, b| not(a.is_eq(b)) diff --git a/src/builtins/roc/Try.roc b/src/builtins/roc/Try.roc deleted file mode 100644 index 788d85029b..0000000000 --- a/src/builtins/roc/Try.roc +++ /dev/null @@ -1,177 +0,0 @@ -module [ - Try, - is_ok, - is_err, - is_eq, - map_ok, - map_err, - on_err, - on_err!, - map_both, - map2, - try, - with_default, -] - -import Bool exposing [Bool.*] - -## The result of an operation that could fail: either the operation went -## okay, or else there was an error of some sort. -Try(ok, err) := [Ok(ok), Err(err)] - -## Returns `Bool.true` if the result indicates a success, else returns `Bool.false`. -## ```roc -## Ok(5).is_ok() -## ``` -is_ok : Try(ok, err) -> Bool -is_ok = |result| match result { - Try.Ok(_) => Bool.true - Try.Err(_) => Bool.false -} - -## Returns `Bool.true` if the result indicates a failure, else returns `Bool.false`. -## ```roc -## Err("uh oh").is_err() -## ``` -is_err : Try(ok, err) -> Bool -is_err = |result| match result { - Try.Ok(_) => Bool.false - Try.Err(_) => Bool.true -} - -## If the result is `Ok`, returns the value it holds. Otherwise, returns -## the given default value. -## -## Note: This function should be used sparingly, because it hides that an error -## happened, which will make debugging harder. Prefer using `?` to forward errors or -## handle them explicitly with `when`. -## ```roc -## Err("uh oh").with_default(42) # = 42 -## -## Ok(7).with_default(42) # = 7 -## ``` -with_default : Try(ok, err), ok -> ok -with_default = |result, default| match result { - Try.Ok(value) => value - Try.Err(_) => default -} - -## If the result is `Ok`, transforms the value it holds by running a conversion -## function on it. Then returns a new `Ok` holding the transformed value. If the -## result is `Err`, this has no effect. Use [map_err] to transform an `Err`. -## ```roc -## Ok(12).map_ok(Num.neg) # = Ok(-12) -## -## Err("yipes!").map_ok(Num.neg) # = Err("yipes!") -## ``` -## -## Functions like `map` are common in Roc; see for example [List.map], -## `Set.map`, and `Dict.map`. -map_ok : Try(a, err), (a -> b) -> Try(b, err) -map_ok = |result, transform| match result { - Try.Ok(v) => Try.Ok(transform(v)) - Try.Err(e) => Try.Err(e) -} - -## If the result is `Err`, transforms the value it holds by running a conversion -## function on it. Then returns a new `Err` holding the transformed value. If -## the result is `Ok`, this has no effect. Use [map] to transform an `Ok`. -## ```roc -## [].last().map_err(|_| ProvidedListIsEmpty) # = Err(ProvidedListIsEmpty) -## -## [4].last().map_err(|_| ProvidedListIsEmpty) # = Ok(4) -## ``` -map_err : Try(ok, a), (a -> b) -> Try(ok, b) -map_err = |result, transform| match result { - Try.Ok(v) => Try.Ok(v) - Try.Err(e) => Try.Err(transform(e)) -} - -## If the result is `Err`, transforms the entire result by running a conversion -## function on the value the `Err` holds. Then returns that new result. If the -## result is `Ok`, this has no effect. Use `?` or [try] to transform an `Ok`. -## ```roc -## Try.on_err(Ok(10), Str.to_u64) # = Ok(10) -## -## Try.on_err(Err("42"), Str.to_u64) # = Ok(42) -## -## Try.on_err(Err("string"), Str.to_u64) # = Err(InvalidNumStr) -## ``` -on_err : Try(a, err), (err -> Try(a, other_err)) -> Try(a, other_err) -on_err = |result, transform| match result { - Try.Ok(v) => Try.Ok(v) - Try.Err(e) => transform(e) -} - -expect Try.on_err(Ok(10), Str.to_u64) == Try.Ok(10) -expect Try.on_err(Err("42"), Str.to_u64) == Try.Ok(42) -expect Try.on_err(Err("string"), Str.to_u64) == Try.Err(InvalidNumStr) - -## Like [on_err], but it allows the transformation function to produce effects. -## -## ```roc -## Err("missing user").on_err(|msg| { -## Stdout.line!("ERROR: ${msg}")? -## Err(msg) -## }) -## ``` -on_err! : Try(a, err), (err => Try(a, other_err)) => Try(a, other_err) -on_err! = |result, transform!| match result { - Try.Ok(v) => Try.Ok(v) - Try.Err(e) => transform!(e) -} - -## Maps both the `Ok` and `Err` values of a `Try` to new values. -map_both : Try(ok1, err1), (ok1 -> ok2), (err1 -> err2) -> Try(ok2, err2) -map_both = |result, ok_transform, err_transform| match result { - Try. Ok(val) => Try.Ok(ok_transform(val)) - Try. Err(err) => Try.Err(err_transform(err)) -} - -## Maps the `Ok` values of two `Try`s to a new value using a given transformation, -## or returns the first `Err` value encountered. -map2 : Try(a, err), Try(b, err), (a, b -> c) -> Try(c, err) -map2 = |first_result, second_result, transform| match (first_result, second_result) { - (Try.Ok(first), Try.Ok(second)) => Ok(transform(first, second)) - (Try.Err(err), _) => Try.Err(err) - (_, Try.Err(err)) => Try.Err(err) -} - -## If the result is `Ok`, transforms the entire result by running a conversion -## function on the value the `Ok` holds. Then returns that new result. If the -## result is `Err`, this has no effect. Use `on_err` to transform an `Err`. -## -## We recommend using `?` instead of `try`, it makes the code easier to read. -## ```roc -## Ok(-1).try(|num| if num < 0 then Err("negative!") else Ok(-num)) # = Err("negative!") -## -## Ok(1).try(|num| if num < 0 then Err("negative!") else Ok(-num)) # = Ok(-1) -## -## Err("yipes!").try(|num| if num < 0 then Err("negative!") else Ok(-num)) # = Err("yipes!") -## ``` -try : Try(a, err), (a -> Try(b, err)) -> Try(b, err) -try = |result, transform| match result { - Try.Ok(v) => transform(v) - Try.Err(e) => Try.Err(e) -} - -expect Ok(-1).try(|num| if num < 0 then Err("negative!") else Ok(-num)) == Try.Err("negative!") -expect Ok(1).try(|num| if num < 0 then Err("negative!") else Ok(-num)) == Try.Ok(-1) -expect Err("yipes!").try(|num| if num < 0 then Err("negative!") else Ok(-num)) == Try.Err("yipes!") - -## Implementation of [Bool.Eq]. Checks if two results that have both `ok` and `err` types that are `Eq` are themselves equal. -## -## ```roc -## Ok("Hello").is_eq(Ok("Hello")) -## ``` -is_eq : Try(ok, err), Try(ok, err) -> Bool where [ok.Eq, err.Eq] -is_eq = |r1, r2| match (r1, r2) { - (Try.Ok(ok1), Try.Ok(ok2)) => ok1 == ok2 - (Try.Err(err1), Try.Err(err2)) => err1 == err2 -} - -expect Try.Ok(1) == Try.Ok(1) -expect Try.Ok(2) != Try.Ok(1) -expect Try.Err("Foo") == Try.Err("Foo") -expect Try.Err("Bar") != Try.Err("Foo") -expect Try.Ok("Foo") != Try.Err("Foo") diff --git a/src/builtins/roc/main.roc b/src/builtins/roc/main.roc deleted file mode 100644 index 83be04ddd1..0000000000 --- a/src/builtins/roc/main.roc +++ /dev/null @@ -1,6 +0,0 @@ -package - [ - Bool, - Try, - ] - {} \ No newline at end of file From a6b94fc5a94aabb4ff66eec51eb8b85d7292e607 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 25 Nov 2025 18:38:57 -0500 Subject: [PATCH 5/6] More platform fixes --- src/canonicalize/Can.zig | 46 +++++- src/check/Check.zig | 32 +++- src/cli/cli_args.zig | 1 + src/cli/main.zig | 43 +++-- src/cli/test/fx_platform_test.zig | 4 +- src/eval/interpreter.zig | 251 +++++++++++++++++++++++++----- test/fx/app.roc | 13 +- test/fx/match_str_return.roc | 10 +- test/fx/stdin_echo.roc | 5 +- test/fx/stdin_simple.roc | 5 +- test/fx/stdin_test.roc | 7 +- test/fx/stdin_to_stdout.roc | 8 +- test/fx/test_direct_string.roc | 7 + test/fx/test_one_call.roc | 11 ++ test/fx/test_with_wrapper.roc | 10 ++ test/serialization_size_check.zig | 2 +- 16 files changed, 376 insertions(+), 79 deletions(-) create mode 100644 test/fx/test_direct_string.roc create mode 100644 test/fx/test_one_call.roc create mode 100644 test/fx/test_with_wrapper.roc diff --git a/src/canonicalize/Can.zig b/src/canonicalize/Can.zig index 4cd8832af7..ae96c0f01e 100644 --- a/src/canonicalize/Can.zig +++ b/src/canonicalize/Can.zig @@ -1702,7 +1702,8 @@ pub fn canonicalizeFile( try self.createExposedScope(h.exposes); // Extract required type signatures for type checking // This stores the types in env.requires_types without creating local definitions - try self.processRequiresSignatures(h.requires_signatures); + // Pass requires_rigids so R1, R2, etc. are in scope when processing signatures + try self.processRequiresSignatures(h.requires_rigids, h.requires_signatures); }, .hosted => |h| { self.env.module_kind = .hosted; @@ -2598,10 +2599,40 @@ fn createExposedScope( /// header and stores them in `env.requires_types`. These are used during app type checking /// to ensure the app's provided values match the platform's expected types. /// +/// The requires_rigids parameter contains the type variables declared in `requires { R1, R2 }`. +/// These are introduced into scope before processing the signatures so that references to +/// R1, R2, etc. in the signatures are properly resolved as type variables. +/// /// Note: This does NOT create local definitions for the required identifiers. The platform /// body can reference these identifiers as forward references that will be resolved to /// the app's exports at runtime. -fn processRequiresSignatures(self: *Self, requires_signatures_idx: AST.TypeAnno.Idx) std.mem.Allocator.Error!void { +fn processRequiresSignatures(self: *Self, requires_rigids_idx: AST.Collection.Idx, requires_signatures_idx: AST.TypeAnno.Idx) std.mem.Allocator.Error!void { + // First, process the requires_rigids to add them to the type variable scope + // This allows R1, R2, etc. to be recognized when processing the signatures + const rigids_collection = self.parse_ir.store.getCollection(requires_rigids_idx); + for (self.parse_ir.store.exposedItemSlice(.{ .span = rigids_collection.span })) |exposed_idx| { + const exposed_item = self.parse_ir.store.getExposedItem(exposed_idx); + switch (exposed_item) { + .upper_ident => |upper| { + // Get the identifier for this rigid type variable (e.g., "R1") + const rigid_name = self.parse_ir.tokens.resolveIdentifier(upper.ident) orelse continue; + const rigid_region = self.parse_ir.tokenizedRegionToRegion(upper.region); + + // Create a type annotation for this rigid variable + const rigid_anno_idx = try self.env.addTypeAnno(.{ .rigid_var = .{ + .name = rigid_name, + } }, rigid_region); + + // Introduce it into the type variable scope + _ = try self.scopeIntroduceTypeVar(rigid_name, rigid_anno_idx); + }, + else => { + // Skip lower_ident, upper_ident_star, malformed - these aren't valid for requires rigids + }, + } + } + + // Now process the requires_signatures with the rigids in scope const requires_signatures = self.parse_ir.store.getTypeAnno(requires_signatures_idx); // The requires_signatures should be a record type like { main! : () => {} } @@ -7160,6 +7191,17 @@ fn canonicalizeTypeAnnoBasicType( } } + // Check if this is a type variable in scope (e.g., R1, R2 from requires { R1, R2 }) + switch (self.scopeLookupTypeVar(type_name_ident)) { + .found => |found_anno_idx| { + // Found a type variable with this name - create a reference to it + return try self.env.addTypeAnno(.{ .rigid_var_lookup = .{ + .ref = found_anno_idx, + } }, region); + }, + .not_found => {}, + } + // Not found anywhere - undeclared type return try self.env.pushMalformed(TypeAnno.Idx, Diagnostic{ .undeclared_type = .{ .name = type_name_ident, diff --git a/src/check/Check.zig b/src/check/Check.zig index e7cc67b533..4d665df481 100644 --- a/src/check/Check.zig +++ b/src/check/Check.zig @@ -1128,9 +1128,15 @@ pub fn checkPlatformRequirements(self: *Self, platform_env: *const ModuleEnv) st // Instantiate the copied variable before unifying (to avoid poisoning the cached copy) const instantiated_required_var = try self.instantiateVar(copied_required_var, &env, .{ .explicit = required_type.region }); - // Unify the app export's type with the platform's required type - // The platform type is the "expected" type, app export is "actual" - _ = try self.unifyFromAnno(instantiated_required_var, export_var, &env); + // Create a copy of the export's type for unification. + // This prevents unification failure from corrupting the app's actual types + // (which would cause the interpreter to fail when trying to get layouts). + const export_copy = try self.copyVar(export_var, self.cir, required_type.region); + const instantiated_export_copy = try self.instantiateVar(export_copy, &env, .{ .explicit = required_type.region }); + + // Unify the platform's required type with the COPY of the app's export type. + // The platform type is the "expected" type, app export copy is "actual". + _ = try self.unifyFromAnno(instantiated_required_var, instantiated_export_copy, &env); } // Note: If the export is not found, the canonicalizer should have already reported an error } @@ -3226,8 +3232,10 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) // without doing any additional work try self.unifyWith(expr_var, .err, env); } else { - // From the base function type, extract the actual function info - const mb_func: ?types_mod.Func = inner_blk: { + // From the base function type, extract the actual function info + // and also track whether the function is effectful + const FuncInfo = struct { func: types_mod.Func, is_effectful: bool }; + const mb_func_info: ?FuncInfo = inner_blk: { // Here, we unwrap the function, following aliases, to get // the actual function we want to check against var var_ = func_var; @@ -3235,9 +3243,9 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) switch (self.types.resolveVar(var_).desc.content) { .structure => |flat_type| { switch (flat_type) { - .fn_pure => |func| break :inner_blk func, - .fn_unbound => |func| break :inner_blk func, - .fn_effectful => |func| break :inner_blk func, + .fn_pure => |func| break :inner_blk FuncInfo{ .func = func, .is_effectful = false }, + .fn_unbound => |func| break :inner_blk FuncInfo{ .func = func, .is_effectful = false }, + .fn_effectful => |func| break :inner_blk FuncInfo{ .func = func, .is_effectful = true }, else => break :inner_blk null, } }, @@ -3248,6 +3256,14 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) } } }; + const mb_func = if (mb_func_info) |info| info.func else null; + + // If the function being called is effectful, mark this expression as effectful + if (mb_func_info) |info| { + if (info.is_effectful) { + does_fx = true; + } + } // Get the name of the function (for error messages) const func_name: ?Ident.Idx = inner_blk: { diff --git a/src/cli/cli_args.zig b/src/cli/cli_args.zig index 7247428513..7d98af0cea 100644 --- a/src/cli/cli_args.zig +++ b/src/cli/cli_args.zig @@ -136,6 +136,7 @@ pub const ExperimentalLspArgs = struct { pub fn parse(alloc: mem.Allocator, args: []const []const u8) !CliArgs { if (args.len == 0) return try parseRun(alloc, args); + if (mem.eql(u8, args[0], "run")) return try parseRun(alloc, args[1..]); if (mem.eql(u8, args[0], "check")) return parseCheck(args[1..]); if (mem.eql(u8, args[0], "build")) return parseBuild(args[1..]); if (mem.eql(u8, args[0], "bundle")) return try parseBundle(alloc, args[1..]); diff --git a/src/cli/main.zig b/src/cli/main.zig index dcb4652605..e19cf8a3fd 100644 --- a/src/cli/main.zig +++ b/src/cli/main.zig @@ -1370,19 +1370,10 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons has_platform = false; }; - // Compile platform main.roc to get requires_types (if platform exists) - var platform_main_env: ?*ModuleEnv = null; - if (has_platform) { - platform_main_env = compileModuleToSharedMemory( - allocs, - platform_main_path, - "main.roc", - shm_allocator, - &builtin_modules, - ) catch null; - } - - // Create header - use multi-module format + // IMPORTANT: Create header FIRST before any module compilation. + // The interpreter_shim expects the Header to be at FIRST_ALLOC_OFFSET (504). + // If we compile modules first, they would occupy that offset and break + // shared memory layout assumptions. const Header = struct { parent_base_addr: u64, module_count: u32, @@ -1395,6 +1386,20 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons const shm_base_addr = @intFromPtr(shm.base_ptr); header_ptr.parent_base_addr = shm_base_addr; + // Compile platform main.roc to get requires_types (if platform exists) + // This must come AFTER header allocation to preserve memory layout. + var platform_main_env: ?*ModuleEnv = null; + if (has_platform) { + platform_main_env = compileModuleToSharedMemory( + allocs, + platform_main_path, + "main.roc", + shm_allocator, + &builtin_modules, + &.{}, + ) catch null; + } + // Module count = 1 (app) + number of platform modules const total_module_count: u32 = 1 + @as(u32, @intCast(exposed_modules.items.len)); header_ptr.module_count = total_module_count; @@ -1421,6 +1426,7 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons module_filename, shm_allocator, &builtin_modules, + &.{}, ); // Add exposed item aliases with "pf." prefix for import resolution @@ -1571,6 +1577,11 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons builtin_modules.builtin_indices, ); + for (platform_env_ptrs) |mod_env| { + const name = try app_env.insertIdent(base.Ident.for_text(mod_env.module_name)); + try app_module_envs_map.put(name, .{ .env = mod_env }); + } + // Add platform modules to the module envs map for canonicalization // Two keys are needed for each platform module: // 1. "pf.Stdout" - used during import validation (import pf.Stdout) @@ -1726,6 +1737,7 @@ fn compileModuleToSharedMemory( module_name_arg: []const u8, shm_allocator: std.mem.Allocator, builtin_modules: *eval.BuiltinModules, + additional_modules: []const *ModuleEnv, ) !*ModuleEnv { // Read file const file = try std.fs.cwd().openFile(file_path, .{}); @@ -1762,6 +1774,11 @@ fn compileModuleToSharedMemory( builtin_modules.builtin_indices, ); + for (additional_modules) |mod_env| { + const name = try env.insertIdent(base.Ident.for_text(mod_env.module_name)); + try module_envs_map.put(name, .{ .env = mod_env }); + } + // Canonicalize (without root_is_platform - we'll run HostedCompiler separately) var canonicalizer = try Can.init(&env, &parse_ast, &module_envs_map); defer canonicalizer.deinit(); diff --git a/src/cli/test/fx_platform_test.zig b/src/cli/test/fx_platform_test.zig index d8356d831d..b45ddb53f4 100644 --- a/src/cli/test/fx_platform_test.zig +++ b/src/cli/test/fx_platform_test.zig @@ -300,8 +300,8 @@ test "fx platform match returning string" { } // The app should run successfully and exit with code 0 - // It produces no output since it just returns a string that is discarded - try testing.expectEqualStrings("", run_result.stdout); + // It outputs "0" from the match expression + try testing.expectEqualStrings("0\n", run_result.stdout); try testing.expectEqualStrings("", run_result.stderr); } diff --git a/src/eval/interpreter.zig b/src/eval/interpreter.zig index 12892a0990..8b114604d4 100644 --- a/src/eval/interpreter.zig +++ b/src/eval/interpreter.zig @@ -234,6 +234,26 @@ pub const Interpreter = struct { var next_id: u32 = 1; // Start at 1, reserve 0 for current module + var imported_modules = std.StringHashMap(*const can.ModuleEnv).init(allocator); + errdefer imported_modules.deinit(); + + if (other_envs.len > 0) { + // Populate imported_modules with platform modules and builtin module + // This allows dynamic lookup by name, which is needed for cross-module calls + // when imports are processed in different orders across modules + for (other_envs) |module_env| { + const module_name = module_env.module_name; + // Add full name "Stdout.roc" + try imported_modules.put(module_name, module_env); + + // Add name without extension if present "Stdout" + if (std.mem.endsWith(u8, module_name, ".roc")) { + const short_name = module_name[0 .. module_name.len - 4]; + try imported_modules.put(short_name, module_env); + } + } + } + // Safely access import count const import_count = if (env.imports.imports.items.items.len > 0) env.imports.imports.items.items.len @@ -320,7 +340,7 @@ pub const Interpreter = struct { } } - return initWithModuleEnvs(allocator, env, module_envs, module_ids, import_envs, next_id, builtin_types, builtin_module_env); + return initWithModuleEnvs(allocator, env, module_envs, module_ids, import_envs, imported_modules, next_id, builtin_types, builtin_module_env); } /// Deinit the interpreter and also free the module maps if they were allocated by init() @@ -334,6 +354,7 @@ pub const Interpreter = struct { module_envs: std.AutoHashMapUnmanaged(base_pkg.Ident.Idx, *const can.ModuleEnv), module_ids: std.AutoHashMapUnmanaged(base_pkg.Ident.Idx, u32), import_envs: std.AutoHashMapUnmanaged(can.CIR.Import.Idx, *const can.ModuleEnv), + imported_modules: std.StringHashMap(*const can.ModuleEnv), next_module_id: u32, builtin_types: BuiltinTypes, builtin_module_env: ?*const can.ModuleEnv, @@ -368,7 +389,7 @@ pub const Interpreter = struct { .canonical_bool_rt_var = null, .scratch_tags = try std.array_list.Managed(types.Tag).initCapacity(allocator, 8), .builtins = builtin_types, - .imported_modules = std.StringHashMap(*const can.ModuleEnv).init(allocator), + .imported_modules = imported_modules, .def_stack = try std.array_list.Managed(DefInProgress).initCapacity(allocator, 4), .num_literal_target_type = null, .last_error_message = null, @@ -416,7 +437,8 @@ pub const Interpreter = struct { defer func_val.decref(&self.runtime_layout_store, roc_ops); if (func_val.layout.tag != .closure) { - return error.NotImplemented; + self.triggerCrash("DEBUG: evaluateExpression func_val not closure", false, roc_ops); + return error.Crash; } const header: *const layout.Closure = @ptrCast(@alignCast(func_val.ptr.?)); @@ -691,7 +713,10 @@ pub const Interpreter = struct { }, .s_reassign => |r| { const patt = self.env.store.getPattern(r.pattern_idx); - if (patt != .assign) return error.NotImplemented; + if (patt != .assign) { + self.triggerCrash("DEBUG: s_reassign pattern not assign", false, roc_ops); + return error.Crash; + } const new_val = try self.evalExprMinimal(r.expr, roc_ops, null); // Search through all bindings, not just current block scope // This allows reassigning variables from outer scopes (e.g., in for loops) @@ -828,7 +853,10 @@ pub const Interpreter = struct { // While loop completes and returns {} (implicitly) }, - else => return error.NotImplemented, + else => { + self.triggerCrash("DEBUG: stmt not implemented", false, roc_ops); + return error.Crash; + }, } } @@ -1382,7 +1410,10 @@ pub const Interpreter = struct { break :blk try self.translateTypeVar(self.env, ct_var); }; const resolved = self.runtime_types.resolveVar(rt_var); - if (resolved.desc.content != .structure or resolved.desc.content.structure != .tag_union) return error.NotImplemented; + if (resolved.desc.content != .structure or resolved.desc.content.structure != .tag_union) { + self.triggerCrash("DEBUG: e_zero_argument_tag not tag union", false, roc_ops); + return error.Crash; + } const tu = resolved.desc.content.structure.tag_union; const tags = self.runtime_types.getTagsSlice(tu.tags); // Find index by name @@ -1415,19 +1446,26 @@ pub const Interpreter = struct { out.is_initialized = true; return out; } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_zero_argument_tag scalar not int", false, roc_ops); + return error.Crash; } else if (layout_val.tag == .record) { // Record { tag: Discriminant, payload: ZST } var dest = try self.pushRaw(layout_val, 0); var acc = try dest.asRecord(&self.runtime_layout_store); - const tag_idx = acc.findFieldIndex(self.env, "tag") orelse return error.NotImplemented; + const tag_idx = acc.findFieldIndex(self.env, "tag") orelse { + self.triggerCrash("DEBUG: e_zero_argument_tag tag field not found", false, roc_ops); + return error.Crash; + }; const tag_field = try acc.getFieldByIndex(tag_idx); // write tag as int if (tag_field.layout.tag == .scalar and tag_field.layout.data.scalar.tag == .int) { var tmp = tag_field; tmp.is_initialized = false; try tmp.setInt(@intCast(tag_index)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: e_zero_argument_tag tag field not int", false, roc_ops); + return error.Crash; + } return dest; } else if (layout_val.tag == .tuple) { // Tuple (payload, tag) - tag unions are now represented as tuples @@ -1440,10 +1478,14 @@ pub const Interpreter = struct { var tmp = tag_field; tmp.is_initialized = false; try tmp.setInt(@intCast(tag_index)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: e_zero_argument_tag tuple tag not int", false, roc_ops); + return error.Crash; + } return dest; } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_zero_argument_tag layout not implemented", false, roc_ops); + return error.Crash; }, .e_tag => |tag| { // Construct a tag union value with payloads @@ -1453,7 +1495,10 @@ pub const Interpreter = struct { }; // Unwrap nominal types and aliases to get the base tag union const resolved = self.resolveBaseVar(rt_var); - if (resolved.desc.content != .structure or resolved.desc.content.structure != .tag_union) return error.NotImplemented; + if (resolved.desc.content != .structure or resolved.desc.content.structure != .tag_union) { + self.triggerCrash("DEBUG: e_tag not tag union", false, roc_ops); + return error.Crash; + } const name_text = self.env.getIdent(tag.name); var tag_list = std.array_list.AlignedManaged(types.Tag, null).init(self.allocator); defer tag_list.deinit(); @@ -1487,20 +1532,30 @@ pub const Interpreter = struct { out.is_initialized = true; return out; } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_tag scalar not int", false, roc_ops); + return error.Crash; } else if (layout_val.tag == .record) { // Has payload: record { tag, payload } var dest = try self.pushRaw(layout_val, 0); var acc = try dest.asRecord(&self.runtime_layout_store); - const tag_field_idx = acc.findFieldIndex(self.env, "tag") orelse return error.NotImplemented; - const payload_field_idx = acc.findFieldIndex(self.env, "payload") orelse return error.NotImplemented; + const tag_field_idx = acc.findFieldIndex(self.env, "tag") orelse { + self.triggerCrash("DEBUG: e_tag tag field not found", false, roc_ops); + return error.Crash; + }; + const payload_field_idx = acc.findFieldIndex(self.env, "payload") orelse { + self.triggerCrash("DEBUG: e_tag payload field not found", false, roc_ops); + return error.Crash; + }; // write tag discriminant const tag_field = try acc.getFieldByIndex(tag_field_idx); if (tag_field.layout.tag == .scalar and tag_field.layout.data.scalar.tag == .int) { var tmp = tag_field; tmp.is_initialized = false; try tmp.setInt(@intCast(tag_index)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: e_tag tag field not int", false, roc_ops); + return error.Crash; + } const args_exprs = self.env.store.sliceExpr(tag.args); const arg_vars_range = tag_list.items[tag_index].args; @@ -1575,7 +1630,10 @@ pub const Interpreter = struct { var tmp = tag_field; tmp.is_initialized = false; try tmp.setInt(@intCast(tag_index)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: e_tag (nullable) tag field not int", false, roc_ops); + return error.Crash; + } const args_exprs = self.env.store.sliceExpr(tag.args); const arg_vars_range = tag_list.items[tag_index].args; @@ -1707,7 +1765,8 @@ pub const Interpreter = struct { return dest; } } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_tag layout not implemented", false, roc_ops); + return error.Crash; }, .e_match => |m| { // Evaluate scrutinee once and protect from stack corruption @@ -1808,7 +1867,32 @@ pub const Interpreter = struct { }; const closure_layout = try self.getRuntimeLayout(rt_var); // Expect a closure layout from type-to-layout translation - if (closure_layout.tag != .closure) return error.NotImplemented; + if (closure_layout.tag != .closure) { + // Debug: print what type we got instead + const resolved = self.runtime_types.resolveVar(rt_var); + const ct_var_debug = can.ModuleEnv.varFrom(expr_idx); + const ct_resolved = self.env.types.resolveVar(ct_var_debug); + // Build a message with the expression index + var msg_buf: [256]u8 = undefined; + const expr_idx_int = @intFromEnum(expr_idx); + const types_len = self.env.types.len(); + const msg = switch (ct_resolved.desc.content) { + .flex => std.fmt.bufPrint(&msg_buf, "e_lambda: type is FLEX, expr_idx={}, types.len={}", .{ expr_idx_int, types_len }) catch "e_lambda: FLEX", + .rigid => std.fmt.bufPrint(&msg_buf, "e_lambda: type is RIGID, expr_idx={}, types.len={}", .{ expr_idx_int, types_len }) catch "e_lambda: RIGID", + .structure => |s| switch (s) { + .fn_pure => "e_lambda: type is fn_pure (should work!)", + .fn_effectful => "e_lambda: type is fn_effectful (should work!)", + .fn_unbound => "e_lambda: type is fn_unbound", + else => std.fmt.bufPrint(&msg_buf, "e_lambda: type is structure, expr_idx={}, types.len={}", .{ expr_idx_int, types_len }) catch "e_lambda: structure", + }, + .alias => "e_lambda: type is alias", + .recursion_var => "e_lambda: type is recursion_var", + .err => std.fmt.bufPrint(&msg_buf, "e_lambda: type is ERROR, expr_idx={}, types.len={}", .{ expr_idx_int, types_len }) catch "e_lambda: ERROR", + }; + _ = resolved; + self.triggerCrash(msg, false, roc_ops); + return error.Crash; + } const value = try self.pushRaw(closure_layout, 0); self.registerDefValue(expr_idx, value); // Initialize the closure header @@ -1891,7 +1975,10 @@ pub const Interpreter = struct { .e_closure => |cls| { // Build a closure value with concrete captures. The closure references a lambda. const lam_expr = self.env.store.getExpr(cls.lambda_idx); - if (lam_expr != .e_lambda) return error.NotImplemented; + if (lam_expr != .e_lambda) { + self.triggerCrash("DEBUG: e_closure expr not lambda", false, roc_ops); + return error.Crash; + } const lam = lam_expr.e_lambda; // Collect capture layouts and names from current bindings @@ -2006,8 +2093,12 @@ pub const Interpreter = struct { var accessor = try rec_val.asRecord(&self.runtime_layout_store); for (caps) |cap_idx2| { const cap2 = self.env.store.getCapture(cap_idx2); - const cap_val2 = resolveCapture(self, cap2, roc_ops) orelse return error.NotImplemented; - const idx_opt = accessor.findFieldIndex(self.env, self.env.getIdent(cap2.name)) orelse return error.NotImplemented; + const cap_val2 = resolveCapture(self, cap2, roc_ops) orelse { + return error.NotImplemented; + }; + const idx_opt = accessor.findFieldIndex(self.env, self.env.getIdent(cap2.name)) orelse { + return error.NotImplemented; + }; try accessor.setFieldByIndex(idx_opt, cap_val2, roc_ops); } } @@ -2262,7 +2353,8 @@ pub const Interpreter = struct { return try self.evalExprMinimal(lambda.body, roc_ops, null); } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_call NotImplemented", false, roc_ops); + return error.Crash; }, .e_dot_access => |dot_access| { const receiver_ct_var = can.ModuleEnv.varFrom(dot_access.receiver); @@ -2584,12 +2676,55 @@ pub const Interpreter = struct { } } - return error.NotImplemented; + self.triggerCrash("DEBUG: e_lookup_local not found", false, roc_ops); + return error.Crash; }, .e_lookup_external => |lookup| { // Cross-module reference - look up in imported module - const other_env = self.import_envs.get(lookup.module_idx) orelse { - return error.NotImplemented; + const other_env = self.import_envs.get(lookup.module_idx) orelse blk: { + // Fallback: dynamic lookup by name + // This is needed when the current module (self.env) has imports in a different order + // than the root module, so the Import.Idx doesn't match what was populated in init(). + // We need to get the module name from the import list using the Import.Idx. + if (self.env.imports.map.count() > @intFromEnum(lookup.module_idx)) { + // Retrieve the interned string index for this import + const import_list = self.env.imports.imports.items.items; + if (@intFromEnum(lookup.module_idx) < import_list.len) { + const str_idx = import_list[@intFromEnum(lookup.module_idx)]; + const import_name = self.env.common.getString(str_idx); + + // Try to find it in imported_modules + // First try exact match + if (self.imported_modules.get(import_name)) |env| { + break :blk env; + } + + // Try stripping .roc if present + if (std.mem.endsWith(u8, import_name, ".roc")) { + const short = import_name[0 .. import_name.len - 4]; + if (self.imported_modules.get(short)) |env| { + break :blk env; + } + } + + // Try extracting module name from "pf.Module" + if (std.mem.lastIndexOf(u8, import_name, ".")) |dot_idx| { + const short = import_name[dot_idx + 1 ..]; + if (self.imported_modules.get(short)) |env| { + break :blk env; + } + } + + self.triggerCrash("DEBUG: Failed to resolve import in imported_modules", false, roc_ops); + return error.Crash; + } else { + self.triggerCrash("DEBUG: lookup.module_idx >= import_list.len", false, roc_ops); + return error.Crash; + } + } else { + self.triggerCrash("DEBUG: lookup.module_idx >= map.count", false, roc_ops); + return error.Crash; + } }; // The target_node_idx is a Def.Idx in the other module @@ -2619,7 +2754,10 @@ pub const Interpreter = struct { }, // no if handling in minimal evaluator // no second e_binop case; handled above - else => return error.NotImplemented, + else => { + self.triggerCrash("DEBUG: evalExprMinimal catch-all NotImplemented", false, roc_ops); + return error.Crash; + }, } } @@ -3491,8 +3629,12 @@ pub const Interpreter = struct { // Record { tag, payload } var dest = try self.pushRaw(result_layout, 0); var acc = try dest.asRecord(&self.runtime_layout_store); - const tag_field_idx = acc.findFieldIndex(self.env, "tag") orelse return error.NotImplemented; - const payload_field_idx = acc.findFieldIndex(self.env, "payload") orelse return error.NotImplemented; + const tag_field_idx = acc.findFieldIndex(self.env, "tag") orelse { + return error.NotImplemented; + }; + const payload_field_idx = acc.findFieldIndex(self.env, "payload") orelse { + return error.NotImplemented; + }; // Write tag discriminant const tag_field = try acc.getFieldByIndex(tag_field_idx); @@ -3501,7 +3643,10 @@ pub const Interpreter = struct { tmp.is_initialized = false; const tag_idx: usize = if (in_range) ok_index orelse 0 else err_index orelse 1; try tmp.setInt(@intCast(tag_idx)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: callLowLevelBuiltin tag not int (1)", false, roc_ops); + return error.Crash; + } // Clear payload area const payload_field = try acc.getFieldByIndex(payload_field_idx); @@ -3817,8 +3962,12 @@ pub const Interpreter = struct { var dest = try self.pushRaw(result_layout, 0); var result_acc = try dest.asRecord(&self.runtime_layout_store); // Use layout_env for field lookups since record fields use layout store's env idents - const tag_field_idx = result_acc.findFieldIndex(layout_env, "tag") orelse return error.NotImplemented; - const payload_field_idx = result_acc.findFieldIndex(layout_env, "payload") orelse return error.NotImplemented; + const tag_field_idx = result_acc.findFieldIndex(layout_env, "tag") orelse { + return error.NotImplemented; + }; + const payload_field_idx = result_acc.findFieldIndex(layout_env, "payload") orelse { + return error.NotImplemented; + }; // Write tag discriminant const tag_field = try result_acc.getFieldByIndex(tag_field_idx); @@ -3827,7 +3976,10 @@ pub const Interpreter = struct { tmp.is_initialized = false; const tag_idx: usize = if (in_range) ok_index orelse 0 else err_index orelse 1; try tmp.setInt(@intCast(tag_idx)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: callLowLevelBuiltin tag not int (2)", false, roc_ops); + return error.Crash; + } // Clear payload area const payload_field = try result_acc.getFieldByIndex(payload_field_idx); @@ -4062,7 +4214,10 @@ pub const Interpreter = struct { tmp.is_initialized = false; const tag_idx: usize = if (in_range) ok_index orelse 0 else err_index orelse 1; try tmp.setInt(@intCast(tag_idx)); - } else return error.NotImplemented; + } else { + self.triggerCrash("DEBUG: callLowLevelBuiltin tag not int (3)", false, roc_ops); + return error.Crash; + } // Clear payload area (element 0) const payload_field = try result_acc.getElement(0); @@ -4442,7 +4597,9 @@ pub const Interpreter = struct { if (rhs_dec.num == 0) return error.DivisionByZero; break :blk RocDec{ .num = @rem(lhs_dec.num, rhs_dec.num) }; }, - else => return error.NotImplemented, + else => { + return error.NotImplemented; + }, }; var out = try self.pushRaw(result_layout, 0); @@ -4482,7 +4639,9 @@ pub const Interpreter = struct { if (rhs_float == 0) return error.DivisionByZero; break :blk @rem(lhs_float, rhs_float); }, - else => return error.NotImplemented, + else => { + return error.NotImplemented; + }, }; var out = try self.pushRaw(result_layout, 0); @@ -4696,14 +4855,18 @@ pub const Interpreter = struct { const rhs_str: *const RocStr = @ptrCast(@alignCast(rhs.ptr.?)); return std.mem.eql(u8, lhs_str.asSlice(), rhs_str.asSlice()); }, - else => return error.NotImplemented, + else => { + return error.NotImplemented; + }, } } // Ensure runtime vars resolve to the same descriptor before structural comparison. const lhs_resolved = self.resolveBaseVar(lhs_var); const lhs_content = lhs_resolved.desc.content; - if (lhs_content != .structure) return error.NotImplemented; + if (lhs_content != .structure) { + return error.NotImplemented; + } return switch (lhs_content.structure) { .tuple => |tuple| { @@ -4722,7 +4885,9 @@ pub const Interpreter = struct { // For nominal types, dispatch to their is_eq method return try self.dispatchNominalIsEq(lhs, rhs, nom, lhs_var); }, - .record_unbound, .fn_pure, .fn_effectful, .fn_unbound => error.NotImplemented, + .record_unbound, .fn_pure, .fn_effectful, .fn_unbound => { + return error.NotImplemented; + }, }; } @@ -4929,13 +5094,16 @@ pub const Interpreter = struct { // For other cases, fall back to attempting scalar comparison // This handles cases like Bool which wraps a tag union but is represented as a scalar if (lhs.layout.tag == .scalar and rhs.layout.tag == .scalar) { - const order = self.compareNumericScalars(lhs, rhs) catch return error.NotImplemented; + const order = self.compareNumericScalars(lhs, rhs) catch { + return error.NotImplemented; + }; return order == .eq; } // Can't compare - likely a user-defined nominal type that needs is_eq dispatch // TODO: Implement proper method dispatch by looking up is_eq in the nominal type's module _ = lhs_var; + return error.NotImplemented; } @@ -5778,7 +5946,8 @@ pub const Interpreter = struct { const result = self.valuesStructurallyEqual(lhs, lhs_rt_var, rhs, rhs_rt_var) catch |err| { // If structural equality is not implemented for this type, return false if (err == error.NotImplemented) { - return try self.makeBoolValue(false); + self.triggerCrash("DEBUG: dispatchBinaryOpMethod NotImplemented", false, roc_ops); + return error.Crash; } return err; }; diff --git a/test/fx/app.roc b/test/fx/app.roc index 983620cd91..844154279a 100644 --- a/test/fx/app.roc +++ b/test/fx/app.roc @@ -3,10 +3,13 @@ app [main!] { pf: platform "./platform/main.roc" } import pf.Stdout import pf.Stderr +str : Str -> Str +str = |s| s + main! = || { - Stdout.line!("Hello from stdout!") - Stdout.line!("Line 1 to stdout") - Stderr.line!("Line 2 to stderr") - Stdout.line!("Line 3 to stdout") - Stderr.line!("Error from stderr!") + Stdout.line!(str("Hello from stdout!")) + Stdout.line!(str("Line 1 to stdout")) + Stderr.line!(str("Line 2 to stderr")) + Stdout.line!(str("Line 3 to stdout")) + Stderr.line!(str("Error from stderr!")) } diff --git a/test/fx/match_str_return.roc b/test/fx/match_str_return.roc index 4b2cebb2f0..3c59695676 100644 --- a/test/fx/match_str_return.roc +++ b/test/fx/match_str_return.roc @@ -1,7 +1,13 @@ app [main!] { pf: platform "./platform/main.roc" } +import pf.Stdout + +str : Str -> Str +str = |s| s + main! = || { - match 0 { - _ => "0" + x = match 0 { + _ => str("0") } + Stdout.line!(x) } diff --git a/test/fx/stdin_echo.roc b/test/fx/stdin_echo.roc index f013dfd45a..2320e2a2d7 100644 --- a/test/fx/stdin_echo.roc +++ b/test/fx/stdin_echo.roc @@ -3,7 +3,10 @@ app [main!] { pf: platform "./platform/main.roc" } import pf.Stdin import pf.Stdout +str : Str -> Str +str = |s| s + main! = || { line = Stdin.line!() - Stdout.line!(line) + Stdout.line!(str(line)) } diff --git a/test/fx/stdin_simple.roc b/test/fx/stdin_simple.roc index 1730df95b4..1f7ac92fc5 100644 --- a/test/fx/stdin_simple.roc +++ b/test/fx/stdin_simple.roc @@ -3,7 +3,10 @@ app [main!] { pf: platform "./platform/main.roc" } import pf.Stdin import pf.Stderr +str : Str -> Str +str = |s| s + main! = || { line = Stdin.line!() - Stderr.line!(line) + Stderr.line!(str(line)) } diff --git a/test/fx/stdin_test.roc b/test/fx/stdin_test.roc index 2a6520919b..41d7208cc1 100644 --- a/test/fx/stdin_test.roc +++ b/test/fx/stdin_test.roc @@ -3,8 +3,11 @@ app [main!] { pf: platform "./platform/main.roc" } import pf.Stdout import pf.Stdin +str : Str -> Str +str = |s| s + main! = || { - Stdout.line!("Before stdin") + Stdout.line!(str("Before stdin")) Stdin.line!() - Stdout.line!("After stdin") + Stdout.line!(str("After stdin")) } diff --git a/test/fx/stdin_to_stdout.roc b/test/fx/stdin_to_stdout.roc index bfb3a94c9d..2320e2a2d7 100644 --- a/test/fx/stdin_to_stdout.roc +++ b/test/fx/stdin_to_stdout.roc @@ -3,4 +3,10 @@ app [main!] { pf: platform "./platform/main.roc" } import pf.Stdin import pf.Stdout -main! = || Stdout.line!(Stdin.line!()) +str : Str -> Str +str = |s| s + +main! = || { + line = Stdin.line!() + Stdout.line!(str(line)) +} diff --git a/test/fx/test_direct_string.roc b/test/fx/test_direct_string.roc new file mode 100644 index 0000000000..7cdf822334 --- /dev/null +++ b/test/fx/test_direct_string.roc @@ -0,0 +1,7 @@ +app [main!] { pf: platform "./platform/main.roc" } + +import pf.Stdout + +main! = || { + Stdout.line!("Hello") +} diff --git a/test/fx/test_one_call.roc b/test/fx/test_one_call.roc new file mode 100644 index 0000000000..03a53ddbb2 --- /dev/null +++ b/test/fx/test_one_call.roc @@ -0,0 +1,11 @@ +app [main!] { pf: platform "./platform/main.roc" } + +import pf.Stdout + +identity : a -> a +identity = |x| x + +main! = || { + str = identity("Hello") + Stdout.line!(str) +} diff --git a/test/fx/test_with_wrapper.roc b/test/fx/test_with_wrapper.roc new file mode 100644 index 0000000000..5e3f699ac7 --- /dev/null +++ b/test/fx/test_with_wrapper.roc @@ -0,0 +1,10 @@ +app [main!] { pf: platform "./platform/main.roc" } + +import pf.Stdout + +str : Str -> Str +str = |s| s + +main! = || { + Stdout.line!(str("Hello")) +} diff --git a/test/serialization_size_check.zig b/test/serialization_size_check.zig index 4c12caa099..cece7842cc 100644 --- a/test/serialization_size_check.zig +++ b/test/serialization_size_check.zig @@ -31,7 +31,7 @@ const expected_safelist_u8_size = 24; const expected_safelist_u32_size = 24; const expected_safemultilist_teststruct_size = 24; const expected_safemultilist_node_size = 24; -const expected_moduleenv_size = 712; // Platform-independent size +const expected_moduleenv_size = 736; // Platform-independent size const expected_nodestore_size = 96; // Platform-independent size // Compile-time assertions - build will fail if sizes don't match expected values From acbd606301a8ec7abe5cfe63c3954298785f03f0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 25 Nov 2025 20:42:53 -0500 Subject: [PATCH 6/6] Remove unnecessary .md file --- test/fx/match_str_return_analysis.md | 217 --------------------------- 1 file changed, 217 deletions(-) delete mode 100644 test/fx/match_str_return_analysis.md diff --git a/test/fx/match_str_return_analysis.md b/test/fx/match_str_return_analysis.md deleted file mode 100644 index 9515940ea7..0000000000 --- a/test/fx/match_str_return_analysis.md +++ /dev/null @@ -1,217 +0,0 @@ -# Analysis: Match Expression String Return Crash - -## Summary - -`roc check` succeeds but `roc run` crashes with "incorrect alignment" for the following code: - -```roc -app [main!] { pf: platform "./platform/main.roc" } - -main! = || { - match 0 { - _ => "0" - } -} -``` - -## Root Cause - -The crash occurs because the `ret_ptr` (return pointer) passed from the host to the Roc entrypoint is not properly aligned for `RocStr`, which requires 8-byte alignment on 64-bit systems. - -## Detailed Analysis - -### Pipeline Differences - -**`roc check`:** -- Parses, canonicalizes, and type-checks the code -- Does NOT evaluate or execute the code at runtime -- The `copyToPtr` function is never called with an actual pointer - -**`roc run`:** -1. Compiles modules to shared memory (`setupSharedMemoryWithModuleEnv`) -2. Links host library with interpreter shim to create executable -3. Launches the executable which reads from shared memory -4. Calls `roc_entrypoint` with host-provided `ret_ptr` -5. Interpreter evaluates the expression and calls `copyToPtr` to write result - -### Where the Crash Happens - -**File:** `src/eval/StackValue.zig:62-64` - -```zig -pub fn copyToPtr(self: StackValue, layout_cache: *LayoutStore, dest_ptr: *anyopaque, ops: *RocOps) !void { - // ... - if (self.layout.tag == .scalar) { - switch (self.layout.data.scalar.tag) { - .str => { - const src_str: *const RocStr = @ptrCast(@alignCast(self.ptr.?)); - const dest_str: *RocStr = @ptrCast(@alignCast(dest_ptr)); // <-- CRASH HERE - dest_str.* = src_str.clone(ops); - return; - }, - // ... - } - } - // ... -} -``` - -The `@alignCast(dest_ptr)` requires the pointer to be aligned to `RocStr`'s alignment (8 bytes on 64-bit). If `dest_ptr` is misaligned, Zig's runtime safety check triggers a panic. - -### The Call Chain - -1. **Host calls Roc:** `test/fx/platform/host.zig:201` - ```zig - var ret: [0]u8 = undefined; - roc__main_for_host(&roc_ops, @ptrCast(&ret), @ptrCast(&args)); - ``` - -2. **Roc entrypoint:** `src/interpreter_shim/main.zig:62` - ```zig - export fn roc_entrypoint(entry_idx: u32, ops: *RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) - ``` - -3. **Interpreter evaluates:** `src/eval/interpreter.zig:492` - ```zig - try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); - ``` - -4. **copyToPtr crashes** when casting `ret_ptr` to `*RocStr` - -### Why `main!` Returns a String - -The `main!` function in this test returns a `Str` (from the match expression), but the platform defines: - -```roc -main! : List(Str) => {} -``` - -The platform expects `main!` to return `{}` (unit), but the actual Roc code returns `"0"` (a string). However, `roc check` passes because: -- The match expression type-checks (it returns `Str`) -- The effectful block discards its result value - -But at runtime, the interpreter still evaluates the match expression and tries to copy the string result, even though the return type should be `{}`. - -## The Bug - -There appear to be two issues: - -### Issue 1: Host Provides Wrong Return Buffer - -The host in `test/fx/platform/host.zig:194` provides: -```zig -var ret: [0]u8 = undefined; // Zero-sized buffer for {} return type -``` - -But the interpreter tries to write a `RocStr` (24 bytes) to this location. - -### Issue 2: Discarded Expression Values Still Copied - -The interpreter evaluates the match expression and attempts to copy its result to `ret_ptr`, even when the result should be discarded (because `main!` returns `{}`). - -## Potential Fixes - -### Fix Option 1: Interpreter Should Not Copy Discarded Results - -In `evaluateExpression`, check if the return type is unit `{}` and skip the `copyToPtr` call: - -```zig -// If return type is unit (zero-sized), don't copy anything -const result_size = layout_cache.layoutSize(result.layout); -if (result_size > 0) { - try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); -} -``` - -### Fix Option 2: Ensure ret_ptr Alignment - -The host should always provide an aligned return buffer, or the interpreter should check alignment before casting: - -```zig -.str => { - const alignment = @alignOf(RocStr); - if (@intFromPtr(dest_ptr) % alignment != 0) { - return error.MisalignedPointer; - } - // ... proceed with cast -}, -``` - -### Fix Option 3: Handle Unit Return Type at Entry Point - -The interpreter shim should detect when `main!` returns `{}` and not attempt to write any result: - -```zig -// In evaluateExpression, check layout and skip copy for ZSTs -if (layout_cache.layoutSize(result.layout) == 0) { - return; // Nothing to copy for zero-sized types -} -try result.copyToPtr(...); -``` - -## Implemented Fix - -The fix was implemented in `src/eval/interpreter.zig` in the `evaluateExpression` function. - -We added a `shouldCopyResult` helper function that checks whether the result should be copied to `ret_ptr` based on: -1. Zero-sized types (size == 0) never need copying -2. If `ret_ptr` is not properly aligned for the result type, skip the copy - -```zig -/// Check if we should copy the result to ret_ptr based on the result's layout. -/// Returns false if the result shouldn't be copied (e.g., when the evaluated result -/// type doesn't match what the caller expects based on pointer alignment). -fn shouldCopyResult(self: *Interpreter, result: StackValue, ret_ptr: *anyopaque) bool { - const result_size = self.runtime_layout_store.layoutSize(result.layout); - if (result_size == 0) { - // Zero-sized types don't need copying - return false; - } - - // Check if ret_ptr is properly aligned for the result type. - // This handles the case where the platform declares a function returning {} - // but the Roc code evaluates to a different type (e.g., Str). - // When the platform expects {}, the host provides a zero-sized or misaligned buffer. - // We detect this by checking alignment: if ret_ptr isn't aligned for the result type, - // the caller doesn't expect this type and we should skip the copy. - const required_alignment = result.layout.alignment(self.runtime_layout_store.targetUsize()); - const ret_addr = @intFromPtr(ret_ptr); - if (ret_addr % required_alignment.toByteUnits() != 0) { - // Destination not properly aligned for result type - skip copy - return false; - } - - return true; -} -``` - -The `evaluateExpression` function now calls this helper before copying: - -```zig -// Only copy result if the result type is compatible with ret_ptr -if (self.shouldCopyResult(result, ret_ptr)) { - try result.copyToPtr(&self.runtime_layout_store, ret_ptr, roc_ops); -} -``` - -This fix works because: -1. When the host expects `{}`, it provides a pointer to a zero-sized buffer which may not be aligned -2. When we try to copy the result (e.g., `Str`), we first check if `ret_ptr` is aligned for the result type -3. If misaligned, we skip the copy since the caller doesn't expect this result type anyway -4. This avoids the Zig runtime "incorrect alignment" panic in `copyToPtr` - -### Why Not Use Type Annotations? - -We initially tried to use the function's type annotation to determine the expected return type, but this approach failed because: -1. Type unification merges declared and inferred types, so querying the pattern's type after type-checking returns the unified type (e.g., `Str`) not the declared type (`{}`) -2. The `def.annotation` field is `null` for definitions where the type annotation is on a separate line in the Roc source - -The alignment-based approach is robust because it relies on the host's behavior: when expecting `{}`, the host allocates a zero-sized or arbitrarily-aligned buffer. - -## Test Reproduction - -The test case is now in: -- **Roc file:** `test/fx/match_str_return.roc` -- **Test:** `src/cli/test/fx_platform_test.zig` - "fx platform match returning string" - -Run with: `zig build test` and look for the fx_platform_test failure.