diff --git a/examples/1056-errors-enum-value-failable-tuple-and-comptime.sx b/examples/1056-errors-enum-value-failable-tuple-and-comptime.sx new file mode 100644 index 0000000..f1e150b --- /dev/null +++ b/examples/1056-errors-enum-value-failable-tuple-and-comptime.sx @@ -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; +} diff --git a/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.exit b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.exit @@ -0,0 +1 @@ +0 diff --git a/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stderr b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stderr new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stderr @@ -0,0 +1 @@ + diff --git a/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stdout b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stdout new file mode 100644 index 0000000..5301b42 --- /dev/null +++ b/examples/expected/1056-errors-enum-value-failable-tuple-and-comptime.stdout @@ -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) diff --git a/issues/0097-enum-value-failable-error-slot-corruption.md b/issues/0097-enum-value-failable-error-slot-corruption.md index ccfa1b0..a9cabcc 100644 --- a/issues/0097-enum-value-failable-error-slot-corruption.md +++ b/issues/0097-enum-value-failable-error-slot-corruption.md @@ -12,17 +12,37 @@ with the tuple type. `lowerFailableSuccessReturn` then saw `val_ty == ret_ty` an `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`. +**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`. -**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. +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. diff --git a/src/ir/lower.zig b/src/ir/lower.zig index d7a2af2..640f2ae 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2216,22 +2216,18 @@ pub const Lowering = struct { self.module.functions.items[@intFromEnum(fid)].ret else TypeId.s64; - // 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 + // 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 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; + // 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; @@ -2251,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) @@ -15185,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.