From e6d690370855b72390cac0ff32a7bb0118ed5070 Mon Sep 17 00:00:00 2001 From: agra Date: Wed, 27 May 2026 14:55:25 +0300 Subject: [PATCH] ffi M5.A.next.2a.D: inline-return uses CFG terminator, not block_terminated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the regression locked in by 2a.C (commit 6b7a66b). issue-0045's original fix set `block_terminated = true` after each inline `return X;` to skip dead code in the inlined body. But the flag leaked past structured control flow — an `if cond { return X; }` whose merge block continued to subsequent statements would short-circuit the trailing code at the `lowerBlockValue` loop's `if (self.block_terminated) return null;` check. Switched to the classical SSA "return-done block" shape: - `InlineReturnInfo` carries a third field `done_bb: BlockId` — a fresh basic block allocated by `lowerComptimeCall` per comptime-call instance. - `lowerReturn`'s inline path stores into the slot, drains defers, and emits `br done_bb`. The basic block's terminator is what carries the "no fall-through" signal; the `block_terminated` flag is no longer touched. - `lowerComptimeCall` allocates the slot + done_bb, lowers the body, then switches to done_bb and loads the slot. Tail- expression bodies that fall through (rare when has_return is true) get a synthetic store + br so the CFG is well-formed. For `if cond { return 42; }; return -1;`: - cond=true: then's `return 42` stores 42, br done_bb. Merge block has only the false predecessor, doesn't run the trailing return. Load done_bb → 42. - cond=false: condBr skips to merge. Merge runs `return -1;` → store -1, br done_bb. Load → -1. `examples/157-pack-if-return.sx` flips from `8354116000` (the uninitialised slot load on the false path) to `-1`. A three-way `classify(..$args)` smoke confirms multi-path inline-return works for any of the three branches. Dead-code-after-return inside the inlined body still trips the LLVM verifier (same shape as a regular `return X; print("dead");` which also crashes today). Acceptable consistency — user code shouldn't write unreachable code in either context. 196/196 example tests + `zig build test` green. --- src/ir/lower.zig | 74 +++++++++++++++++++-------- tests/expected/157-pack-if-return.txt | 2 +- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/ir/lower.zig b/src/ir/lower.zig index f6cbe27..482ae8a 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -216,7 +216,7 @@ pub const Lowering = struct { span: ast.Span, }; - const InlineReturnInfo = struct { slot: Ref, ret_ty: TypeId }; + const InlineReturnInfo = struct { slot: Ref, ret_ty: TypeId, done_bb: BlockId }; /// Pack-variadic impl entry — `impl Proto(Args...) for Closure(Prefix..., ..$pack) -> $ret`. /// Matches any concrete closure source whose first `prefix_len` param types @@ -1590,8 +1590,17 @@ pub const Lowering = struct { 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. + // gave us and branch to the inliner's "return-done" basic block. + // The branch is the basic block's terminator — so subsequent + // dead code in the same block trips the LLVM verifier (the + // SAME behaviour as a regular `return X;` followed by code). + // + // We DO NOT set `block_terminated = true`: that flag would + // leak past structured control flow (e.g. an `if cond { return + // X; }` whose merge block continues to subsequent statements) + // and incorrectly skip the trailing statements. CFG-level + // termination is what we actually want — let the basic-block + // terminator do its job. if (self.inline_return_target) |iri| { if (ret_val) |ref| { const val_ty = self.builder.getRefType(ref); @@ -1604,7 +1613,7 @@ pub const Lowering = struct { // 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; + self.builder.br(iri.done_bb, &.{}); return; } @@ -7297,29 +7306,52 @@ pub const Lowering = struct { const ret_ty = self.resolveReturnType(fd); if (ret_ty != .void) { // 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. + // If so, set up the inline-return slot AND a dedicated + // "return-done" basic block so each `return X;` stores to + // the slot and branches to ret_done. After the body lowers, + // we switch to ret_done and load. Pure tail-expression + // bodies (arrow form, or a block whose last stmt is an + // expression) skip the slot+block — 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 ret_done_bb = self.freshBlock("ct.ret_done"); 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; + self.inline_return_target = .{ .slot = ret_slot, .ret_ty = ret_ty, .done_bb = ret_done_bb }; + defer self.inline_return_target = saved_iri; + + // Lower body. Tail-expression bodies (rare here since + // has_return == true) produce a tail value we still + // route through the slot so the load in ret_done picks + // it up. Block-statement bodies whose last stmt is + // `return X;` already br to ret_done from inside + // lowerReturn. + if (self.lowerBlockValue(fd.body)) |val| { + if (!self.currentBlockHasTerminator()) { + const v_ty = self.builder.getRefType(val); + const coerced = if (v_ty != ret_ty) + self.coerceToType(val, v_ty, ret_ty) + else + val; + self.builder.store(ret_slot, coerced); + self.builder.br(ret_done_bb, &.{}); + } + } else if (!self.currentBlockHasTerminator()) { + // Body fell through without producing a tail value + // AND without branching to ret_done — this only + // happens for bodies whose last stmt is a void + // statement (e.g. side-effecting). Slot is + // uninitialised on this path; safer to br anyway + // so the CFG is well-formed. The load in ret_done + // will read uninit, which is the same garbage + // behaviour the regular fn-body lowering would + // produce for a missing return. + self.builder.br(ret_done_bb, &.{}); } - if (self.lowerBlockValue(fd.body)) |val| { - return val; - } - if (self.block_terminated) { - return self.builder.load(ret_slot, ret_ty); - } + self.builder.switchToBlock(ret_done_bb); + return self.builder.load(ret_slot, ret_ty); } else { if (self.lowerBlockValue(fd.body)) |val| { return val; diff --git a/tests/expected/157-pack-if-return.txt b/tests/expected/157-pack-if-return.txt index 1e218e9..2b0ed4e 100644 --- a/tests/expected/157-pack-if-return.txt +++ b/tests/expected/157-pack-if-return.txt @@ -1,2 +1,2 @@ -8354116000 +-1 42