fix: initialize the error-channel slot on every failable implicit success return (issue 0190)
A failable function that returned by IMPLICIT success (no explicit
`return`) left its error-tag slot uninitialized, so a caller's `catch` /
`or` (or `main`) read a garbage tag and reported a phantom unhandled
error — and for value-carrying failables the success value was dropped.
The "no error" sentinel was only written on the explicit-`return;` path.
Unified all function-body-return lowering so the failable-success slot
is always written:
- void `-> !` fall-through: `ensureTerminator` (control_flow.zig) now
emits `ret constInt(0)` for a pure-failable end-of-body.
- value-failable trailing-expression success: `lowerValueBody`
(stmt.zig) routes through `lowerFailableSuccessReturn`.
- generic + pack-fn instances: `monomorphizeFunction` (generic.zig) and
`monomorphizePackFn` (pack.zig) now DELEGATE their body-return to
`lowerValueBody` instead of hand-rolling a `coerce`+`ret` that drifted
(covers generic/pack value-failables).
Also fixes the missing-value diagnostic guard added here: it now counts
`.err`-level diagnostics (new `DiagnosticList.errorCount`) rather than the
total list length, so a warning/note emitted while lowering the body
(e.g. an ObjC selector arity warning) can no longer suppress a genuine
"body produces no value" error — which previously shipped an
uninitialized return at exit 0.
Regressions: examples/errors/1061 (void fall-through), 1062 (value-failable
trailing expr), 1063 (generic value-failable trailing expr).
This commit is contained in:
agra
21
examples/errors/1061-errors-void-failable-fallthrough.sx
Normal file
21
examples/errors/1061-errors-void-failable-fallthrough.sx
Normal file
@@ -0,0 +1,21 @@
|
||||
// A pure-failable function (`-> !`) that succeeds by IMPLICIT fall-through —
|
||||
// no explicit `return;` — must initialize its error-channel slot to 0 ("no
|
||||
// error"), exactly like an explicit `return;` would. Otherwise the slot is
|
||||
// left undefined and a caller's `catch` (or `main`) reads a garbage tag and
|
||||
// reports a phantom unhandled error.
|
||||
//
|
||||
// This exercises:
|
||||
// - a `-> !` callee that falls off the end (no `return;`) — its `catch`
|
||||
// handler must NOT fire;
|
||||
// - a `main :: () -> !` that falls off the end — must exit 0.
|
||||
//
|
||||
// Regression (issue 0190).
|
||||
|
||||
#import "modules/std.sx";
|
||||
|
||||
noop :: () -> ! { } // success by fall-through, no `return;`
|
||||
|
||||
main :: () -> ! {
|
||||
noop() catch (e) { print("phantom: {}\n", e); }; // must NOT fire
|
||||
print("ok\n"); // main falls through → exit 0
|
||||
}
|
||||
43
examples/errors/1062-errors-value-failable-trailing-expr.sx
Normal file
43
examples/errors/1062-errors-value-failable-trailing-expr.sx
Normal file
@@ -0,0 +1,43 @@
|
||||
// Value-carrying failable functions (`-> T !E`) whose body ends in a trailing
|
||||
// success EXPRESSION (no explicit `return`) must set the success error slot to
|
||||
// 0 — the caller's `catch` must NOT fire and the success value must be intact.
|
||||
// Regression (issue 0190): `lowerValueBody` used to `coerceToType`+`ret` the
|
||||
// bare success value to the full failable tuple, leaving the error-tag slot
|
||||
// uninitialized → phantom catch on success (and dropped value for string /
|
||||
// multi-value returns).
|
||||
#import "modules/std.sx";
|
||||
|
||||
E :: error { Bad }
|
||||
|
||||
// Single-value trailing-expression success.
|
||||
val :: () -> i64 !E { 99 }
|
||||
|
||||
// String trailing-expression success.
|
||||
sval :: () -> string !E { "hi" }
|
||||
|
||||
// Multi-value (tuple) trailing-expression success.
|
||||
mval :: () -> Tuple(i64, i64) !E { .(1, 2) }
|
||||
|
||||
// A real error still propagates through the value-failable channel.
|
||||
fval :: (n: i64) -> i64 !E {
|
||||
if n < 0 { raise error.Bad; }
|
||||
n + 1
|
||||
}
|
||||
|
||||
main :: () -> i32 {
|
||||
x := val() catch (e) { print("PHANTOM val\n"); return 1; };
|
||||
print("x={}\n", x);
|
||||
|
||||
s := sval() catch (e) { print("PHANTOM sval\n"); return 1; };
|
||||
print("s={}\n", s);
|
||||
|
||||
t := mval() catch (e) { print("PHANTOM mval\n"); return 1; };
|
||||
print("t=({},{})\n", t.0, t.1);
|
||||
|
||||
ok := fval(10) catch (e) { print("PHANTOM fval-ok\n"); return 1; };
|
||||
print("ok={}\n", ok);
|
||||
|
||||
fval(-1) catch (e) { print("real error caught\n"); return 0; };
|
||||
print("UNEXPECTED no error\n");
|
||||
return 1;
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
// Generic value-carrying failable functions (`($T) -> T !E`) whose body ends in
|
||||
// a trailing success EXPRESSION (no explicit `return`) must set the success
|
||||
// error slot to 0 — the caller's `catch` must NOT fire and the value must be
|
||||
// intact across instantiations (i64 / string / struct), and `or` must yield the
|
||||
// real value not the fallback.
|
||||
// Regression (issue 0190): `lowerGenericInstance` hand-rolled a body-return
|
||||
// (coerceToType+ret) that missed the value-failable success routing, leaving
|
||||
// the error-tag slot uninitialized → phantom catch on success / value
|
||||
// corruption for generic instantiations.
|
||||
#import "modules/std.sx";
|
||||
|
||||
E :: error { Bad }
|
||||
|
||||
Point :: struct { x: i64; y: i64; }
|
||||
|
||||
// Generic trailing-expression success — instantiated at i64 / string / struct.
|
||||
gen :: ($T: Type, v: T) -> T !E { v }
|
||||
|
||||
// Generic that RAISES — the caller's catch must still fire.
|
||||
gfail :: ($T: Type, v: T, bad: bool) -> T !E {
|
||||
if bad { raise error.Bad; }
|
||||
v
|
||||
}
|
||||
|
||||
main :: () -> i32 {
|
||||
a := gen(i64, 42) catch (e) { print("PHANTOM i64\n"); return 1; };
|
||||
print("i64: {}\n", a);
|
||||
|
||||
s := gen(string, "hello") catch (e) { print("PHANTOM string\n"); return 1; };
|
||||
print("string: {}\n", s);
|
||||
|
||||
p := gen(Point, Point.{ x = 3, y = 4 }) catch (e) { print("PHANTOM struct\n"); return 1; };
|
||||
print("struct: ({},{})\n", p.x, p.y);
|
||||
|
||||
// `or`-form on success must yield the real value, not the fallback.
|
||||
o := gen(i64, 7) or 999;
|
||||
print("or: {}\n", o);
|
||||
|
||||
// A generic that raises still propagates to the caller's catch.
|
||||
gfail(i64, 0, true) catch (e) {
|
||||
print("raise caught\n");
|
||||
return 0;
|
||||
};
|
||||
print("UNEXPECTED no error\n");
|
||||
return 1;
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
ok
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
x=99
|
||||
s=hi
|
||||
t=(1,2)
|
||||
ok=11
|
||||
real error caught
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
i64: 42
|
||||
string: hello
|
||||
struct: (3,4)
|
||||
or: 7
|
||||
raise caught
|
||||
@@ -1,6 +1,70 @@
|
||||
# 0190 — void failable (`-> !`) implicit fall-through leaves the error slot uninitialized
|
||||
|
||||
**Status:** OPEN
|
||||
**Status:** RESOLVED
|
||||
|
||||
> **Root cause:** `ensureTerminator` (the unified implicit fall-through /
|
||||
> epilogue synthesis in `src/ir/lower/control_flow.zig`) handled `void` and
|
||||
> `noreturn`, but for a pure-failable return type (an `.error_set`) it fell
|
||||
> through to the generic `else` arm and emitted `ret const_undef(ret_ty)`,
|
||||
> leaving the error-channel slot undefined. The bare-`return;` path in
|
||||
> `lowerReturn` (`src/ir/lower/stmt.zig`) already wrote `constInt(0, ret_ty)`
|
||||
> ("no error") for the same case, so adding an explicit `return;` masked the
|
||||
> bug.
|
||||
>
|
||||
> **Fix:** `src/ir/lower/control_flow.zig` `ensureTerminator` — added an arm
|
||||
> that, for a pure-failable (`!ret_ty.isBuiltin() and types.get(ret_ty) ==
|
||||
> .error_set`) end-of-body fall-through, emits `ret constInt(0, ret_ty)`,
|
||||
> matching the explicit-`return;` success path. This covers ALL failable
|
||||
> functions that fall through, not just `main`.
|
||||
>
|
||||
> **Sibling fix (value-failable trailing success expression):** adversarial
|
||||
> review found the SAME uninitialized-error-slot bug on a value-carrying
|
||||
> failable (`-> T !E` / `-> Tuple(A,B) !E`) whose body ends in a trailing
|
||||
> success EXPRESSION (no explicit `return`). `lowerValueBody`
|
||||
> (`src/ir/lower/stmt.zig`) blindly `coerceToType`+`ret`'d the bare success
|
||||
> value to the full failable tuple type, leaving the success error-tag slot
|
||||
> uninitialized → phantom `catch`/`or` on SUCCESS (and a dropped value /
|
||||
> `ret { ... } undef` for string + multi-value returns). **Fix:** before the
|
||||
> generic `coerceToType`+`ret`, mirror the explicit-`return EXPR;` branch — when
|
||||
> `ret_ty` is a value-failable tuple (`!ret_ty.isBuiltin() and
|
||||
> types.get(ret_ty) == .tuple and self.errorChannelOf(ret_ty) != null`) call
|
||||
> `self.lowerFailableSuccessReturn(val, ret_ty, span)` so the success error slot
|
||||
> is set to 0. The pure-failable fall-through (above) and the missing-value
|
||||
> error case (`-> i64 !E { }`) are untouched.
|
||||
>
|
||||
> **Generic + pack-instance fix (unification):** the same trailing-value
|
||||
> body-return was hand-rolled (`coerceToType`+`ret`, no failable-success
|
||||
> routing) in TWO more places — `monomorphizeFunction`
|
||||
> (`src/ir/lower/generic.zig`) and `monomorphizePackFn`
|
||||
> (`src/ir/lower/pack.zig`). A generic value-failable `($T) -> T !E { v }`
|
||||
> instantiated at i64 / string / struct shipped an uninitialized error slot →
|
||||
> phantom `catch` on success and `or` silently yielding the fallback (value
|
||||
> corruption). **Fix:** both sites now DELEGATE the trailing-value return to the
|
||||
> shared `lowerValueBody` (the same helper the decl path uses) instead of
|
||||
> re-implementing it, so the value-failable success routing, the pure-failable
|
||||
> fall-through, and the missing-value diagnostic are all handled in one place
|
||||
> and can't drift again. With this, every body-return path that can carry a
|
||||
> failable channel (decl, generic, pack-instance, closure/lambda) routes the
|
||||
> trailing-success value through `lowerFailableSuccessReturn`. (The JNI
|
||||
> native-method entry wrapper in `ffi.zig` still hand-rolls its body-return,
|
||||
> but a JNI export crosses the C-ABI boundary where the error channel is
|
||||
> forbidden by the ERR E5.1 FFI-boundary rule, so it can never be value-failable.)
|
||||
>
|
||||
> **Regression tests:**
|
||||
> - `examples/errors/1061-errors-void-failable-fallthrough.sx` — a `-> !` callee
|
||||
> that succeeds by fall-through (its `catch` must not fire) called from a
|
||||
> `main :: () -> !` that also falls through (exit 0).
|
||||
> - `examples/errors/1062-errors-value-failable-trailing-expr.sx` — value-failable
|
||||
> trailing-expression successes (`-> i64 !E { 99 }`, `-> string !E { "hi" }`,
|
||||
> `-> Tuple(i64,i64) !E { .(1,2) }`) each `catch`-handled (catch must not fire,
|
||||
> value correct), plus a real `raise` still firing the caller's catch.
|
||||
> - `examples/errors/1063-errors-generic-value-failable-trailing-expr.sx` — a
|
||||
> generic value-failable `($T) -> T !E { v }` instantiated at i64 / string /
|
||||
> struct (each `catch`-handled, catch must not fire, value correct), the
|
||||
> `or`-form yielding the real value not the fallback, plus a generic that
|
||||
> `raise`s still firing the caller's catch.
|
||||
>
|
||||
> Full suite green (examples: 813 ran, 0 failed).
|
||||
|
||||
## Symptom
|
||||
|
||||
|
||||
@@ -239,6 +239,17 @@ pub const DiagnosticList = struct {
|
||||
return false;
|
||||
}
|
||||
|
||||
/// Count of `.err`-level diagnostics (excludes warnings / notes / help).
|
||||
/// Used to detect whether a NEW error was reported across a span of work,
|
||||
/// without a warning/note bumping the total `items` length.
|
||||
pub fn errorCount(self: *const DiagnosticList) usize {
|
||||
var n: usize = 0;
|
||||
for (self.items.items) |d| {
|
||||
if (d.level == .err) n += 1;
|
||||
}
|
||||
return n;
|
||||
}
|
||||
|
||||
fn resolveSourceAndFile(self: *const DiagnosticList, d: Diagnostic) struct { source: []const u8, file_name: []const u8 } {
|
||||
if (d.source_file) |sf| {
|
||||
if (self.import_sources) |is| {
|
||||
|
||||
@@ -1190,6 +1190,14 @@ pub fn ensureTerminator(self: *Lowering, ret_ty: TypeId) void {
|
||||
self.builder.emitUnreachable();
|
||||
} else if (ret_ty == .void) {
|
||||
self.builder.retVoid();
|
||||
} else if (!ret_ty.isBuiltin() and self.module.types.get(ret_ty) == .error_set) {
|
||||
// A pure-failable function (`-> !` / `-> !Named`, whose return type IS
|
||||
// the error set) that falls off the end with no explicit `return;` is
|
||||
// a SUCCESS exit — the error slot must carry 0 ("no error"), exactly
|
||||
// like the bare-`return;` path in lowerReturn. Without this the slot is
|
||||
// left undefined and the caller (or main) reads a garbage tag and
|
||||
// reports a phantom unhandled error (issue 0190).
|
||||
self.builder.ret(self.builder.constInt(0, ret_ty), ret_ty);
|
||||
} else {
|
||||
// Use const_undef for complex types (string, struct, etc.)
|
||||
const default_val = if (ret_ty == .string or !ret_ty.isBuiltin())
|
||||
|
||||
@@ -181,18 +181,15 @@ pub fn monomorphizeFunction(self: *Lowering, fd: *const ast.FnDecl, mangled_name
|
||||
if (!self.currentBlockHasTerminator()) self.builder.emitUnreachable();
|
||||
self.builder.finalize();
|
||||
} else {
|
||||
// Lower the function body
|
||||
// Lower the function body. Delegate the trailing-value return to the
|
||||
// shared `lowerValueBody` so the generic-instantiation path can't drift
|
||||
// from the decl path — it handles all three body-return shapes: the
|
||||
// value-failable success routing (append the success error slot via
|
||||
// `lowerFailableSuccessReturn`, NOT a bare coerce+ret that leaves the
|
||||
// error-tag slot uninitialized — issue 0190), the pure-failable
|
||||
// fall-through, and the missing-value diagnostic.
|
||||
if (ret_ty != .void) {
|
||||
const body_val = self.lowerBlockValue(fd.body);
|
||||
if (!self.currentBlockHasTerminator()) {
|
||||
if (body_val) |val| {
|
||||
const val_ty = self.builder.getRefType(val);
|
||||
const coerced = if (val_ty != .void) self.coerceToType(val, val_ty, ret_ty) else val;
|
||||
self.builder.ret(coerced, ret_ty);
|
||||
} else {
|
||||
self.ensureTerminator(ret_ty);
|
||||
}
|
||||
}
|
||||
self.lowerValueBody(fd.body, ret_ty);
|
||||
} else {
|
||||
self.lowerBlock(fd.body);
|
||||
self.ensureTerminator(ret_ty);
|
||||
|
||||
@@ -1054,16 +1054,13 @@ pub fn monomorphizePackFn(
|
||||
self.lowerBlock(fd.body);
|
||||
if (!self.currentBlockHasTerminator()) self.builder.emitUnreachable();
|
||||
} else if (ret_ty != .void) {
|
||||
const body_val = self.lowerBlockValue(fd.body);
|
||||
if (!self.currentBlockHasTerminator()) {
|
||||
if (body_val) |val| {
|
||||
const val_ty = self.builder.getRefType(val);
|
||||
const coerced = if (val_ty != .void) self.coerceToType(val, val_ty, ret_ty) else val;
|
||||
self.builder.ret(coerced, ret_ty);
|
||||
} else {
|
||||
self.ensureTerminator(ret_ty);
|
||||
}
|
||||
}
|
||||
// Delegate the trailing-value return to the shared `lowerValueBody`
|
||||
// (mirrors the decl + generic paths) so this pack-fn instance can't
|
||||
// drift — it routes the value-failable success through
|
||||
// `lowerFailableSuccessReturn` (appending the success error slot)
|
||||
// instead of a bare coerce+ret that leaves the error-tag slot
|
||||
// uninitialized (issue 0190).
|
||||
self.lowerValueBody(fd.body, ret_ty);
|
||||
} else {
|
||||
self.lowerBlock(fd.body);
|
||||
self.ensureTerminator(ret_ty);
|
||||
|
||||
@@ -127,11 +127,37 @@ pub fn lowerBlockValue(self: *Lowering, node: *const Node) ?Ref {
|
||||
/// `;`-terminated (value discarded) or void — and the body doesn't already
|
||||
/// terminate via `return`/`raise`. Replaces the old silent default-return.
|
||||
pub fn lowerValueBody(self: *Lowering, body: *const Node, ret_ty: TypeId) void {
|
||||
// Snapshot the ERROR count so the missing-value error below can be
|
||||
// suppressed when the body ALREADY reported a real error (e.g. an explicit
|
||||
// `return <pack>` where the pack has no runtime value). Count only `.err`
|
||||
// diagnostics — a warning/note emitted while lowering the body (e.g. an
|
||||
// ObjC selector arity warning) must NOT suppress a genuine missing-value
|
||||
// error, or we'd ship an uninitialized return at exit 0.
|
||||
const errs_before: usize = if (self.diagnostics) |d| d.errorCount() else 0;
|
||||
const body_val = self.lowerBlockValue(body);
|
||||
if (self.currentBlockHasTerminator()) return;
|
||||
if (body_val) |val| {
|
||||
const val_ty = self.builder.getRefType(val);
|
||||
if (val_ty != .void) {
|
||||
// Value-carrying failable `-> (T..., !)`: a trailing success
|
||||
// EXPRESSION (no explicit `return`) yields just the value part —
|
||||
// the compiler must append the success error slot (0). Mirror the
|
||||
// explicit-`return EXPR;` path; a plain `coerceToType` would leave
|
||||
// the error-tag slot uninitialized (phantom catch on success).
|
||||
if (!ret_ty.isBuiltin() and
|
||||
self.module.types.get(ret_ty) == .tuple and
|
||||
self.errorChannelOf(ret_ty) != null)
|
||||
{
|
||||
const span = blk: {
|
||||
if (body.data == .block) {
|
||||
const stmts = body.data.block.stmts;
|
||||
if (stmts.len > 0) break :blk stmts[stmts.len - 1].span;
|
||||
}
|
||||
break :blk body.span;
|
||||
};
|
||||
self.lowerFailableSuccessReturn(val, ret_ty, span);
|
||||
return;
|
||||
}
|
||||
const coerced = self.coerceToType(val, val_ty, ret_ty);
|
||||
self.builder.ret(coerced, ret_ty);
|
||||
return;
|
||||
@@ -148,17 +174,23 @@ pub fn lowerValueBody(self: *Lowering, body: *const Node, ret_ty: TypeId) void {
|
||||
}
|
||||
}
|
||||
if (self.diagnostics) |diags| {
|
||||
if (body.data == .block and body.data.block.discarded_semi != null) {
|
||||
diags.addFmt(.err, body.data.block.discarded_semi.?, "function returns '{s}' but the last expression's value is discarded by this `;` — drop the `;` to return it (or use an explicit `return`)", .{self.formatTypeName(ret_ty)});
|
||||
} else {
|
||||
const span = blk: {
|
||||
if (body.data == .block) {
|
||||
const stmts = body.data.block.stmts;
|
||||
if (stmts.len > 0) break :blk stmts[stmts.len - 1].span;
|
||||
}
|
||||
break :blk body.span;
|
||||
};
|
||||
diags.addFmt(.err, span, "function returns '{s}' but its body produces no value — end it with a trailing expression (no `;`) or an explicit `return`", .{self.formatTypeName(ret_ty)});
|
||||
// Only the body produced no value AND no error was reported while
|
||||
// lowering it — a genuine "missing trailing value", not the fallout of
|
||||
// an already-diagnosed failed return. (If a real error fired, surfacing
|
||||
// the redundant missing-value note would just be noise.)
|
||||
if (diags.errorCount() == errs_before) {
|
||||
if (body.data == .block and body.data.block.discarded_semi != null) {
|
||||
diags.addFmt(.err, body.data.block.discarded_semi.?, "function returns '{s}' but the last expression's value is discarded by this `;` — drop the `;` to return it (or use an explicit `return`)", .{self.formatTypeName(ret_ty)});
|
||||
} else {
|
||||
const span = blk: {
|
||||
if (body.data == .block) {
|
||||
const stmts = body.data.block.stmts;
|
||||
if (stmts.len > 0) break :blk stmts[stmts.len - 1].span;
|
||||
}
|
||||
break :blk body.span;
|
||||
};
|
||||
diags.addFmt(.err, span, "function returns '{s}' but its body produces no value — end it with a trailing expression (no `;`) or an explicit `return`", .{self.formatTypeName(ret_ty)});
|
||||
}
|
||||
}
|
||||
}
|
||||
self.ensureTerminator(ret_ty);
|
||||
|
||||
Reference in New Issue
Block a user