fix(lower): null-FuncId path restores full caller state [0100 F2]
lazyLowerFunction's three exit paths (non-null branch, already-promoted
early return, null-FuncId `ns.fn` qualified-alias branch) each duplicated
the caller-state restore, and the null branch's copy had drifted: it
restored every saved field EXCEPT `block_terminated`. A qualified alias
whose body terminates (e.g. a constant-folded `if true { return ... }`)
leaves `block_terminated = true` after lowerFunction; the null path
returned without resetting it, so the flag leaked into the CALLER's body
lowering and the caller's own trailing statements / `return` were rejected
as dead-after-terminator ("function ... body produces no value").
Fix: collapse the three restores into a single `defer` registered right
after the state is saved, so every exit path restores the identical full
set and the class cannot diverge again. Fields restored on all paths:
current_source_file (F1), scope, func_defer_base, block_terminated (F2),
force_block_value, builder.func/current_block/inst_counter. The
foreign-class / jni-env / pack-mono / inline-return fields already had
their own defers and are unchanged.
Regression: examples/0721-modules-qualified-terminating-callee.sx — a
qualified alias `m.foo` folds `if true { return helper(); }` (helper from
m.sx's own import) and is followed by caller statements + the caller's own
`return 0`. Reports "body produces no value" pre-fix; prints
"terminating-callee: ok" / "after" and exits 0 after. 0719 (collision) and
0720 (F1 own-import visibility) stay green. issues/0100 RESOLVED banner
extended with the F2 follow-up.
This commit is contained in:
31
examples/0721-modules-qualified-terminating-callee.sx
Normal file
31
examples/0721-modules-qualified-terminating-callee.sx
Normal file
@@ -0,0 +1,31 @@
|
||||
// Regression (issue 0100 F2): lowering a QUALIFIED imported function whose
|
||||
// body terminates must leave the CALLER's lowering state untouched.
|
||||
//
|
||||
// `m :: #import …` registers `m.foo` as a module-qualified alias with a unique
|
||||
// FuncId (the identity fix, issue 0100 / example 0719) and lowers it through
|
||||
// `lazyLowerFunction`'s null-FuncId `lowerFunction` path. `foo`'s body folds
|
||||
// `if true { return helper(); }` to an unconditional return, so its lowering
|
||||
// ends with `block_terminated = true`. The null-FuncId path used to restore
|
||||
// every saved caller field EXCEPT `block_terminated`, so that flag leaked back
|
||||
// into `main`, and `main`'s own trailing `print` / `return 0` were treated as
|
||||
// dead-after-terminator — the compiler rejected `return 0` with "body produces
|
||||
// no value". The fix routes all exit paths through one save/restore defer, so
|
||||
// the qualified alias is transparent to the caller. (`helper` also lives in
|
||||
// m.sx's own flat import, exercising the F1 source-context restore too.)
|
||||
|
||||
#import "modules/std.sx";
|
||||
m :: #import "0721-modules-qualified-terminating-callee/m.sx";
|
||||
|
||||
report :: (label: string, ok: bool) {
|
||||
if ok { print("{}: ok\n", label); } else { print("{}: FAIL\n", label); }
|
||||
}
|
||||
|
||||
main :: () -> s32 {
|
||||
// Qualified callee whose body terminates via a constant-folded `if true`.
|
||||
x := m.foo();
|
||||
// Caller statements AFTER the call must still be emitted (not dead).
|
||||
report("terminating-callee", x == 7);
|
||||
print("after\n");
|
||||
// The caller's OWN return — rejected pre-fix because block_terminated leaked.
|
||||
return 0;
|
||||
}
|
||||
@@ -0,0 +1,3 @@
|
||||
// Lives in m.sx's OWN flat import — reachable from `foo` but not from the
|
||||
// top-level consumer that imports m.sx under a qualified namespace.
|
||||
helper :: () -> s64 { return 7; }
|
||||
10
examples/0721-modules-qualified-terminating-callee/m.sx
Normal file
10
examples/0721-modules-qualified-terminating-callee/m.sx
Normal file
@@ -0,0 +1,10 @@
|
||||
// `foo` is pulled in QUALIFIED by the consumer (`m :: #import …`). Its body
|
||||
// terminates via a constant-folded `if true { return … }`, and the `return`
|
||||
// calls `helper` from m.sx's OWN flat import. Lowering `foo` as a qualified
|
||||
// alias must be transparent to the caller's lowering state — issue 0100 F2.
|
||||
#import "helper.sx";
|
||||
|
||||
foo :: () -> s64 {
|
||||
if true { return helper(); }
|
||||
return 0;
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
terminating-callee: ok
|
||||
after
|
||||
@@ -69,6 +69,53 @@ Regression: `examples/0720-modules-qualified-own-import.sx` — `calc.compute`
|
||||
Reports `'triple' is not visible` on the attempt-1 code; passes after. 0719's
|
||||
cross-module dual-`parse` assertion stays green.
|
||||
|
||||
## F2 follow-up — null-FuncId path must restore the FULL caller lowering state
|
||||
|
||||
The F1 fix patched the **source file** in `lazyLowerFunction`'s null-FuncId
|
||||
branch, but that branch still restored only a SUBSET of the caller state the
|
||||
non-null branch restores — it omitted `self.block_terminated`. A qualified
|
||||
alias whose body terminates (e.g. a constant-folded `if true { return … }`)
|
||||
leaves `block_terminated = true` after `lowerFunction`; the null branch then
|
||||
returned without resetting it, so the flag leaked into the **caller's** body
|
||||
lowering and the caller's own trailing statements / `return` were treated as
|
||||
dead-after-terminator:
|
||||
|
||||
```
|
||||
m :: #import "m.sx"; // m.sx: `#import "helper.sx"; foo :: () -> s64 { if true { return helper(); } return 0; }`
|
||||
main :: () -> s32 {
|
||||
x := m.foo();
|
||||
print("after\n"); // dropped
|
||||
return 0; // → error: body produces no value
|
||||
}
|
||||
```
|
||||
|
||||
**Fix** (`src/ir/lower.zig`): the three exit paths of `lazyLowerFunction` (the
|
||||
null-FuncId branch, the already-promoted early return, and the bottom of the
|
||||
non-null branch) duplicated the restore, and the null branch's copy drifted.
|
||||
They are now collapsed into a **single `defer`** registered right after the
|
||||
state is saved, so every exit path restores the identical full set and the
|
||||
class can't diverge again. The fields the defer now restores on all paths:
|
||||
|
||||
- `current_source_file` (via `setCurrentSourceFile`, which also resyncs
|
||||
`diagnostics.current_source_file`) — F1
|
||||
- `scope`
|
||||
- `func_defer_base`
|
||||
- `block_terminated` — **F2** (was missing on the null path)
|
||||
- `force_block_value`
|
||||
- `builder.func`
|
||||
- `builder.current_block`
|
||||
- `builder.inst_counter`
|
||||
|
||||
(The `current_foreign_class`, `jni_env_stack_base`, and pack-mono /
|
||||
`inline_return_target` fields already had their own `defer`s and apply on all
|
||||
paths; they are unchanged.)
|
||||
|
||||
Regression: `examples/0721-modules-qualified-terminating-callee.sx` — `m.foo`
|
||||
(a qualified alias) folds `if true { return helper(); }` and is followed by
|
||||
caller statements + the caller's own `return 0`. Reports `body produces no
|
||||
value` on the attempt-2 code; prints `terminating-callee: ok` / `after` and
|
||||
exits 0 after. 0719 and 0720 stay green.
|
||||
|
||||
## Symptom
|
||||
|
||||
- **Observed:** a program that imports two modules each exporting a
|
||||
@@ -117,11 +164,15 @@ machinery already existed but was never fed module-qualified entries.
|
||||
## Fix verification
|
||||
|
||||
- `zig build` → 0
|
||||
- `zig build test` → 0 (incl. LSP corpus sweep, 472 examples)
|
||||
- `bash tests/run_examples.sh` → 455 passed, 0 failed
|
||||
- `zig build test` → 0 (incl. LSP corpus sweep, 473 examples; 397/397 tests)
|
||||
- `bash tests/run_examples.sh` → 456 passed, 0 failed
|
||||
- `examples/0719-modules-cli-and-json.sx`: panics pre-fix, passes post-fix.
|
||||
- `examples/0720-modules-qualified-own-import.sx`: `'… is not visible'` on
|
||||
the attempt-1 code, passes after the F1 fix.
|
||||
- `examples/0721-modules-qualified-terminating-callee.sx`: `body produces no
|
||||
value` on the attempt-2 code, passes after the F2 fix.
|
||||
|
||||
Regression tests: `examples/0719-modules-cli-and-json.sx` (collision),
|
||||
`examples/0720-modules-qualified-own-import.sx` (F1 own-import visibility).
|
||||
`examples/0720-modules-qualified-own-import.sx` (F1 own-import visibility),
|
||||
`examples/0721-modules-qualified-terminating-callee.sx` (F2 terminating
|
||||
qualified callee — caller state transparency).
|
||||
|
||||
@@ -1590,6 +1590,25 @@ pub const Lowering = struct {
|
||||
const saved_block_terminated = self.block_terminated;
|
||||
const saved_force_block_value = self.force_block_value;
|
||||
const saved_source_file = self.current_source_file;
|
||||
// Lowering a callee must be transparent to the caller's lowering
|
||||
// state: restore the FULL saved context on EVERY exit path through one
|
||||
// defer so the three exits (non-null branch, already-promoted early
|
||||
// return, null-FuncId `ns.fn` alias branch) cannot drift. Notably
|
||||
// `block_terminated` — a qualified alias whose body terminates (e.g. a
|
||||
// constant-folded `if true { return … }`) leaves it true, and leaking
|
||||
// that into the caller marks the caller's own trailing statements
|
||||
// dead-after-terminator (issue 0100 F2). The jni/pack/foreign-class
|
||||
// fields keep their own defers above.
|
||||
defer {
|
||||
self.setCurrentSourceFile(saved_source_file);
|
||||
self.scope = saved_scope;
|
||||
self.func_defer_base = saved_defer_base;
|
||||
self.block_terminated = saved_block_terminated;
|
||||
self.force_block_value = saved_force_block_value;
|
||||
self.builder.func = saved_func;
|
||||
self.builder.current_block = saved_block;
|
||||
self.builder.inst_counter = saved_counter;
|
||||
}
|
||||
// The `#jni_env` Ref stack is lexical within ONE function's instruction
|
||||
// stream — Refs from the caller don't dereference correctly in this
|
||||
// callee's body. Move the visible base to the current top so
|
||||
@@ -1650,15 +1669,7 @@ pub const Lowering = struct {
|
||||
self.setCurrentSourceFile(src);
|
||||
}
|
||||
self.lowerFunction(fd, name, false);
|
||||
// Restore builder state
|
||||
self.setCurrentSourceFile(saved_source_file);
|
||||
self.scope = saved_scope;
|
||||
self.func_defer_base = saved_defer_base;
|
||||
self.force_block_value = saved_force_block_value;
|
||||
self.builder.func = saved_func;
|
||||
self.builder.current_block = saved_block;
|
||||
self.builder.inst_counter = saved_counter;
|
||||
return;
|
||||
return; // caller state restored by the top-level defer
|
||||
}
|
||||
|
||||
if (func_id) |fid| {
|
||||
@@ -1667,15 +1678,8 @@ pub const Lowering = struct {
|
||||
const func = &self.module.functions.items[@intFromEnum(fid)];
|
||||
self.setCurrentSourceFile(func.source_file);
|
||||
if (!func.is_extern) {
|
||||
// Already promoted (e.g., via lowerComptimeDeps) — skip
|
||||
self.setCurrentSourceFile(saved_source_file);
|
||||
self.scope = saved_scope;
|
||||
self.func_defer_base = saved_defer_base;
|
||||
self.block_terminated = saved_block_terminated;
|
||||
self.force_block_value = saved_force_block_value;
|
||||
self.builder.func = saved_func;
|
||||
self.builder.current_block = saved_block;
|
||||
self.builder.inst_counter = saved_counter;
|
||||
// Already promoted (e.g., via lowerComptimeDeps) — skip.
|
||||
// Caller state restored by the top-level defer.
|
||||
return;
|
||||
}
|
||||
func.is_extern = false; // promote from extern stub to real function
|
||||
@@ -1745,16 +1749,7 @@ pub const Lowering = struct {
|
||||
|
||||
self.builder.finalize();
|
||||
}
|
||||
|
||||
// Restore builder state
|
||||
self.setCurrentSourceFile(saved_source_file);
|
||||
self.scope = saved_scope;
|
||||
self.func_defer_base = saved_defer_base;
|
||||
self.block_terminated = saved_block_terminated;
|
||||
self.force_block_value = saved_force_block_value;
|
||||
self.builder.func = saved_func;
|
||||
self.builder.current_block = saved_block;
|
||||
self.builder.inst_counter = saved_counter;
|
||||
// Caller state restored by the top-level defer.
|
||||
}
|
||||
|
||||
/// Lower a single function declaration.
|
||||
|
||||
Reference in New Issue
Block a user