From 3e8d003e3d9a3bf287c2dff1ebd7b77b743d0ee5 Mon Sep 17 00:00:00 2001 From: agra Date: Mon, 22 Jun 2026 21:04:05 +0300 Subject: [PATCH] fix: bindingless if/while/and/or over optional reads has_value (issue 0164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lowerIfExpr emitted optional_has_value only for the binding form; a bare 'if opt' passed the raw {T,i1} aggregate to condBr, where emitCondBr's catch-all struct arm silently folded it to 'i1 true' (structs always truthy) — a silent miscompile that took the present-branch for null optionals. while / and / or shared the same defect. Reduce bindingless optional conditions to optional_has_value in lowerIfExpr/lowerWhile and via a new lowerBoolCondition helper for and/or operands. Replace the silent-true emitCondBr arm with a lowering-time diagnostic (checkConditionType/isValidConditionType) rejecting conditions whose type isn't bool/integer/pointer/optional; the backend @panic is now an unreachable tripwire. Regressions: examples/optionals/0908..0910 + diagnostics/1194 (negative). Verified by 3+3 adversarial reviews. Filed adjacent bugs found during review: 0168 (array-of-optionals element load), 0169 (optional->bool coercion), 0170 (closure-optional layout). --- ...194-diagnostics-condition-non-bool-type.sx | 16 +++++ ...4-diagnostics-condition-non-bool-type.exit | 1 + ...diagnostics-condition-non-bool-type.stderr | 5 ++ ...diagnostics-condition-non-bool-type.stdout | 1 + .../optionals/0908-if-optional-no-binding.sx | 29 ++++++++ .../0909-optionals-while-no-binding.sx | 33 +++++++++ ...0910-optionals-and-or-optional-operands.sx | 33 +++++++++ .../expected/0908-if-optional-no-binding.exit | 1 + .../0908-if-optional-no-binding.stderr | 1 + .../0908-if-optional-no-binding.stdout | 4 ++ .../0909-optionals-while-no-binding.exit | 1 + .../0909-optionals-while-no-binding.stderr | 1 + .../0909-optionals-while-no-binding.stdout | 2 + ...10-optionals-and-or-optional-operands.exit | 1 + ...-optionals-and-or-optional-operands.stderr | 1 + ...-optionals-and-or-optional-operands.stdout | 11 +++ .../0164-if-optional-no-binding-folds-true.md | 18 +++++ ...-array-of-optionals-element-load-broken.md | 45 ++++++++++++ ...-optional-to-bool-coercion-silent-false.md | 48 +++++++++++++ .../0170-closure-optional-layout-truncated.md | 47 +++++++++++++ src/backend/llvm/ops.zig | 16 +++-- src/ir/lower.zig | 2 + src/ir/lower/control_flow.zig | 46 +++++++++++-- src/ir/lower/expr.zig | 68 +++++++++++++++++-- 24 files changed, 418 insertions(+), 13 deletions(-) create mode 100644 examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx create mode 100644 examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.exit create mode 100644 examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stderr create mode 100644 examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stdout create mode 100644 examples/optionals/0908-if-optional-no-binding.sx create mode 100644 examples/optionals/0909-optionals-while-no-binding.sx create mode 100644 examples/optionals/0910-optionals-and-or-optional-operands.sx create mode 100644 examples/optionals/expected/0908-if-optional-no-binding.exit create mode 100644 examples/optionals/expected/0908-if-optional-no-binding.stderr create mode 100644 examples/optionals/expected/0908-if-optional-no-binding.stdout create mode 100644 examples/optionals/expected/0909-optionals-while-no-binding.exit create mode 100644 examples/optionals/expected/0909-optionals-while-no-binding.stderr create mode 100644 examples/optionals/expected/0909-optionals-while-no-binding.stdout create mode 100644 examples/optionals/expected/0910-optionals-and-or-optional-operands.exit create mode 100644 examples/optionals/expected/0910-optionals-and-or-optional-operands.stderr create mode 100644 examples/optionals/expected/0910-optionals-and-or-optional-operands.stdout create mode 100644 issues/0168-array-of-optionals-element-load-broken.md create mode 100644 issues/0169-optional-to-bool-coercion-silent-false.md create mode 100644 issues/0170-closure-optional-layout-truncated.md diff --git a/examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx b/examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx new file mode 100644 index 00000000..0e733e9b --- /dev/null +++ b/examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx @@ -0,0 +1,16 @@ +// A branch condition (`if` / `while` / `and` / `or`) must reduce to an i1: +// its type must be a bool, integer, pointer, or optional. A struct (or float, +// etc.) has no truthiness — it used to be silently folded truthy at lowering +// then `@panic` in the LLVM backend (issue 0164). It must instead be a clean, +// located compile-time TYPE error. +// +// Negative test: locks the new diagnostic. `if ` is rejected. +#import "modules/std.sx"; + +S :: struct { x: i64; } + +main :: () -> i64 { + s : S = .{ x = 1 }; + if s { return 1; } // ERROR: condition must be a bool, integer, pointer, or optional + return 0; +} diff --git a/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.exit b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.exit new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.exit @@ -0,0 +1 @@ +1 diff --git a/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stderr b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stderr new file mode 100644 index 00000000..4bf94a6e --- /dev/null +++ b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stderr @@ -0,0 +1,5 @@ +error: condition must be a bool, integer, pointer, or optional, but has type 'S' + --> examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx:14:8 + | +14 | if s { return 1; } // ERROR: condition must be a bool, integer, pointer, or optional + | ^ diff --git a/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stdout b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stdout new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/diagnostics/expected/1194-diagnostics-condition-non-bool-type.stdout @@ -0,0 +1 @@ + diff --git a/examples/optionals/0908-if-optional-no-binding.sx b/examples/optionals/0908-if-optional-no-binding.sx new file mode 100644 index 00000000..d3ecab30 --- /dev/null +++ b/examples/optionals/0908-if-optional-no-binding.sx @@ -0,0 +1,29 @@ +// Bindingless `if ` must test the optional's has_value flag, not +// fold the `{T,i1}` aggregate truthy. Covers struct-form optionals (`?i64`, +// `?P`) in both states, exercising BOTH branches — without any `|x|` binding. +// +// Regression (issue 0164): a bare `if opt { }` over a struct-repr optional +// emitted `br i1 true`, so a null optional took the present branch. +#import "modules/std.sx"; + +P :: struct { x: i64; } + +check_i64 :: (n: ?i64) { + if n { print("i64 present\n"); } else { print("i64 absent\n"); } +} + +check_p :: (p: ?P) { + if p { print("P present\n"); } else { print("P absent\n"); } +} + +main :: () { + a : ?i64 = null; + b : ?i64 = 42; + check_i64(a); // i64 absent + check_i64(b); // i64 present + + p_null : ?P = null; + p_some : ?P = P.{ x = 9 }; + check_p(p_null); // P absent + check_p(p_some); // P present +} diff --git a/examples/optionals/0909-optionals-while-no-binding.sx b/examples/optionals/0909-optionals-while-no-binding.sx new file mode 100644 index 00000000..b01bd78c --- /dev/null +++ b/examples/optionals/0909-optionals-while-no-binding.sx @@ -0,0 +1,33 @@ +// Bindingless `while ` must test the optional's has_value flag, +// exactly like `if ` — not fold the `{T,i1}` aggregate truthy. +// +// Regression (issue 0164): a bare optional loop condition emitted `br i1 true`, +// so the loop never observed `null` and either spun forever (present→drained) +// or ran when it should have been skipped (already-null). +#import "modules/std.sx"; + +main :: () { + // Present `?i64` drained to null: the body runs each step until the + // optional becomes null, then the loop stops. + countdown : ?i64 = 3; + runs := 0; + while countdown { + runs = runs + 1; + v := countdown!; // unwrap the present value + if v <= 1 { + countdown = null; // drain → loop must STOP next header eval + } else { + countdown = v - 1; + } + } + print("drain runs={}\n", runs); // 3 + + // Already-null `?i64`: the body must NOT run at all. + empty : ?i64 = null; + skipped := 0; + while empty { + skipped = skipped + 1; + empty = null; + } + print("skip runs={}\n", skipped); // 0 +} diff --git a/examples/optionals/0910-optionals-and-or-optional-operands.sx b/examples/optionals/0910-optionals-and-or-optional-operands.sx new file mode 100644 index 00000000..4f65bd01 --- /dev/null +++ b/examples/optionals/0910-optionals-and-or-optional-operands.sx @@ -0,0 +1,33 @@ +// `and` / `or` over bare optional operands reduce each operand to its +// has_value flag (presence-as-truth), exactly like `if `. A bare +// optional reaching the short-circuit merge as a `{T,i1}` aggregate used to +// fold truthy (issue 0164); it must instead test presence. +// +// Truth table below: present `?i64` is "true", absent (null) is "false". +#import "modules/std.sx"; + +yn :: (b: bool) -> string => if b then "T" else "F"; + +main :: () { + p : ?i64 = 7; // present → truthy + q : ?i64 = null; // absent → falsy + + // and: true only when BOTH present. + print("PP and = {}\n", yn(p and p)); // T + print("PQ and = {}\n", yn(p and q)); // F (rhs absent) + print("QP and = {}\n", yn(q and p)); // F (lhs absent, short-circuits) + print("QQ and = {}\n", yn(q and q)); // F + + // or: false only when BOTH absent. + print("PP or = {}\n", yn(p or p)); // T + print("PQ or = {}\n", yn(p or q)); // T (lhs present, short-circuits) + print("QP or = {}\n", yn(q or p)); // T (rhs present) + print("QQ or = {}\n", yn(q or q)); // F + + // Mixed optional-and-bool: optional operand reduced to has_value, + // bool operand used as-is. + flag := true; + print("Pb and = {}\n", yn(p and flag)); // T + print("Qb and = {}\n", yn(q and flag)); // F (lhs absent) + print("bQ or = {}\n", yn(flag or q)); // T (lhs true, short-circuits) +} diff --git a/examples/optionals/expected/0908-if-optional-no-binding.exit b/examples/optionals/expected/0908-if-optional-no-binding.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/optionals/expected/0908-if-optional-no-binding.exit @@ -0,0 +1 @@ +0 diff --git a/examples/optionals/expected/0908-if-optional-no-binding.stderr b/examples/optionals/expected/0908-if-optional-no-binding.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/optionals/expected/0908-if-optional-no-binding.stderr @@ -0,0 +1 @@ + diff --git a/examples/optionals/expected/0908-if-optional-no-binding.stdout b/examples/optionals/expected/0908-if-optional-no-binding.stdout new file mode 100644 index 00000000..266e883a --- /dev/null +++ b/examples/optionals/expected/0908-if-optional-no-binding.stdout @@ -0,0 +1,4 @@ +i64 absent +i64 present +P absent +P present diff --git a/examples/optionals/expected/0909-optionals-while-no-binding.exit b/examples/optionals/expected/0909-optionals-while-no-binding.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/optionals/expected/0909-optionals-while-no-binding.exit @@ -0,0 +1 @@ +0 diff --git a/examples/optionals/expected/0909-optionals-while-no-binding.stderr b/examples/optionals/expected/0909-optionals-while-no-binding.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/optionals/expected/0909-optionals-while-no-binding.stderr @@ -0,0 +1 @@ + diff --git a/examples/optionals/expected/0909-optionals-while-no-binding.stdout b/examples/optionals/expected/0909-optionals-while-no-binding.stdout new file mode 100644 index 00000000..04cced71 --- /dev/null +++ b/examples/optionals/expected/0909-optionals-while-no-binding.stdout @@ -0,0 +1,2 @@ +drain runs=3 +skip runs=0 diff --git a/examples/optionals/expected/0910-optionals-and-or-optional-operands.exit b/examples/optionals/expected/0910-optionals-and-or-optional-operands.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/optionals/expected/0910-optionals-and-or-optional-operands.exit @@ -0,0 +1 @@ +0 diff --git a/examples/optionals/expected/0910-optionals-and-or-optional-operands.stderr b/examples/optionals/expected/0910-optionals-and-or-optional-operands.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/optionals/expected/0910-optionals-and-or-optional-operands.stderr @@ -0,0 +1 @@ + diff --git a/examples/optionals/expected/0910-optionals-and-or-optional-operands.stdout b/examples/optionals/expected/0910-optionals-and-or-optional-operands.stdout new file mode 100644 index 00000000..6cef9069 --- /dev/null +++ b/examples/optionals/expected/0910-optionals-and-or-optional-operands.stdout @@ -0,0 +1,11 @@ +PP and = T +PQ and = F +QP and = F +QQ and = F +PP or = T +PQ or = T +QP or = T +QQ or = F +Pb and = T +Qb and = F +bQ or = T diff --git a/issues/0164-if-optional-no-binding-folds-true.md b/issues/0164-if-optional-no-binding-folds-true.md index 8e7d0204..97c7af18 100644 --- a/issues/0164-if-optional-no-binding-folds-true.md +++ b/issues/0164-if-optional-no-binding-folds-true.md @@ -1,5 +1,23 @@ # 0164 — `if ` with no binding silently folds the has_value test to `true` +> **RESOLVED.** Two colluding sites: `lowerIfExpr` emitted `optional_has_value` +> only for the binding form, and `emitCondBr`'s catch-all struct arm silently +> folded any non-i1 condition to `i1 true` ("structs always truthy"). Fix: reduce +> a bindingless optional condition to `optional_has_value` in `lowerIfExpr`/ +> `lowerWhile`, add a shared `lowerBoolCondition` helper for `and`/`or` operands +> (the same defect affected `while`/`and`/`or`), and add a lowering-time +> diagnostic (`checkConditionType`/`isValidConditionType` in `lower/expr.zig`) +> rejecting conditions whose type isn't bool/integer/pointer/optional — turning +> the `emitCondBr` silent-true into a real type error and leaving the backend +> `@panic` as an unreachable tripwire. Regressions: +> `examples/optionals/0908-if-optional-no-binding.sx`, +> `0909-optionals-while-no-binding.sx`, +> `0910-optionals-and-or-optional-operands.sx`, +> `examples/diagnostics/1194-diagnostics-condition-non-bool-type.sx` (negative). +> Verified by 3 + 3 adversarial reviews. (Adjacent bugs found during review and +> filed separately: 0168 array-of-optionals element load, 0169 optional→bool +> coercion, 0170 closure-optional layout.) + ## Symptom Branching on an optional **without a binding** (`if opt { ... }`) takes the diff --git a/issues/0168-array-of-optionals-element-load-broken.md b/issues/0168-array-of-optionals-element-load-broken.md new file mode 100644 index 00000000..bb8228eb --- /dev/null +++ b/issues/0168-array-of-optionals-element-load-broken.md @@ -0,0 +1,45 @@ +# 0168 — indexing an array of optionals `[N]?T` produces a wrong/garbage element (segfault or wrong value) + +## Symptom + +Reading an element of an array whose element type is an optional (`[N]?T`) is +broken: depending on how the result is used it either SEGFAULTS or yields the +WRONG value (reads a present element as absent). Independent of issue 0164 (the +`if`-on-optional fix) — reproduces with a plain `??` and with a copy-to-local. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + arr : [2]?i64 = .{ null, 7 }; + + // (1) index result used directly → SEGFAULT (exit 134) + x := arr[1]; + print("{}\n", x ?? -1); // expected: 7 + + // (2) copy element to a local then test → WRONG VALUE + e := arr[1]; // element 1 is present (7) + if e { print("present\n"); } else { print("absent\n"); } // prints "absent" — WRONG +} +``` + +Expected: element 1 is the present optional `7`. Observed: segfault in case +(1); `absent` (a wrong/absent optional) in case (2). The original surfacing form +`if arr[0] { ... }` also segfaults. + +## Investigation prompt + +The element load / addressing for an array of optionals appears to compute the +wrong element stride or mis-materialize the loaded `{T,i1}` optional aggregate. +Suspect the index/element-load lowering (`src/ir/lower/expr.zig` index-get path, +and `src/backend/llvm/ops.zig` `emitIndexGet`) when the element type is an +optional aggregate `{T,i1}` — check that the element size/alignment used for the +GEP matches the optional's real size (cf. the `size_of` vs `typeSizeBytes` +nuance for optionals), and that the loaded value is the full aggregate, not a +truncated/garbage read. Compare against a working `[N]T` (non-optional) array +load to isolate whether it's stride math or aggregate materialization. + +Verify: case (1) prints `7`, case (2) prints `present`; also test `[N]?T` with a +struct payload (`[2]?Pt`) and writing elements then reading them back. Add an +`examples/optionals/09xx-array-of-optionals.sx` regression. diff --git a/issues/0169-optional-to-bool-coercion-silent-false.md b/issues/0169-optional-to-bool-coercion-silent-false.md new file mode 100644 index 00000000..205dc60c --- /dev/null +++ b/issues/0169-optional-to-bool-coercion-silent-false.md @@ -0,0 +1,48 @@ +# 0169 — optional passed where `bool` is expected silently coerces to `false` (always) + +## Symptom + +Passing an optional (`?T`) to a `bool` parameter (or any bool-typed position: +bool field initializer, `-> bool` return) compiles WITHOUT a diagnostic and +silently yields `false` for EVERY optional — present or null alike. Silent +miscompile. + +This is inconsistent with `if opt` / `while opt`, which (after issue 0164) +correctly test the has_value flag. The argument/field-coercion path does not. + +## Reproduction + +```sx +#import "modules/std.sx"; +takes_bool :: (b: bool) { if b { print("true\n"); } else { print("false\n"); } } +main :: () { + a : ?i64 = 42; + n : ?i64 = null; + takes_bool(a); // prints "false" — should be a type error, or "true" (present) + takes_bool(n); // prints "false" — (correct only by accident) +} +``` + +Expected: EITHER a compile-time type error (no implicit optional→bool +coercion), OR — if implicit coercion is intended — `true` for the present +optional and `false` for null, matching `if opt` semantics. Observed: always +`false`, no diagnostic. + +## Investigation prompt + +Decide the intended semantics first (check `specs.md` for whether optional→bool +is a legal implicit coercion): + +- If NOT legal: the call-argument / assignment type-checker + (`src/ir/expr_typer.zig` / the coercion/check path) must REJECT an optional in + a bool-typed position with a located diagnostic — not silently produce a + zero/false. Find where the bool target type accepts the optional operand and + emit `self.diagnostics.addFmt(.err, span, ...)`. +- If legal (consistent with `if opt`): the coercion must lower to the + optional's has_value test (reuse `optional_has_value`), not a constant/garbage + `false`. The silent always-`false` is the rejected silent-fallback pattern. + +Whichever is correct, the current always-`false` is wrong. Verify with the repro +(present → `true` or a type error; null → `false` or a type error). Add a +regression: an `examples/optionals/09xx-...` if coercion is legal, or a +`examples/diagnostics/11xx-...` negative test if it's rejected. diff --git a/issues/0170-closure-optional-layout-truncated.md b/issues/0170-closure-optional-layout-truncated.md new file mode 100644 index 00000000..855a5da8 --- /dev/null +++ b/issues/0170-closure-optional-layout-truncated.md @@ -0,0 +1,47 @@ +# 0170 — closure optional `?(() -> ...)` alloca is sized one word, truncating the `{fn,env}` closure value + +## Symptom + +An optional of a closure (`?(() -> void)`, `?Fn`) is mis-laid-out: the optional +alloca is typed `{ {ptr}, i1 }` (one pointer word + flag) but a closure value is +two words `{ {ptr, ptr}, i1 }` = `{ {fn, env}, has }`. Storing the two-word +closure constant into the one-word-typed alloca truncates it; reading the +has_value flag (`extractvalue …, 1`) then returns the closure's `env` word +(commonly null → 0 → `i1 false`) instead of the real flag. A PRESENT closure +optional therefore tests as ABSENT. Silent miscompile. + +Independent of issue 0164 (the condition-reduction fix): `g != null` is also +wrong, so it's a layout/representation bug, not a truthiness bug. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + g : ?(() -> void) = () { print("called\n"); }; + if g { print("present\n"); } else { print("absent\n"); } // prints "absent" — WRONG + if g != null { print("nn-present\n"); } else { print("nn-absent\n"); } // "nn-absent" — WRONG +} +``` + +Expected: `present` / `nn-present` (a freshly-assigned closure is present). +Observed: `absent` / `nn-absent`, exit 0. + +## Investigation prompt + +The optional-of-closure type lowering uses the wrong child layout — it sizes the +optional payload as a single pointer rather than the closure's `{fn, env}` fat +value. Suspect the optional type lowering / `toLLVMType` for `?Closure` +(`src/ir/types.zig` optional lowering + `src/backend/llvm/types.zig`), and the +`optional_wrap` / has_value codegen (`src/backend/llvm/ops.zig` +`emitOptionalWrap` / `emitOptionalHasValue`) — the payload offset/size and the +has_value flag offset must use the closure's full 2-word size. Compare against +the `?Closure` handling the comptime VM already documents (issue 0162's fix +notes a `?Closure` sentinel/`{fn,env}` layout). Decide the canonical runtime +repr (sentinel fn-ptr-null vs discriminated `{ {fn,env}, i1 }`) and make alloca +size, store, and has_value read all agree. + +Verify: the repro prints `present` / `nn-present`; calling through the unwrapped +closure (`g!()`) prints `called`; a null `?Fn` tests absent. Add an +`examples/optionals/09xx-closure-optional.sx` regression (present + null + +call-through). diff --git a/src/backend/llvm/ops.zig b/src/backend/llvm/ops.zig index bfa40af8..b4f6265b 100644 --- a/src/backend/llvm/ops.zig +++ b/src/backend/llvm/ops.zig @@ -2375,11 +2375,19 @@ pub const Ops = struct { cond = c.LLVMBuildICmp(self.e.builder, c.LLVMIntNE, cond, c.LLVMConstNull(cond_ty), "tobool"); } else if (cond_kind == c.LLVMIntegerTypeKind) { cond = c.LLVMBuildICmp(self.e.builder, c.LLVMIntNE, cond, c.LLVMConstInt(cond_ty, 0, 0), "tobool"); - } else if (cond_kind == c.LLVMStructTypeKind) { - // Struct values are always truthy - cond = c.LLVMConstInt(self.e.cached_i1, 1, 0); } else { - cond = c.LLVMConstInt(self.e.cached_i1, 1, 0); // default truthy + // UNREACHABLE backend tripwire. A condBr condition must be i1, + // an integer, or a pointer. Anything else (a struct — e.g. an + // optional `{T,i1}` aggregate — or a float) is now rejected at + // lowering with a located type error: `checkConditionType` in + // src/ir/lower/expr.zig gates every condition site (`if` / + // `while` / `and` / `or`), and optionals are reduced to their + // has_value i1 before reaching here (issue 0164). Folding such a + // condition truthy was a silent miscompile (`if opt { }` always + // took the present branch); reaching this @panic now means a NEW + // condition site bypassed `checkConditionType` — add the check + // there, don't fold truthy. + @panic("emitCondBr: non-boolean condition reached condBr — should have been rejected at lowering as a type error (issue 0164; see checkConditionType in src/ir/lower/expr.zig)"); } } _ = c.LLVMBuildCondBr(self.e.builder, cond, then_bb, else_bb); diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 5415e3ea..0773f08c 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2008,6 +2008,8 @@ pub const Lowering = struct { pub const asmResultType = lower_expr.asmResultType; pub const refCapturePointee = lower_expr.refCapturePointee; pub const lowerBinaryOp = lower_expr.lowerBinaryOp; + pub const lowerBoolCondition = lower_expr.lowerBoolCondition; + pub const checkConditionType = lower_expr.checkConditionType; pub const lowerTupleOp = lower_expr.lowerTupleOp; pub const lowerTupleLexCompare = lower_expr.lowerTupleLexCompare; pub const lowerTupleMembership = lower_expr.lowerTupleMembership; diff --git a/src/ir/lower/control_flow.zig b/src/ir/lower/control_flow.zig index 6c4bcef3..a34c71e4 100644 --- a/src/ir/lower/control_flow.zig +++ b/src/ir/lower/control_flow.zig @@ -66,10 +66,30 @@ pub fn lowerIfExpr(self: *Lowering, ie: *const ast.IfExpr) Ref { self.target_type = null; const opt_val = self.lowerExpr(ie.condition); self.target_type = saved_cond_target; - const cond = if (ie.binding_name != null) blk: { - // The condition is an optional — emit has_value check - break :blk self.builder.emit(.{ .optional_has_value = .{ .operand = opt_val } }, .bool); - } else opt_val; + // Whenever the condition is an optional we must test its has_value flag, + // not the optional aggregate itself. This holds with OR without a binding: + // a bare `if opt { }` must read has_value too (else the `{T,i1}` struct + // reaches condBr and gets folded truthy — issue 0164). `optional_has_value` + // handles every optional repr (struct `{T,i1}`, `?Closure` {fn,env}, + // pointer-sentinel `?*T`/`?cstring`), so this is uniform across all of them. + const cond_ty = self.inferExprType(ie.condition); + const cond_is_optional = blk: { + if (ie.binding_name != null) break :blk true; + if (cond_ty.isBuiltin()) break :blk false; + break :blk self.module.types.get(cond_ty) == .optional; + }; + // A bare `if { }` (no binding) must have a condition type that can + // be tested as an i1 (bool/integer/pointer/optional). Anything else — a + // struct, float, etc. — used to be folded truthy then `@panic` in the + // backend (issue 0164); reject it here with a located type error. With a + // binding (`if v := opt`) the condition is required to be an optional, so + // the optional reduction below applies and we skip the bare-cond check. + const cond = if (cond_is_optional) + self.builder.emit(.{ .optional_has_value = .{ .operand = opt_val } }, .bool) + else if (self.checkConditionType(cond_ty, ie.condition.span)) + opt_val + else + self.builder.constBool(false); const has_else = ie.else_branch != null; // If-else produces a value when inline OR when in value position (force_block_value) var is_value = (ie.is_inline or self.force_block_value) and has_else; @@ -220,7 +240,23 @@ pub fn lowerWhile(self: *Lowering, we: *const ast.WhileExpr) Ref { // Header: evaluate condition self.builder.switchToBlock(header_bb); - const cond = self.lowerExpr(we.condition); + const cond_val = self.lowerExpr(we.condition); + // A bare optional loop condition (`while opt { }`) must test has_value, + // exactly like `if opt { }` — otherwise the `{T,i1}` aggregate reaches + // condBr and folds truthy (issue 0164). `optional_has_value` covers every + // optional repr (struct / `?Closure` / pointer-sentinel). A non-condition + // type (struct/float/...) is a located type error (same as `if`), not a + // backend `@panic`. + const cond = blk: { + const cond_ty = self.inferExprType(we.condition); + if (!cond_ty.isBuiltin() and self.module.types.get(cond_ty) == .optional) { + break :blk self.builder.emit(.{ .optional_has_value = .{ .operand = cond_val } }, .bool); + } + if (!self.checkConditionType(cond_ty, we.condition.span)) { + break :blk self.builder.constBool(false); + } + break :blk cond_val; + }; self.builder.condBr(cond, body_bb, &.{}, exit_bb, &.{}); // Body diff --git a/src/ir/lower/expr.zig b/src/ir/lower/expr.zig index af292d57..97cc3ebf 100644 --- a/src/ir/lower/expr.zig +++ b/src/ir/lower/expr.zig @@ -2745,16 +2745,76 @@ pub fn refCapturePointee(self: *Lowering, node: *const Node) ?TypeId { return if (info == .pointer) info.pointer.pointee else null; } +/// Is `ty` a type that may be used directly as a runtime branch condition? +/// A condBr (and the short-circuit `and`/`or` merges) ultimately tests an +/// i1: lowering must therefore reduce the condition to something the backend +/// can compare against zero/null. The acceptable categories all lower to an +/// LLVM integer or pointer (or, for `.optional`, are reduced to their +/// has_value i1 by the caller): +/// • bool / integers (signed/unsigned/usize/isize) +/// • integer-backed nominals: `enum` (incl. `enum flags`, e.g. `if p & .read`) +/// and `error_set` (a u32 tag) — both reach condBr as a plain integer +/// • pointers: `*T` / `[*]T` / `cstring` (compared against null) +/// • `optional` — the caller emits `optional_has_value` +/// Everything else (float, void, string, any, type_value, struct/union/tuple/ +/// array/slice/vector/function/closure/protocol/pack) reaches condBr as a +/// non-comparable aggregate or a value with no truthiness, and previously got +/// silently folded truthy then `@panic`d in the backend (issue 0164). Such a +/// condition is a type error — see `checkConditionType`. +fn isValidConditionType(self: *Lowering, ty: TypeId) bool { + if (ty == .unresolved) return true; // already-diagnosed elsewhere; don't double-report + return switch (self.module.types.get(ty)) { + .bool, .signed, .unsigned, .usize, .isize => true, + .pointer, .many_pointer, .cstring => true, + .@"enum", .error_set => true, + .optional => true, + else => false, + }; +} + +/// Emit a located type error when `ty` cannot be used as a branch condition +/// (see `isValidConditionType`). Returns `true` if the type is valid (caller +/// proceeds normally), `false` if a diagnostic was emitted (caller should +/// recover with a placeholder bool so lowering doesn't crash before the +/// diagnostic surfaces). This is the lowering-time replacement for the +/// backend `@panic` in `emitCondBr` (issue 0164): the type and span are both +/// available here, so we report a clean compile-time error instead. +pub fn checkConditionType(self: *Lowering, ty: TypeId, span: ast.Span) bool { + if (isValidConditionType(self, ty)) return true; + if (self.diagnostics) |d| d.addFmt(.err, span, "condition must be a bool, integer, pointer, or optional, but has type '{s}'", .{self.formatTypeName(ty)}); + return false; +} + +/// Lower `node` as a boolean condition. If its type is an optional, reduce +/// it to its has_value flag (presence-as-truth) — same rule as `if opt`/ +/// `while opt`. Without this, a bare optional operand reaches a condBr/phi as +/// a `{T,i1}` aggregate and folds truthy (issue 0164). Returns an i1/bool Ref. +/// A non-condition-typed operand (struct/float/...) is rejected with a located +/// type error via `checkConditionType`; on rejection a placeholder `false` is +/// returned so lowering can continue to surface the diagnostic. +pub fn lowerBoolCondition(self: *Lowering, node: *const Node) Ref { + const ty = self.inferExprType(node); + if (!self.checkConditionType(ty, node.span)) { + _ = self.lowerExpr(node); // still lower for side effects / further diagnostics + return self.builder.constBool(false); + } + const v = self.lowerExpr(node); + if (!ty.isBuiltin() and self.module.types.get(ty) == .optional) { + return self.builder.emit(.{ .optional_has_value = .{ .operand = v } }, .bool); + } + return v; +} + pub fn lowerBinaryOp(self: *Lowering, bop: *const ast.BinaryOp) Ref { // Short-circuit: `a and b` → if a then b else false if (bop.op == .and_op) { - const lhs = self.lowerExpr(bop.lhs); + const lhs = self.lowerBoolCondition(bop.lhs); const rhs_bb = self.freshBlock("and.rhs"); const merge_bb = self.freshBlockWithParams("and.merge", &.{.bool}); const false_val = self.builder.constBool(false); self.builder.condBr(lhs, rhs_bb, &.{}, merge_bb, &.{false_val}); self.builder.switchToBlock(rhs_bb); - const rhs = self.lowerExpr(bop.rhs); + const rhs = self.lowerBoolCondition(bop.rhs); self.builder.br(merge_bb, &.{rhs}); self.builder.switchToBlock(merge_bb); return self.builder.blockParam(merge_bb, 0, .bool); @@ -2768,13 +2828,13 @@ pub fn lowerBinaryOp(self: *Lowering, bop: *const ast.BinaryOp) Ref { if (self.orIsFailableChain(bop)) { return self.lowerFailableOr(bop); } - const lhs = self.lowerExpr(bop.lhs); + const lhs = self.lowerBoolCondition(bop.lhs); const rhs_bb = self.freshBlock("or.rhs"); const merge_bb = self.freshBlockWithParams("or.merge", &.{.bool}); const true_val = self.builder.constBool(true); self.builder.condBr(lhs, merge_bb, &.{true_val}, rhs_bb, &.{}); self.builder.switchToBlock(rhs_bb); - const rhs = self.lowerExpr(bop.rhs); + const rhs = self.lowerBoolCondition(bop.rhs); self.builder.br(merge_bb, &.{rhs}); self.builder.switchToBlock(merge_bb); return self.builder.blockParam(merge_bb, 0, .bool);