Merge branch 'flow/distribution/fix-0097'
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,66 @@
|
||||
// Enum-valued value-carrying failables, two paths the bare-success fix (issue
|
||||
// 0097) did NOT originally cover. Regression (issue 0097, attempt-2 review F1+F2).
|
||||
//
|
||||
// F1 — EXPLICIT full failable tuple return `return (.v, error.X)`. The bug was
|
||||
// the inverse of 0097: narrowing the return target to the value type for ALL
|
||||
// value-failables broke the explicit-tuple form (the trailing error element no
|
||||
// longer resolved against the error set → `.unresolved` field → LLVM-emit
|
||||
// panic). The target must stay the FULL failable tuple for an explicit tuple
|
||||
// literal of full arity, and narrow to the value type only for a BARE value.
|
||||
// This pins both branches in one fn: bare-value success + explicit-tuple error.
|
||||
//
|
||||
// F2 — COMPTIME-PARAM ($n) value-failable. The body is INLINED at the call
|
||||
// site (lowerComptimeCall), so the success return takes the inline-return path,
|
||||
// which the original fix skipped — it stored `{value, undef}` (error slot
|
||||
// undefined) on success. Read the error slot at RUNTIME on the success path
|
||||
// (cast, bare `if`, `== error.X`) so an undef slot is caught, not masked by the
|
||||
// `if !e` proof.
|
||||
|
||||
#import "modules/std.sx";
|
||||
|
||||
Color :: enum { red; green; blue; }
|
||||
E :: error { Nope }
|
||||
|
||||
// F1: bare-value success path AND explicit-tuple error path in one function.
|
||||
classify :: (s: string) -> (Color, !E) {
|
||||
if s == "ok" { return .blue; } // bare value → {2, 0}
|
||||
return (.red, error.Nope); // explicit full tuple → {0, 1}
|
||||
}
|
||||
|
||||
// F2: comptime parameter forces inline lowering of the body.
|
||||
ct_pick :: ($n: s32, s: string) -> (Color, !E) {
|
||||
if s == "red" { return .red; } // bare value, inline path → {0, 0}
|
||||
if s == "blue" { return .blue; } // bare value, inline path → {2, 0}
|
||||
raise error.Nope; // inline error path → {undef, 1}
|
||||
}
|
||||
|
||||
main :: () -> s32 {
|
||||
// ── F1 success (bare value, explicit-tuple error fn): error slot 0 ──
|
||||
c, e := classify("ok");
|
||||
print("F1 ok: err int = {}\n", cast(s64) e); // 0
|
||||
if e { print("F1 ok bare-if: ERROR (WRONG)\n"); } else { print("F1 ok bare-if: ok\n"); }
|
||||
if !e { print("F1 ok guard: c = {}\n", cast(s64) c); } // 2 (blue)
|
||||
|
||||
// ── F1 error (explicit tuple): right tag flows, no panic ──
|
||||
c2, e2 := classify("bad");
|
||||
print("F1 bad: err int = {}\n", cast(s64) e2); // 1
|
||||
if e2 == error.Nope { print("F1 bad: is Nope (ok)\n"); } else { print("F1 bad: not Nope (WRONG)\n"); }
|
||||
print("F1 bad: tag name = {}\n", error_tag_name(e2)); // Nope
|
||||
|
||||
// ── F2 success (comptime-param, inline path): error slot 0 at runtime ──
|
||||
c3, e3 := ct_pick(7, "red");
|
||||
print("F2 red: err int = {}\n", cast(s64) e3); // 0
|
||||
if e3 { print("F2 red bare-if: ERROR (WRONG)\n"); } else { print("F2 red bare-if: ok\n"); }
|
||||
if e3 == error.Nope { print("F2 red == Nope (WRONG)\n"); } else { print("F2 red != Nope (ok)\n"); }
|
||||
if !e3 { print("F2 red guard: c = {}\n", cast(s64) c3); } // 0
|
||||
|
||||
c4, e4 := ct_pick(7, "blue");
|
||||
if !e4 { print("F2 blue: err int = {}, c = {}\n", cast(s64) e4, cast(s64) c4); } // 0, 2
|
||||
|
||||
// ── F2 error (comptime-param, inline error path): right tag ──
|
||||
c5, e5 := ct_pick(7, "x");
|
||||
print("F2 err: err int = {}\n", cast(s64) e5); // 1
|
||||
if e5 == error.Nope { print("F2 err: is Nope (ok)\n"); } else { print("F2 err: not Nope (WRONG)\n"); }
|
||||
|
||||
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
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
F1 ok: err int = 0
|
||||
F1 ok bare-if: ok
|
||||
F1 ok guard: c = 2
|
||||
F1 bad: err int = 1
|
||||
F1 bad: is Nope (ok)
|
||||
F1 bad: tag name = Nope
|
||||
F2 red: err int = 0
|
||||
F2 red bare-if: ok
|
||||
F2 red != Nope (ok)
|
||||
F2 red guard: c = 0
|
||||
F2 blue: err int = 0, c = 2
|
||||
F2 err: err int = 1
|
||||
F2 err: is Nope (ok)
|
||||
120
issues/0097-enum-value-failable-error-slot-corruption.md
Normal file
120
issues/0097-enum-value-failable-error-slot-corruption.md
Normal file
@@ -0,0 +1,120 @@
|
||||
# 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`, choose the `target_type` for the returned expression via
|
||||
`failableReturnTarget(ret_ty, value_node)`: for a value-carrying failable a **bare** returned
|
||||
value resolves against `failableSuccessType(ret_ty)` (the value type / value-tuple) so an enum
|
||||
literal gets its real ordinal and the success-return path appends the `0` error slot; an
|
||||
**explicit full failable tuple** literal (`return (v..., e)`, arity == full-tuple field count)
|
||||
keeps the full-tuple target so its trailing error element resolves against the error set and is
|
||||
forwarded as-is. The s32 case was already correct because integer literals don't resolve variants
|
||||
against `target_type`.
|
||||
|
||||
Two follow-up defects from the first cut of this fix were corrected (attempt-2 review):
|
||||
|
||||
- **F1 — explicit full tuple return panicked.** Narrowing the target to the value type for *all*
|
||||
value-failables broke `return (.blue, error.Nope)`: the trailing error element no longer
|
||||
resolved against the error set, leaving an `.unresolved` tuple field that tripped the
|
||||
"unresolved type reached LLVM emission" panic in `src/backend/llvm/types.zig`. The
|
||||
arity-aware `failableReturnTarget` keeps the full-tuple target for the explicit form, so it
|
||||
lowers and forwards as before.
|
||||
- **F2 — comptime-param inline return still corrupted.** A `-> (Enum, !E)` body with a comptime
|
||||
parameter is inlined (`lowerComptimeCall`), so its success `return .red` took the
|
||||
inline-return path (`if (self.inline_return_target)`), which the first cut skipped — it stored
|
||||
`{value, undef}` (error slot `undef`) into the inline slot. That path now applies the same
|
||||
target narrowing and routes a value-carrying failable through `lowerFailableSuccessReturn`
|
||||
(whose `emitTupleRet` stores `{value, 0}` into the inline slot + branches), so the success
|
||||
error slot is `0` there too.
|
||||
|
||||
**Regression:** `examples/1055-errors-enum-value-failable-error-slot.sx` (bare-enum success slot)
|
||||
and `examples/1056-errors-enum-value-failable-tuple-and-comptime.sx` (F1 explicit-tuple error +
|
||||
bare-value success in one fn; F2 comptime-param enum value-failable read at runtime on the success
|
||||
path — `cast`, bare `if`, `== error.X`, plus the error path). Both read the slot at runtime so an
|
||||
`undef` is caught, not masked by the `if !e` proof. Fail on pre-fix code, pass after. Verified
|
||||
`zig build`, `zig build test`, and `bash tests/run_examples.sh` (453 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,19 @@ 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 and
|
||||
// the success error slot (0) is appended by lowerFailableSuccessReturn.
|
||||
// Resolve a BARE returned value 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 the success-return path then
|
||||
// mistakes for a forwarded full tuple, dropping the appended `0` slot.
|
||||
// An explicit full failable tuple return (`return (v..., e)`) keeps the
|
||||
// full-tuple target so its trailing error element resolves against the
|
||||
// error set; it is then forwarded as-is. Applies to the inlined
|
||||
// comptime-body return path too (iri.ret_ty is the failable tuple there).
|
||||
const target_for_value = self.failableReturnTarget(ret_ty_for_target, rs.value);
|
||||
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;
|
||||
@@ -2235,6 +2247,20 @@ pub const Lowering = struct {
|
||||
// terminator do its job.
|
||||
if (self.inline_return_target) |iri| {
|
||||
if (ret_val) |ref| {
|
||||
// Value-carrying failable inlined body: append the success error
|
||||
// slot (0) exactly like the real-return path below.
|
||||
// lowerFailableSuccessReturn routes through emitTupleRet, which
|
||||
// stores into iri.slot and branches to iri.done_bb for an inline
|
||||
// target. Defers first, so the returned SSA value is materialized
|
||||
// before they run (matching the real-return ordering).
|
||||
if (!iri.ret_ty.isBuiltin() and
|
||||
self.module.types.get(iri.ret_ty) == .tuple and
|
||||
self.errorChannelOf(iri.ret_ty) != null)
|
||||
{
|
||||
self.emitBlockDefers(self.func_defer_base);
|
||||
self.lowerFailableSuccessReturn(ref, iri.ret_ty, rs.value.?.span);
|
||||
return;
|
||||
}
|
||||
const val_ty = self.builder.getRefType(ref);
|
||||
const coerced = if (val_ty != iri.ret_ty)
|
||||
self.coerceToType(ref, val_ty, iri.ret_ty)
|
||||
@@ -15169,6 +15195,25 @@ pub const Lowering = struct {
|
||||
} });
|
||||
}
|
||||
|
||||
/// The `target_type` to lower a returned expression against. For a
|
||||
/// value-carrying failable (`-> (T..., !)`) a BARE returned value resolves
|
||||
/// against the success value type (so a bare enum literal gets its real
|
||||
/// ordinal); an EXPLICIT full failable tuple literal (`return (v..., e)`,
|
||||
/// arity == full-tuple field count) keeps the failable-tuple target so its
|
||||
/// trailing error element resolves against the error set and is forwarded
|
||||
/// as-is. Every other return type passes through unchanged.
|
||||
fn failableReturnTarget(self: *Lowering, ret_ty: TypeId, value_node: ?*const Node) TypeId {
|
||||
if (ret_ty.isBuiltin()) return ret_ty;
|
||||
if (self.module.types.get(ret_ty) != .tuple) return ret_ty;
|
||||
if (self.errorChannelOf(ret_ty) == null) return ret_ty;
|
||||
if (value_node) |vn| {
|
||||
if (vn.data == .tuple_literal and
|
||||
vn.data.tuple_literal.elements.len == self.module.types.get(ret_ty).tuple.fields.len)
|
||||
return ret_ty;
|
||||
}
|
||||
return self.failableSuccessType(ret_ty);
|
||||
}
|
||||
|
||||
/// Extract the success value from an evaluated value-carrying failable
|
||||
/// tuple `result` (type `op_ty`): the lone value slot for single-value,
|
||||
/// or an assembled value-tuple (typed `succ_ty`) for multi-value.
|
||||
|
||||
Reference in New Issue
Block a user