`createComptimeFunction` wraps a comptime expression into a fresh fn that the interp executes in isolation. The wrapper must not inherit the enclosing call's lowering state — any leaked slot, binding, or scope flag corrupts the wrapper's own lowering. Pre-fix, only `func` / `current_block` / `inst_counter` / `scope` / `current_ctx_ref` were saved. Specifically NOT saved: - `inline_return_target` — set by `lowerComptimeCall` for an outer comptime body with `return X;`. The wrapper's body was lowering through this slot, routing the wrapper's `ret` into a basic block from a different function. - `pack_arg_nodes`, `pack_param_count`, `pack_arg_types` — active during a pack-fn mono's body lowering. (Pack-fn face of 0046 was already fixed by step 2b moving pack-fn calls off the inline path; these saves close a latent cross-contamination if any future pack-mono body invokes the comptime interp.) - `comptime_param_nodes` — active during an outer `lowerComptimeCall` to bind `$fmt`-style substitutions. - `block_terminated`, `target_type`, `func_defer_base` — fn- local flags that the wrapper's lowering needs fresh. All eight now save/restore in `createComptimeFunction`. The wrapper runs in a clean state. `examples/issue-0046.sx` flips from the non-deterministic interp panic to "inside\n" + "n=42\n". 204/204 example tests + `zig build test` green. Issue file marked FIXED with a pointer to the regression test.
160 lines
6.2 KiB
Markdown
160 lines
6.2 KiB
Markdown
**FIXED.** `createComptimeFunction` now saves/restores the
|
|
outer `lowerComptimeCall`'s state — specifically
|
|
`inline_return_target`, `pack_arg_nodes`, `pack_param_count`,
|
|
`pack_arg_types`, `comptime_param_nodes`, `block_terminated`,
|
|
`target_type`, and `func_defer_base` — so the wrapper fn it
|
|
builds for the nested comptime expression runs in isolation.
|
|
Without the saves, the wrapper inherited an inline-return slot
|
|
belonging to a different basic block; the interp executed it
|
|
and tripped a null pointer store at `storeAtRawPtr`.
|
|
|
|
The pack-fn face of this bug (filed as face 2) was fixed
|
|
incidentally by step 2b's mono refactor — pack-fn calls
|
|
bypass the inline-return-slot setup entirely. Plain
|
|
`($x: s32)` comptime fns stay on the inline path; the
|
|
`createComptimeFunction` save/restore fix covers that path.
|
|
|
|
Regression test:
|
|
[examples/issue-0046.sx](../examples/issue-0046.sx).
|
|
|
|
# Symptom
|
|
|
|
A comptime fn body containing BOTH a nested comptime call
|
|
(e.g. `print(...)`) AND a `return X;` statement fails in one of
|
|
two shapes depending on the comptime-param flavour:
|
|
|
|
| Outer fn shape | Failure |
|
|
|---|---|
|
|
| `helper :: ($x: s32) -> s64 { print("inside\n"); return 42; }` (plain comptime) | Panic: `cast causes pointer to be null` at `src/ir/interp.zig:207 storeAtRawPtr`. |
|
|
| `dump :: (..$args) -> s64 { n := args[0]; print("got {}\n", n); return n; }` (pack-fn) | Compile error: `unresolved 'result'` at fake span `1:5` (inside the inserted code). |
|
|
|
|
Both vanish if you remove either the nested `print(...)` OR the
|
|
`return X;` statement:
|
|
- Arrow-form bodies (`=> expr`) work.
|
|
- Bodies with `return` but no nested comptime call work.
|
|
- Bodies with a nested comptime call but no `return` work.
|
|
|
|
Both faces share one root: my fix for issue-0045 (commit
|
|
`9e78790`) added an `inline_return_target` slot + alloca in
|
|
`lowerComptimeCall`, which is now active when the outer call's
|
|
body recursively invokes another comptime fn that itself runs
|
|
the `#insert build_format(fmt)` → interpreter → parse-and-lower
|
|
pipeline.
|
|
|
|
Pre-fix this pattern crashed too — at the LLVM verifier with
|
|
"Terminator found in the middle of a basic block" — because the
|
|
outer `return` emitted `ret X` into the caller's basic block
|
|
mid-flight. My fix routed `return` into the slot so the outer
|
|
body now fully completes, which means the recursive comptime
|
|
call runs to completion too. The interpreter / #insert scope
|
|
chain then has to be correct in this newly-reachable context,
|
|
and it isn't.
|
|
|
|
# Reproduction
|
|
|
|
```sx
|
|
#import "modules/std.sx";
|
|
|
|
// Face 1 — interp panic:
|
|
helper :: ($x: s32) -> s64 {
|
|
print("inside\n");
|
|
return 42;
|
|
}
|
|
|
|
// Face 2 — "unresolved 'result'":
|
|
dump :: (..$args) -> s64 {
|
|
n : s64 = args[0];
|
|
print("got {}\n", n);
|
|
return n;
|
|
}
|
|
|
|
main :: () -> s32 {
|
|
n := helper(7); // ← panic in interp
|
|
print("{}\n", dump(7)); // ← "unresolved 'result'"
|
|
return 0;
|
|
}
|
|
```
|
|
|
|
Each face reproduces independently — they don't need to coexist
|
|
in the same program.
|
|
|
|
# What's NOT happening
|
|
|
|
- Not a regression introduced by issue-0045's fix per se: the
|
|
same pattern hit a different fatal stage (LLVM verifier) before
|
|
the fix. The fix exposed it; it didn't create it.
|
|
- Not caused by step 2a (pack typed indexing, commit `cd36784`):
|
|
Face 1 reproduces with a plain `($x: s32)` comptime fn, no
|
|
pack involved.
|
|
- Not exercised by any test in the suite today. `format`/`print`
|
|
use arrow form or `#insert`-only bodies — no `return` in a
|
|
block. User code historically followed the same pattern.
|
|
|
|
# Why this didn't block step 2a
|
|
|
|
Step 2a only tests `args[$i]` in arrow-form pack bodies and
|
|
arithmetic chains. No nested comptime call in any test body.
|
|
Step 2b (per-mono mangling) and step 3 (type-reflection
|
|
intrinsics, `$args[$i]` in type positions) don't inherently
|
|
require nested comptime calls either — builder fns run inside
|
|
`#insert` contexts, not inside the public pack-fn body, so
|
|
they have a different lowering path.
|
|
|
|
The pattern WILL bite when:
|
|
- Step 5 of the pack plan refactors stdlib's `print`/`format` to
|
|
use `..$args` — print's body itself becomes the outer comptime
|
|
fn that nests comptime calls.
|
|
- User code writes a pack-fn that wants both `print` for debug
|
|
output AND `return X;` for early exit.
|
|
|
|
# Investigation prompt
|
|
|
|
For a fresh session picking this up:
|
|
|
|
The interaction is between (a) my issue-0045 `inline_return_target`
|
|
slot + alloca setup in `lowerComptimeCall` and (b) the recursive
|
|
comptime path that invokes `#insert build_format(fmt)` →
|
|
`evalComptimeString` → `createComptimeFunction` → `interp.call`
|
|
on a wrapper fn, then parses the returned source string and
|
|
lowers each parsed statement into the current scope.
|
|
|
|
Three angles worth probing:
|
|
|
|
1. **Saved/restored state in `createComptimeFunction`** at
|
|
`src/ir/lower.zig:8851+`. It saves `builder.func`,
|
|
`builder.current_block`, `builder.inst_counter`, `self.scope`,
|
|
`current_ctx_ref`. It does NOT save/restore
|
|
`inline_return_target`, `pack_arg_nodes`, `comptime_param_nodes`.
|
|
The first two were added by my recent commits (`9e78790`,
|
|
`cd36784`). One of these leaking into the wrapper-fn lowering
|
|
is the most likely cause of Face 1.
|
|
|
|
2. **Ref numbering** — the alloca I added for `ret_slot` shifts
|
|
subsequent Ref values in the outer fn (main). The interp
|
|
shouldn't see those refs (it executes the wrapper fn's IR,
|
|
not main's), but check whether the wrapper fn carries a stale
|
|
Ref handle from the outer build context.
|
|
|
|
3. **Scope chain visible to parsed `#insert` statements**. For
|
|
Face 2 the inserted code declares `result := ""` then references
|
|
`result` in the next stmt. The lookup fails. Maybe the
|
|
`lowerBlockValue` exit defer fires the parent scope deinit
|
|
before the next stmt lowers — or `block_terminated` from the
|
|
inline-return slot setup is interfering with the inserted-stmt
|
|
loop in `lowerInsertExprValue`
|
|
(`src/ir/lower.zig:7065+`).
|
|
|
|
A reasonable starting place: add the missing save/restore for
|
|
`inline_return_target` (and `pack_arg_nodes`) in
|
|
`createComptimeFunction`, then re-run both repros. If Face 1
|
|
disappears, that confirms angle 1.
|
|
|
|
# Verification
|
|
|
|
```sh
|
|
./zig-out/bin/sx run /tmp/issue-0046-face1.sx # expect "n=42"
|
|
./zig-out/bin/sx run /tmp/issue-0046-face2.sx # expect "got 7\n7"
|
|
```
|
|
|
|
Full suite + zig test must still pass after the fix.
|