lab/sx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ffi M5.A.next.2a.D: inline-return uses CFG terminator, not block_terminated

Browse Source 
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.
This commit is contained in:
agra
2026-05-27 14:55:25 +03:00
parent 6b7a66ba4d
commit e6d6903708
2 changed files with 54 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
src/ir/lower.zig
View File

@@ -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;

2
tests/expected/157-pack-if-return.txt
View File

@@ -1,2 +1,2 @@
8354116000
-1
42