fix(ir): value-failable returning an enum zeroes the success error slot [0097]
A `-> (Enum, !E)` `return .variant` lowered the enum literal with
`target_type` set to the full failable tuple `(Enum, !E)` instead of the
success value type. The bare literal resolves its tag against `target_type`;
against a tuple it matched no variant (silent tag 0) and was stamped with the
tuple type, so `lowerFailableSuccessReturn` saw `val_ty == ret_ty` and took the
forwarding branch — returning the half-built `{value, undef}` aggregate and
never appending the `0` error slot. Every runtime read of the slot on the
success path (`cast(s64) e`, bare `if e`, `e == error.X`) saw garbage nonzero;
only the compile-time `if !e` proof masked it. The s32 case was already correct
because integer literals don't resolve variants against `target_type`.
Fix: in lowerReturn, narrow `target_type` to `failableSuccessType(ret_ty)` for
a value-carrying failable before lowering the returned expression. The enum
literal then resolves to its real ordinal and is typed as the value type, so
the success path correctly appends `0`. Forwarding (`return call()` / explicit
`(v, e)`) is unaffected — those still yield a value typed equal to the tuple.
Regression: examples/1055-errors-enum-value-failable-error-slot.sx reads the
error slot at runtime on the success path (cast, bare if, == error.X), checks a
non-zero ordinal (.blue=2, also corrupted to 0 pre-fix), and asserts the error
path still carries the right tag + error_tag_name. Fails pre-fix, passes after.
This commit is contained in:
44
examples/1055-errors-enum-value-failable-error-slot.sx
Normal file
44
examples/1055-errors-enum-value-failable-error-slot.sx
Normal file
@@ -0,0 +1,44 @@
|
||||
// Enum-valued value-carrying failable: the SUCCESS path must zero the trailing
|
||||
// error slot. Regression (issue 0097). A `-> (Enum, !E)` `return .variant`
|
||||
// resolves the enum literal against the function's VALUE type (the enum), not
|
||||
// the failable tuple — otherwise the literal mis-resolves (tag 0) and is stamped
|
||||
// with the tuple type, which the success-return lowering mistakes for a forwarded
|
||||
// full tuple and leaves the error slot UNDEFINED (read back as garbage nonzero).
|
||||
//
|
||||
// This pins the slot at RUNTIME on the success path (cast(s64) e, bare `if e`,
|
||||
// and `e == error.X`) — not only via the `if !e` proof that the compiler can
|
||||
// fold away. It also exercises a non-zero ordinal (`.blue` = 2) so a value slot
|
||||
// that collapses to 0 is caught, and asserts the error PATH still carries the
|
||||
// right tag and `error_tag_name`.
|
||||
|
||||
#import "modules/std.sx";
|
||||
|
||||
Color :: enum { red; green; blue; }
|
||||
E :: error { Nope }
|
||||
|
||||
pick :: (s: string) -> (Color, !E) {
|
||||
if s == "red" { return .red; }
|
||||
if s == "blue" { return .blue; } // non-zero ordinal (2)
|
||||
raise error.Nope;
|
||||
}
|
||||
|
||||
main :: () -> s32 {
|
||||
// ── success path: error slot MUST read 0 at runtime ──
|
||||
c, e := pick("red");
|
||||
print("success err int = {}\n", cast(s64) e); // 0
|
||||
if e { print("bare-if e: ERROR (WRONG)\n"); } else { print("bare-if e: ok\n"); }
|
||||
if e == error.Nope { print("e == Nope (WRONG)\n"); } else { print("e != Nope (ok)\n"); }
|
||||
if !e { print("guard !e: c = {}\n", cast(s64) c); } // 0 (red)
|
||||
|
||||
// ── non-zero ordinal: value slot must carry the real ordinal ──
|
||||
c2, e2 := pick("blue");
|
||||
if !e2 { print("blue: err int = {}, c = {}\n", cast(s64) e2, cast(s64) c2); } // 0, 2
|
||||
|
||||
// ── error path: the right tag flows through ──
|
||||
c3, e3 := pick("xxx");
|
||||
print("error err int = {}\n", cast(s64) e3); // 1
|
||||
if e3 == error.Nope { print("error: is Nope (ok)\n"); } else { print("error: not Nope (WRONG)\n"); }
|
||||
print("error tag name = {}\n", error_tag_name(e3)); // Nope
|
||||
|
||||
return 0;
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
success err int = 0
|
||||
bare-if e: ok
|
||||
e != Nope (ok)
|
||||
guard !e: c = 0
|
||||
blue: err int = 0, c = 2
|
||||
error err int = 1
|
||||
error: is Nope (ok)
|
||||
error tag name = Nope
|
||||
100
issues/0097-enum-value-failable-error-slot-corruption.md
Normal file
100
issues/0097-enum-value-failable-error-slot-corruption.md
Normal file
@@ -0,0 +1,100 @@
|
||||
# 0097 — value-failable returning an ENUM corrupts the error slot on the success path
|
||||
|
||||
**RESOLVED.** Root cause was **not** the field-offset/width miscalculation originally
|
||||
hypothesized — `tuple_init` / `tuple_get` and the backend struct layout were correct. The real
|
||||
cause was upstream in `lowerReturn` (`src/ir/lower.zig`): when lowering the returned expression of
|
||||
a value-carrying failable `-> (T..., !)`, `target_type` was set to the **full failable tuple**
|
||||
`(Color, !E)` instead of the success **value** type `Color`. A bare enum literal `.red` resolves
|
||||
its variant tag against `target_type` (`lowerEnumLiteral` → `resolveVariantValue`); against a tuple
|
||||
type there is no matching variant, so it returned the silent `0` default AND stamped the result
|
||||
with the tuple type. `lowerFailableSuccessReturn` then saw `val_ty == ret_ty` and took the
|
||||
**forwarding** branch, returning the half-built aggregate `{ value, undef }` as-is — the appended
|
||||
`constInt(0, err_ty)` was never inserted, leaving the error slot `undef` (read back as garbage
|
||||
nonzero) on the success path.
|
||||
|
||||
**Fix:** in `lowerReturn`, when the function's return type is a value-carrying failable tuple,
|
||||
narrow `target_type` to `failableSuccessType(ret_ty)` (the value type / value-tuple) before
|
||||
lowering the returned expression. The enum literal then resolves to its real ordinal and is typed
|
||||
as the value type, so the success-return path correctly appends the `0` error slot. The s32 case
|
||||
was already correct because integer literals don't resolve variants against `target_type`.
|
||||
|
||||
**Regression:** `examples/1055-errors-enum-value-failable-error-slot.sx` — reads the error slot at
|
||||
runtime on the success path (`cast(s64) e`, bare `if e`, `e == error.X`), exercises a non-zero
|
||||
ordinal (`.blue` = 2, which the bug also corrupted to 0), and asserts the error path still carries
|
||||
the right tag + `error_tag_name`. Fails on pre-fix code, passes after. Verified `zig build`,
|
||||
`zig build test`, and `bash tests/run_examples.sh` (452 ok) all green.
|
||||
|
||||
Below preserved as a record of the original problem.
|
||||
|
||||
## Symptom
|
||||
|
||||
A value-failable function `-> (EnumType, !ErrSet)` writes a **garbage nonzero tag into the error
|
||||
slot on the SUCCESS path**. Per specs.md the error channel must be `0` on success ("0 in the
|
||||
error slot means no error"). Every **runtime read** of the slot on success (`cast(s64) err`, bare
|
||||
`if err`, `err == error.X`, and therefore `error_tag_name(err)`) reports a false error. Only the
|
||||
path-sensitive compile-time proof `if !err` reads correctly (it is tied to the SSA value, not a
|
||||
runtime load of the slot), which is why it masks the bug.
|
||||
|
||||
- **Observed (enum value):** success path → error slot reads nonzero (garbage `undef`), not `0`.
|
||||
- **Expected:** success path → error slot reads `0`; `if err` is false; `err == error.X` is false.
|
||||
|
||||
## Reproduction (only imports `modules/std.sx`)
|
||||
|
||||
```sx
|
||||
#import "modules/std.sx";
|
||||
|
||||
Color :: enum { red; green; blue; }
|
||||
E :: error { Nope }
|
||||
|
||||
pick :: (s: string) -> (Color, !E) {
|
||||
if s == "red" { return .red; } // SUCCESS path
|
||||
raise error.Nope;
|
||||
}
|
||||
|
||||
main :: () -> s32 {
|
||||
c, e := pick("red"); // SUCCESS -> error slot MUST be 0
|
||||
print("error e (int) = {}\n", cast(s64) e); // EXPECT 0 ; BUG prints 1
|
||||
if e { print("bare-if e: ERROR (WRONG)\n"); } else { print("bare-if e: ok\n"); }
|
||||
if e == error.Nope { print("e == Nope (WRONG)\n"); } else { print("e != Nope (ok)\n"); }
|
||||
if !e { print("guard !e: value c (int) = {}\n", cast(s64) c); } // c = 0 = .red (CORRECT)
|
||||
return 0;
|
||||
}
|
||||
```
|
||||
|
||||
**Actual (buggy):**
|
||||
```text
|
||||
error e (int) = 1
|
||||
bare-if e: ERROR (WRONG)
|
||||
e == Nope (WRONG)
|
||||
guard !e: value c (int) = 0
|
||||
```
|
||||
**Expected (now produced):**
|
||||
```text
|
||||
error e (int) = 0
|
||||
bare-if e: ok
|
||||
e != Nope (ok)
|
||||
guard !e: value c (int) = 0
|
||||
```
|
||||
|
||||
## Contrast — the IDENTICAL shape with an s32 value is CORRECT
|
||||
|
||||
```sx
|
||||
pick :: (n: s32) -> (s32, !E) { if n > 0 { return n; } raise error.Nope; }
|
||||
// v, e := pick(5); → error slot = 0 (correct); bare-if e: ok
|
||||
```
|
||||
The split is **enum-value-specific** because only an enum literal (`return .variant`) resolves its
|
||||
tag against `target_type`. An integer literal does not, so the s32 path never got mis-stamped with
|
||||
the failable-tuple type and never took the false forwarding branch.
|
||||
|
||||
## Root cause (confirmed at ground truth)
|
||||
|
||||
`return .red` in `pick` lowered the enum literal with `target_type = (Color, !E)` (the whole
|
||||
failable tuple). The LLVM IR on the success path was:
|
||||
|
||||
```llvm
|
||||
ret { i64, i32 } { i64 0, i32 undef } ; error slot UNDEF, not 0 (.blue gave i64 0 too — value lost)
|
||||
```
|
||||
|
||||
vs. the s32 case which already produced `ret { i32, i32 } { i32 7, i32 0 }`. After narrowing the
|
||||
return target to the value type, the enum success path produces `ret { i64, i32 } zeroinitializer`
|
||||
(value 0 = `.red`, error slot 0), and `.blue` correctly carries ordinal 2.
|
||||
@@ -2216,7 +2216,23 @@ pub const Lowering = struct {
|
||||
self.module.functions.items[@intFromEnum(fid)].ret
|
||||
else
|
||||
TypeId.s64;
|
||||
if (ret_ty_for_target != .void) self.target_type = ret_ty_for_target;
|
||||
// A value-carrying failable (`-> (T..., !)`) returns its VALUE part; the
|
||||
// success error slot is appended below by lowerFailableSuccessReturn.
|
||||
// Resolve the returned expression against that value type, NOT the
|
||||
// failable tuple: a bare enum literal `.variant` resolves its tag against
|
||||
// `target_type`, and against the tuple it matches no variant (tag 0) and
|
||||
// is stamped with the tuple type — which lowerFailableSuccessReturn then
|
||||
// treats as a forwarded full tuple, dropping the appended `0` error slot.
|
||||
// Forwarding (`return call()` / explicit `(v, e)`) still yields a value
|
||||
// typed equal to the full tuple, so it is unaffected.
|
||||
const target_for_value: TypeId = if (self.inline_return_target == null and
|
||||
!ret_ty_for_target.isBuiltin() and
|
||||
self.module.types.get(ret_ty_for_target) == .tuple and
|
||||
self.errorChannelOf(ret_ty_for_target) != null)
|
||||
self.failableSuccessType(ret_ty_for_target)
|
||||
else
|
||||
ret_ty_for_target;
|
||||
if (target_for_value != .void) self.target_type = target_for_value;
|
||||
// Evaluate return value first (before defers)
|
||||
const ret_val = if (rs.value) |val| self.lowerExpr(val) else null;
|
||||
self.target_type = old_target;
|
||||
|
||||
Reference in New Issue
Block a user