Merge pull request #8492 from roc-lang/platform-fixes

Some fixes for platforms

Author: rtfeldman

src/canonicalize/Can.zig

@@ -1753,6 +1753,10 @@ pub fn canonicalizeFile(
         .platform => |h| {
             self.env.module_kind = .platform;
             try self.createExposedScope(h.exposes);
+            // Also add the 'provides' items (what platform provides to the host, e.g., main_for_host!)
+            // These need to be in the exposed scope so they become exports
+            // Platform provides uses curly braces { main_for_host! } so it's parsed as record fields
+            try self.addPlatformProvidesItems(h.provides);
             // Extract required type signatures for type checking
             // This stores the types in env.requires_types without creating local definitions
             // Pass requires_rigids so R1, R2, etc. are in scope when processing signatures
@@ -2531,6 +2535,17 @@ fn createExposedScope(
     self.exposed_scope.deinit(gpa);
     self.exposed_scope = Scope.init(false);
 
+    try self.addToExposedScope(exposes);
+}
+
+/// Add items to the exposed scope without resetting it.
+/// Used for platforms which have both 'exposes' (for apps) and 'provides' (for the host).
+fn addToExposedScope(
+    self: *Self,
+    exposes: AST.Collection.Idx,
+) std.mem.Allocator.Error!void {
+    const gpa = self.env.gpa;
+
     const collection = self.parse_ir.store.getCollection(exposes);
     const exposed_items = self.parse_ir.store.exposedItemSlice(.{ .span = collection.span });
 
@@ -2654,6 +2669,42 @@ fn createExposedScope(
     }
 }
 
+/// Add platform provides items to the exposed scope.
+/// Platform provides uses curly braces { main_for_host!: "main" } so it's parsed as record fields.
+/// The string value is the FFI symbol name exported to the host (becomes roc__<symbol>).
+fn addPlatformProvidesItems(
+    self: *Self,
+    provides: AST.Collection.Idx,
+) std.mem.Allocator.Error!void {
+    const gpa = self.env.gpa;
+
+    const collection = self.parse_ir.store.getCollection(provides);
+    const record_fields = self.parse_ir.store.recordFieldSlice(.{ .span = collection.span });
+
+    for (record_fields) |field_idx| {
+        const field = self.parse_ir.store.getRecordField(field_idx);
+
+        // Get the identifier text from the field name token
+        if (self.parse_ir.tokens.resolveIdentifier(field.name)) |ident_idx| {
+            // Add to exposed_items for permanent storage
+            try self.env.addExposedById(ident_idx);
+
+            // Add to exposed_scope so it becomes an export
+            const dummy_idx = @as(Pattern.Idx, @enumFromInt(0));
+            try self.exposed_scope.put(gpa, .ident, ident_idx, dummy_idx);
+
+            // Also track in exposed_ident_texts
+            const token_region = self.parse_ir.tokens.resolve(@intCast(field.name));
+            const ident_text = self.parse_ir.env.source[token_region.start.offset..token_region.end.offset];
+            const region = self.parse_ir.tokenizedRegionToRegion(field.region);
+            _ = try self.exposed_ident_texts.getOrPut(gpa, ident_text);
+            if (self.exposed_ident_texts.getPtr(ident_text)) |ptr| {
+                ptr.* = region;
+            }
+        }
+    }
+}
+
 /// Process the requires_signatures from a platform header.
 ///
 /// This extracts the required type signatures (like `main! : () => {}`) from the platform
@@ -8238,7 +8289,6 @@ fn canonicalizeBlock(self: *Self, e: AST.Block) std.mem.Allocator.Error!Canonica
             // canonicalize the expr directly without adding it as a statement
             switch (ast_stmt) {
                 .expr => |expr_stmt| {
-                    //
                     last_expr = try self.canonicalizeExprOrMalformed(expr_stmt.expr);
                 },
                 .dbg => |dbg_stmt| {

src/canonicalize/ModuleEnv.zig

@@ -440,7 +440,7 @@ pub fn init(gpa: std.mem.Allocator, source: []const u8) std.mem.Allocator.Error!
         .external_decls = try CIR.ExternalDecl.SafeList.initCapacity(gpa, 16),
         .imports = CIR.Import.Store.init(),
         .module_name = undefined, // Will be set later during canonicalization
-        .module_name_idx = undefined, // Will be set later during canonicalization
+        .module_name_idx = Ident.Idx.NONE, // Will be set later during canonicalization
         .diagnostics = CIR.Diagnostic.Span{ .span = base.DataSpan{ .start = 0, .len = 0 } },
         .store = try NodeStore.initCapacity(gpa, 10_000), // Default node store capacity
         .evaluation_order = null, // Will be set after canonicalization completes
@@ -2624,12 +2624,19 @@ pub fn getMethodIdent(self: *const Self, type_name: []const u8, method_name: []c
             const qualified = std.fmt.bufPrint(&buf, "{s}.{s}", .{ type_name, method_name }) catch return null;
             return self.getIdentStoreConst().findByString(qualified);
         } else {
-            // Need to add module prefix
+            // Try module-qualified name first (e.g., "Builtin.Num.U64.from_numeral")
             const qualified = std.fmt.bufPrint(&buf, "{s}.{s}.{s}", .{ self.module_name, type_name, method_name }) catch return null;
-            return self.getIdentStoreConst().findByString(qualified);
+            if (self.getIdentStoreConst().findByString(qualified)) |idx| {
+                return idx;
+            }
+            // Fallback: try without module prefix (e.g., "Color.as_str" for app-defined types)
+            // This handles the case where methods are registered with just the type-qualified name
+            const simple_qualified = std.fmt.bufPrint(&buf, "{s}.{s}", .{ type_name, method_name }) catch return null;
+            return self.getIdentStoreConst().findByString(simple_qualified);
         }
     } else {
         // Use heap allocation for large identifiers (rare case)
+        // Try module-qualified name first
         const qualified = if (type_name.len > self.module_name.len and
             std.mem.startsWith(u8, type_name, self.module_name) and
             type_name[self.module_name.len] == '.')
@@ -2639,7 +2646,19 @@ pub fn getMethodIdent(self: *const Self, type_name: []const u8, method_name: []c
         else
             std.fmt.allocPrint(self.gpa, "{s}.{s}.{s}", .{ self.module_name, type_name, method_name }) catch return null;
         defer self.gpa.free(qualified);
-        return self.getIdentStoreConst().findByString(qualified);
+        if (self.getIdentStoreConst().findByString(qualified)) |idx| {
+            return idx;
+        }
+        // Fallback for the module-qualified case
+        if (type_name.len <= self.module_name.len or
+            !std.mem.startsWith(u8, type_name, self.module_name) or
+            type_name[self.module_name.len] != '.')
+        {
+            const simple_qualified = std.fmt.allocPrint(self.gpa, "{s}.{s}", .{ type_name, method_name }) catch return null;
+            defer self.gpa.free(simple_qualified);
+            return self.getIdentStoreConst().findByString(simple_qualified);
+        }
+        return null;
     }
 }
 

src/check/Check.zig

@@ -1121,15 +1121,10 @@ pub fn checkPlatformRequirements(
             // 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 });
 
-            // 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);
+            // Unify the platform's required type with the app's export type.
+            // This constrains type variables in the export (e.g., closure params)
+            // to match the platform's expected types.
+            _ = try self.unifyFromAnno(instantiated_required_var, export_var, &env);
         }
         // Note: If the export is not found, the canonicalizer should have already reported an error
     }

src/cli/app_stub.zig

@@ -161,12 +161,12 @@ fn addRocCallAbiStub(
     wip.cursor = .{ .block = entry };
 
     // Generate actual implementation based on function name
-    if (std.mem.eql(u8, name, "addInts")) {
+    if (std.mem.eql(u8, name, "add_ints")) {
         try addIntsImplementation(&wip, llvm_builder);
-    } else if (std.mem.eql(u8, name, "multiplyInts")) {
+    } else if (std.mem.eql(u8, name, "multiply_ints")) {
         try multiplyIntsImplementation(&wip, llvm_builder);
-    } else if (std.mem.eql(u8, name, "processString")) {
-        // processString not supported in cross-compilation stubs - only int platform supported
+    } else if (std.mem.eql(u8, name, "process_string")) {
+        // process_string not supported in cross-compilation stubs - only int platform supported
         _ = try wip.retVoid();
     } else {
         // Default: just return void for unknown functions
@@ -180,11 +180,11 @@ fn addRocCallAbiStub(
 pub fn getTestPlatformEntrypoints(allocator: Allocator, platform_type: []const u8) ![]PlatformEntrypoint {
     if (std.mem.eql(u8, platform_type, "int")) {
         // Based on test/int/platform/host.zig:
-        // extern fn roc__addInts(ops: *builtins.host_abi.RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) callconv(.c) void;
-        // extern fn roc__multiplyInts(ops: *builtins.host_abi.RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) callconv(.c) void;
+        // extern fn roc__add_ints(ops: *builtins.host_abi.RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) callconv(.c) void;
+        // extern fn roc__multiply_ints(ops: *builtins.host_abi.RocOps, ret_ptr: *anyopaque, arg_ptr: ?*anyopaque) callconv(.c) void;
         const entrypoints = try allocator.alloc(PlatformEntrypoint, 2);
-        entrypoints[0] = PlatformEntrypoint{ .name = "addInts" };
-        entrypoints[1] = PlatformEntrypoint{ .name = "multiplyInts" };
+        entrypoints[0] = PlatformEntrypoint{ .name = "add_ints" };
+        entrypoints[1] = PlatformEntrypoint{ .name = "multiply_ints" };
         return entrypoints;
     }
 

src/cli/main.zig

@@ -625,7 +625,8 @@ fn generatePlatformHostShim(allocs: *Allocators, cache_dir: []const u8, entrypoi
     }
 
     // Create the complete platform shim
-    platform_host_shim.createInterpreterShim(&llvm_builder, entrypoints.items) catch |err| {
+    // Note: Symbol names include platform-specific prefixes (underscore for macOS)
+    platform_host_shim.createInterpreterShim(&llvm_builder, entrypoints.items, target) catch |err| {
         std.log.err("Failed to create interpreter shim: {}", .{err});
         return err;
     };
@@ -1103,15 +1104,16 @@ fn runWithWindowsHandleInheritance(allocs: *Allocators, exe_path: []const u8, sh
     _ = ipc.platform.windows.CloseHandle(process_info.hProcess);
     _ = ipc.platform.windows.CloseHandle(process_info.hThread);
 
-    // Check exit code
+    // Check exit code and propagate to parent
     if (exit_code != 0) {
-        std.log.err("Child process {s} exited with code: {}", .{ exe_path, exit_code });
+        std.log.debug("Child process {s} exited with code: {}", .{ exe_path, exit_code });
         if (exit_code == 0xC0000005) { // STATUS_ACCESS_VIOLATION
             std.log.err("Child process crashed with access violation (segfault)", .{});
         } else if (exit_code >= 0xC0000000) { // NT status codes for exceptions
             std.log.err("Child process crashed with exception code: 0x{X}", .{exit_code});
         }
-        return error.ProcessExitedWithError;
+        // Propagate the exit code (truncated to u8 for compatibility)
+        std.process.exit(@truncate(exit_code));
     }
 
     std.log.debug("Child process completed successfully", .{});
@@ -1198,9 +1200,9 @@ fn runWithPosixFdInheritance(allocs: *Allocators, exe_path: []const u8, shm_hand
             if (exit_code == 0) {
                 std.log.debug("Child process completed successfully", .{});
             } else {
-                // The host exited with an error - it should have printed any error messages
+                // Propagate the exit code from the child process to our parent
                 std.log.debug("Child process {s} exited with code: {}", .{ temp_exe_path, exit_code });
-                return error.ProcessExitedWithError;
+                std.process.exit(exit_code);
             }
         },
         .Signal => |signal| {
@@ -1212,7 +1214,8 @@ fn runWithPosixFdInheritance(allocs: *Allocators, exe_path: []const u8, shm_hand
             } else if (signal == 9) { // SIGKILL
                 std.log.err("Child process was killed (SIGKILL)", .{});
             }
-            return error.ProcessKilledBySignal;
+            // Standard POSIX convention: exit with 128 + signal number
+            std.process.exit(128 +| @as(u8, @truncate(signal)));
         },
         .Stopped => |signal| {
             std.log.err("Child process {s} stopped by signal: {}", .{ temp_exe_path, signal });
@@ -1358,6 +1361,10 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons
         entry_count: u32,
         def_indices_offset: u64,
         module_envs_offset: u64,
+        /// Offset to platform's main.roc env (0 if no platform, entry points are in app)
+        platform_main_env_offset: u64,
+        /// Offset to app env (always present, used for e_lookup_required resolution)
+        app_env_offset: u64,
     };
 
     const header_ptr = try shm_allocator.create(Header);
@@ -1624,7 +1631,24 @@ pub fn setupSharedMemoryWithModuleEnv(allocs: *Allocators, roc_file_path: []cons
     // Store app env at the last index (N-1, after platform modules at 0..N-2)
     module_env_offsets_ptr[total_module_count - 1] = @intFromPtr(app_env_ptr) - @intFromPtr(shm.base_ptr);
 
-    const exports_slice = app_env.store.sliceDefs(app_env.exports);
+    // Store app env offset for e_lookup_required resolution
+    header_ptr.app_env_offset = @intFromPtr(app_env_ptr) - @intFromPtr(shm.base_ptr);
+
+    // Entry points are defined in the platform's `provides` section.
+    // The platform wraps app-provided functions (from `requires`) and exports them for the host.
+    // For example: `provides { main_for_host!: "main" }` where `main_for_host! = main!`
+    const platform_env = platform_main_env orelse {
+        std.log.err("No platform found. Every Roc app requires a platform.", .{});
+        return error.NoPlatformFound;
+    };
+    const exports_slice = platform_env.store.sliceDefs(platform_env.exports);
+    if (exports_slice.len == 0) {
+        std.log.err("Platform has no exports in `provides` clause.", .{});
+        return error.NoEntrypointFound;
+    }
+
+    // Store platform env offset for entry point lookups
+    header_ptr.platform_main_env_offset = @intFromPtr(platform_env) - @intFromPtr(shm.base_ptr);
     header_ptr.entry_count = @intCast(exports_slice.len);
 
     const def_indices_ptr = try shm_allocator.alloc(u32, exports_slice.len);
@@ -2347,15 +2371,39 @@ fn extractEntrypointsFromPlatform(allocs: *Allocators, roc_file_path: []const u8
             const provides_coll = parse_ast.store.getCollection(platform_header.provides);
             const provides_fields = parse_ast.store.recordFieldSlice(.{ .span = provides_coll.span });
 
-            // Extract all field names as entrypoints
+            // Extract FFI symbol names from provides clause
+            // Format: `provides { roc_identifier: "ffi_symbol_name" }`
+            // The string value specifies the symbol name exported to the host (becomes roc__<symbol>)
             for (provides_fields) |field_idx| {
                 const field = parse_ast.store.getRecordField(field_idx);
-                const field_name = parse_ast.resolve(field.name);
-                // Strip trailing '!' from effectful function names for the exported symbol
-                const symbol_name = if (std.mem.endsWith(u8, field_name, "!"))
-                    field_name[0 .. field_name.len - 1]
-                else
-                    field_name;
+
+                // Require explicit string value for symbol name
+                const symbol_name = if (field.value) |value_idx| blk: {
+                    const value_expr = parse_ast.store.getExpr(value_idx);
+                    switch (value_expr) {
+                        .string => |str_like| {
+                            const parts = parse_ast.store.exprSlice(str_like.parts);
+                            if (parts.len > 0) {
+                                const first_part = parse_ast.store.getExpr(parts[0]);
+                                switch (first_part) {
+                                    .string_part => |sp| break :blk parse_ast.resolve(sp.token),
+                                    else => {},
+                                }
+                            }
+                            std.log.err("Invalid provides entry: string value is empty", .{});
+                            return error.InvalidProvidesEntry;
+                        },
+                        .string_part => |str_part| break :blk parse_ast.resolve(str_part.token),
+                        else => {
+                            std.log.err("Invalid provides entry: expected string value for symbol name", .{});
+                            return error.InvalidProvidesEntry;
+                        },
+                    }
+                } else {
+                    const field_name = parse_ast.resolve(field.name);
+                    std.log.err("Provides entry '{s}' missing symbol name. Use format: {{ {s}: \"symbol_name\" }}", .{ field_name, field_name });
+                    return error.InvalidProvidesEntry;
+                };
                 try entrypoints.append(try allocs.arena.dupe(u8, symbol_name));
             }