From 9e78790ebf9d13e0c6210b7250f82ed2a8aa70e6 Mon Sep 17 00:00:00 2001 From: agra Date: Wed, 27 May 2026 13:21:23 +0300 Subject: [PATCH] ffi issue-0045 fix: inline-return slot for comptime-call bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lowerComptimeCall` now scans the body for `return` statements via `fnBodyHasReturn`. When found, it allocates a stack slot typed to the fn's return type and installs it as `self.inline_return_target` before lowering the body. `lowerReturn` checks `inline_return_target` first: - If set, it stores the coerced return value into the slot, drains pending defers, sets `block_terminated = true`, and returns without emitting a `ret` into the caller's basic block. - Otherwise it emits the standard `ret` as before. After the body lowers, the inliner either returns the tail-expression value (existing fast path — bodies with no `return` skip the slot entirely) or loads the slot when `block_terminated` is set. Why the bug was invisible until now: `format`/`print` and every other stdlib comptime fn use arrow form (`=> expr`) or `#insert`-only bodies — no `return` statement, no path through `lowerReturn`. Step 1.b of the pack feature made `..$args` parseable; the natural smoke test `foo :: (..$args) -> s64 { return 42; }` was the first comptime-fn body to take the `return`-with-trailing-statements path, surfacing the LLVM verifier crash. `examples/issue-0045.sx` flips from the lock-in failure to `42`. 194/194 example tests + `zig build test` green. --- ...0045-pack-fn-call-llvm-verifier-failure.md | 6 ++ src/ir/lower.zig | 98 ++++++++++++++++++- tests/expected/issue-0045.exit | 2 +- tests/expected/issue-0045.txt | 4 +- 4 files changed, 102 insertions(+), 8 deletions(-) diff --git a/issues/0045-pack-fn-call-llvm-verifier-failure.md b/issues/0045-pack-fn-call-llvm-verifier-failure.md index 6bb920f..7aad6f9 100644 --- a/issues/0045-pack-fn-call-llvm-verifier-failure.md +++ b/issues/0045-pack-fn-call-llvm-verifier-failure.md @@ -1,3 +1,9 @@ +**FIXED.** `lowerComptimeCall` now allocates a result slot when +the body contains a `return` statement and reroutes +`lowerReturn` to store into it instead of emitting `ret` into the +caller's basic block. Regression test: +[examples/issue-0045.sx](../examples/issue-0045.sx). + # Symptom Calling a fn declared with `..$args` (variadic heterogeneous type diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 854af42..cb3f207 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -146,6 +146,15 @@ pub const Lowering = struct { /// `resolveTypeWithBindings` on closure_type_expr to substitute /// `Closure(..$args) -> $R` into a concrete closure type. pack_bindings: ?std.StringHashMap([]const TypeId) = null, + /// Active when lowering an inlined comptime-call body. `return X;` + /// inside the body must NOT emit a `ret` into the caller's LLVM + /// function — instead it stores X into `.slot` (typed `.ret_ty`) + /// and sets `block_terminated` so the inliner can load the slot + /// once the body finishes. Without this, a body like + /// `{ return 42; }` truncates the caller's basic block mid-flight + /// and trips LLVM's "Terminator found in the middle of a basic + /// block" verifier. + inline_return_target: ?InlineReturnInfo = null, struct_const_map: std.StringHashMap(StructConstInfo) = std.StringHashMap(StructConstInfo).init(std.heap.page_allocator), // "Struct.CONST" → value info module_const_map: std.StringHashMap(ModuleConstInfo) = std.StringHashMap(ModuleConstInfo).init(std.heap.page_allocator), // module-level value constants (e.g. AF_INET :s32: 2) foreign_name_map: std.StringHashMap([]const u8) = std.StringHashMap([]const u8).init(std.heap.page_allocator), // sx name → C name for #foreign renames @@ -198,6 +207,8 @@ pub const Lowering = struct { span: ast.Span, }; + const InlineReturnInfo = struct { slot: Ref, ret_ty: TypeId }; + /// Pack-variadic impl entry — `impl Proto(Args...) for Closure(Prefix..., ..$pack) -> $ret`. /// Matches any concrete closure source whose first `prefix_len` param types /// equal `source_pack_ty`'s fixed prefix; the tail binds to `pack_var_name` @@ -1553,9 +1564,14 @@ pub const Lowering = struct { } fn lowerReturn(self: *Lowering, rs: *const ast.ReturnStmt) void { - // Set target_type to function return type so null_literal etc. get the right type + // Set target_type to function return type so null_literal etc. get the right type. + // When inlining a comptime body, the *inlined* fn's declared return type wins + // over the caller's — otherwise `return 42` inside a `-> s64` body lowered into + // a `-> s32` caller would coerce 42 to s32 before storing into the s64 slot. const old_target = self.target_type; - const ret_ty_for_target = if (self.builder.func) |fid| + const ret_ty_for_target: TypeId = if (self.inline_return_target) |iri| + iri.ret_ty + else if (self.builder.func) |fid| self.module.functions.items[@intFromEnum(fid)].ret else TypeId.s64; @@ -1564,6 +1580,25 @@ pub const Lowering = struct { const ret_val = if (rs.value) |val| self.lowerExpr(val) else null; self.target_type = old_target; + // Inlined-comptime-body return: store into the slot the inliner + // gave us instead of emitting a `ret` into the caller's LLVM + // function. The inliner reads the slot once the body finishes. + if (self.inline_return_target) |iri| { + if (ret_val) |ref| { + const val_ty = self.builder.getRefType(ref); + const coerced = if (val_ty != iri.ret_ty) + self.coerceToType(ref, val_ty, iri.ret_ty) + else + ref; + self.builder.store(iri.slot, coerced); + } + // Drain block-scoped defers up to the inlined-body base so + // they fire on this return path the same as a real fn return. + self.emitBlockDefers(self.func_defer_base); + self.block_terminated = true; + return; + } + // Emit ALL pending defers for THIS function in LIFO order before the return self.emitBlockDefers(self.func_defer_base); @@ -7195,8 +7230,34 @@ pub const Lowering = struct { // Lower the body — capture return value for functions with return type const ret_ty = self.resolveReturnType(fd); if (ret_ty != .void) { - if (self.lowerBlockValue(fd.body)) |val| { - return val; + // Detect whether the body might use `return X;` statements. + // If so, set up the inline-return slot so they store into it + // instead of emitting `ret` into the caller's basic block. + // Pure tail-expression bodies (arrow form, or a block whose + // last stmt is an expression) skip the slot — keeps the + // common `format`/`#insert`-style path unchanged. + const has_return = fnBodyHasReturn(fd.body); + if (has_return) { + const ret_slot = self.builder.alloca(ret_ty); + const saved_iri = self.inline_return_target; + self.inline_return_target = .{ .slot = ret_slot, .ret_ty = ret_ty }; + const saved_term = self.block_terminated; + self.block_terminated = false; + defer { + self.inline_return_target = saved_iri; + self.block_terminated = saved_term; + } + + if (self.lowerBlockValue(fd.body)) |val| { + return val; + } + if (self.block_terminated) { + return self.builder.load(ret_slot, ret_ty); + } + } else { + if (self.lowerBlockValue(fd.body)) |val| { + return val; + } } } else { self.lowerBlock(fd.body); @@ -7205,6 +7266,35 @@ pub const Lowering = struct { return self.builder.constInt(0, .void); } + /// True if `node` (a fn body) contains any top-level `return` statement. + /// Used by inline-comptime lowering to decide whether to allocate a + /// result slot — pure tail-expression bodies skip the slot. Walks past + /// `if`/`while`/`for`/`match` arms (early-return inside a conditional + /// counts) but stops at nested fn/lambda bodies (those have their own + /// return contexts). + fn fnBodyHasReturn(node: *const Node) bool { + return switch (node.data) { + .return_stmt => true, + .block => |b| blk: { + for (b.stmts) |s| if (fnBodyHasReturn(s)) break :blk true; + break :blk false; + }, + .if_expr => |ie| blk: { + if (fnBodyHasReturn(ie.then_branch)) break :blk true; + if (ie.else_branch) |eb| if (fnBodyHasReturn(eb)) break :blk true; + break :blk false; + }, + .while_expr => |we| fnBodyHasReturn(we.body), + .for_expr => |fe| fnBodyHasReturn(fe.body), + .match_expr => |me| blk: { + for (me.arms) |arm| if (fnBodyHasReturn(arm.body)) break :blk true; + break :blk false; + }, + .defer_stmt => |ds| fnBodyHasReturn(ds.expr), + else => false, + }; + } + /// Pack variadic arguments into a []Any slice. Each arg is boxed as Any {tag, value}, /// stored into a stack-allocated array, and the slice {ptr, len} is bound to param_name. fn lowerVariadicArgs(self: *Lowering, param_name: []const u8, call_args: []const *const Node, start_idx: usize) void { diff --git a/tests/expected/issue-0045.exit b/tests/expected/issue-0045.exit index d00491f..573541a 100644 --- a/tests/expected/issue-0045.exit +++ b/tests/expected/issue-0045.exit @@ -1 +1 @@ -1 +0 diff --git a/tests/expected/issue-0045.txt b/tests/expected/issue-0045.txt index 2556f1e..d81cc07 100644 --- a/tests/expected/issue-0045.txt +++ b/tests/expected/issue-0045.txt @@ -1,3 +1 @@ -LLVM verification failed: Terminator found in the middle of a basic block! -label %entry - +42