From 820cd62fa17f3303ce8b518f3d9dd4dcefbd5e11 Mon Sep 17 00:00:00 2001 From: agra Date: Thu, 25 Jun 2026 13:57:56 +0300 Subject: [PATCH] docs: file issues 0185-0188 File four issue write-ups discovered alongside the 0179 work: - 0185: binary-op operand auto-unwrap silently miscompiles a NULL ?T - 0186: closure VALUE call does not coerce arg to ?T parameter - 0187: lambda with inferred return type + block body with early returns mis-infers its return type - 0188: closure-VALUE calls skip argument validation (arity + tuple spread) --- ...85-binop-optional-operand-silent-unwrap.md | 92 +++++++++++++++++++ ...186-closure-optional-param-arg-coercion.md | 91 ++++++++++++++++++ ...mbda-inferred-return-block-early-return.md | 55 +++++++++++ ...-closure-value-call-arg-validation-gaps.md | 65 +++++++++++++ 4 files changed, 303 insertions(+) create mode 100644 issues/0185-binop-optional-operand-silent-unwrap.md create mode 100644 issues/0186-closure-optional-param-arg-coercion.md create mode 100644 issues/0187-lambda-inferred-return-block-early-return.md create mode 100644 issues/0188-closure-value-call-arg-validation-gaps.md diff --git a/issues/0185-binop-optional-operand-silent-unwrap.md b/issues/0185-binop-optional-operand-silent-unwrap.md new file mode 100644 index 00000000..03e6abba --- /dev/null +++ b/issues/0185-binop-optional-operand-silent-unwrap.md @@ -0,0 +1,92 @@ +# 0185 — binary-op operand auto-unwrap silently miscompiles a NULL `?T` operand to garbage + +> **RESOLVED.** Root cause as diagnosed below: `lowerBinaryOp` +> (`src/ir/lower/expr.zig`) auto-unwrapped optional operands UNCONDITIONALLY. +> Fix: gate both operand unwraps on flow narrowing (the same `narrowed_refs` +> mechanism issue 0179 introduced) — an un-narrowed `?T` operand is rejected via +> the new `diagOptionalOperand`; a guard-narrowed operand still unwraps. `== null` +> / `!= null` presence tests are unaffected (they return early before the +> auto-unwrap). While fixing, an adversarial review of 0179 surfaced a real +> soundness hole: `narrowed_refs` (keyed by per-function `Ref` index) leaked into +> nested bodies whose `Ref` space overlaps (closure literals, generic/pack/comptime +> monomorphization, AND — caught by an INDEPENDENT second-pass review — the JNI +> native-method body path, where residue leaked between consecutive `#jni_main` +> method stubs), letting an outer narrowed `Ref` falsely match a nested `Ref`. +> Closed with `Lowering.NarrowGuard` (save/clear/restore around each such nested +> body): lowerLambda (closure.zig), monomorphizeFunction (generic.zig), +> createComptimeFunctionWithPrelude (comptime.zig), monomorphizePackFn (pack.zig), +> synthesizeJniMainStub (ffi.zig). FnBodyReentry + the explicit clear in +> lowerFunction (decl.zig) cover the rest. Regressions: +> `examples/optionals/0921-optionals-binop-narrowing.sx` + +> `examples/optionals/0922-optionals-binop-no-implicit-unwrap.sx`. +> +> **Discovered (separate, NOT fixed here):** calling a closure VALUE/variable +> whose parameter is `?T` does not coerce the argument to the param type — a +> concrete `7` arrives absent and `null` emits `ptr null` against a `{T,i1}` +> param (LLVM verifier failure). Pre-existing, orthogonal to optional unwrap. +> Filed as issue 0186. + +## Symptom + +An arithmetic / comparison binary op with an OPTIONAL operand (`a + b`, +`a < b`, etc.) unconditionally unwraps the optional's payload — for a PRESENT +optional it yields the value, for a NULL optional it yields the zero/garbage +payload with NO diagnostic. Silent miscompile, same spirit as issue 0179 but a +DIFFERENT code path: this auto-unwrap is in the binary-op lowering, NOT the +`classify` / `coerceMode` coercion ladder that 0179 fixed. + +Per specs.md §Optional Types, the only legal ways to extract `T` from `?T` are +`!` / `??` / `if v := opt` / pattern match / flow-sensitive narrowing after a +`!= null` guard. There is no implicit unwrap-at-an-operand-position; a null +operand must not silently become its zero payload. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + a : ?i64 = null; + b : i64 = 10; + c := a + b; // prints "c = 10" (0 + 10) — silent miscompile, no diagnostic + print("c = {}\n", c); +} +``` + +Expected: a compile error like 0179's +(`cannot use a value of type '?i64' where 'i64' is expected: an optional does +not implicitly unwrap; ...`), unless `a` is proven present by a `!= null` +guard (flow narrowing), in which case the unwrap is sound. + +## Root cause + +`src/ir/lower/expr.zig` (`lowerBinaryOp`, ~line 3210, "Auto-unwrap optional +operands for arithmetic/comparison"): both operand arms do an unconditional +`.optional_unwrap` when the operand type is `.optional`, never reading the +has_value flag and never consulting flow narrowing. This is the operand-side +analogue of the coercion-side bug fixed in 0179. + +## Investigation prompt + +Gate the binary-op operand auto-unwrap on the SAME flow-narrowing mechanism +0179 introduced (`Lowering.narrowed` / `narrowed_refs`, see +`src/ir/lower/control_flow.zig` + `coerceMode`'s `.optional_unwrap` arm in +`src/ir/lower/coerce.zig`): + +1. In `lowerBinaryOp`, when an operand is `?T`, only auto-unwrap it when its + lowered `Ref` is in `self.narrowed_refs` (proven present); otherwise emit + the same loud diagnostic 0179 uses and skip the silent unwrap. The operand + `Ref` is produced by `lowerExpr(bop.lhs/rhs)`, so a guard-narrowed local is + already tagged into `narrowed_refs` by `lowerIdentifier` — the gate is a + `narrowed_refs.contains(operand)` check. +2. Decide the semantics for a present-but-mixed case (e.g. `?i64 + i64`): the + result stays `i64` (unwrap the optional operand) only when narrowed, as + above. Confirm comparisons (`==`/`!=`) against `null` are unaffected — those + are presence tests, not operand unwraps, and must keep working. +3. Verify: the repro above must become a compile error; a guarded + `if a != null { c := a + b; }` must still compute `a + b` correctly; the + existing `examples/optionals/0900-optionals-optionals.sx` `guard2` + (`return a + b` after a compound `== null or` guard) must still pass. + +Add a positive regression (guarded arithmetic) + a negative regression +(unguarded `?T` operand rejected), mirroring 0179's +`examples/optionals/0919` / `0920`. diff --git a/issues/0186-closure-optional-param-arg-coercion.md b/issues/0186-closure-optional-param-arg-coercion.md new file mode 100644 index 00000000..6ac9bd17 --- /dev/null +++ b/issues/0186-closure-optional-param-arg-coercion.md @@ -0,0 +1,91 @@ +# 0186 — calling a closure VALUE with a `?T` parameter does not coerce the argument + +> **RESOLVED.** Root cause as diagnosed below: the closure-VALUE call path in +> `src/ir/lower/call.zig` lowered args without coercing to the closure's +> declared parameter types. Two-part fix: (1) `resolveCallParamTypes` now +> returns the closure/function value's param types for an identifier callee +> bound to a closure/fn VALUE in scope (so args lower with the right +> `target_type`; precedes function-name resolution since a local value shadows +> a function); (2) new free fn `coerceClosureCallArgs` coerces each +> already-lowered user arg to the closure's param type via `coerceToType`, +> applied at all three `call_closure` emission sites (local-variable callee, +> struct-field callee, force-unwrap/expr callee) AND the local function-pointer +> `call_indirect` path (which had the identical gap — an adversarial review +> flagged that the `.function` branch of (1) typed fn-ptr args but never +> coerced them). Now a concrete arg wraps present, `null` → absent — matching a +> top-level fn call, for both closure values and fn-pointer values. Regression: +> `examples/closures/0312-closure-optional-param-arg-coercion.sx` (local + +> struct-field closure + fn-pointer value, concrete + null args). +> +> **Discovered while verifying (separate, NOT fixed here):** a lambda with an +> INFERRED return type (no `-> T`) and a block body with early `return`s +> mis-infers its return type (LLVM verifier failure) even with no optionals. +> Filed as issue 0187. (The 0312 regression uses an explicit `-> i64` to avoid +> it.) + +## Symptom + +When a closure value/variable (a `:=`-bound lambda, or any closure passed as a +value) has a parameter of optional type `?T`, the call site does NOT coerce the +argument to `?T`: + +- A concrete argument (`pick(7)`) is NOT wrapped to a present `?i64` — inside + the body the param reads as ABSENT (`p == null` is true), so the closure + silently returns the wrong branch. +- A `null` argument (`pick(null)`) lowers `null` as a bare `ptr null` against a + `{i64, i1}` parameter, which fails LLVM verification: + `Call parameter type does not match function signature! ptr null { i64, i1 }`. + +The SAME signature/body as a TOP-LEVEL function works correctly (e.g. issue +0900's `guard`), so the bug is specific to the closure-VALUE call path +(`src/ir/lower/call.zig`'s closure/fn-pointer call lowering), not optionals or +flow narrowing. Found during the adversarial review of issues 0179 / 0185. + +## Reproduction + +```sx +#import "modules/std.sx"; +norm :: (p: ?i64) -> i64 { if p == null { return -1; } return 99; } +main :: () { + pick := (p: ?i64) -> i64 => { + if p == null { return -1; } + return 99; + }; + print("pick 7: {}\n", pick(7)); // prints -1 (WRONG — should be 99; arg arrives absent) + print("norm 7: {}\n", norm(7)); // prints 99 (top-level fn, correct) + // print("pick null: {}\n", pick(null)); // LLVM verifier failure: ptr null vs {i64,i1} +} +``` + +Expected: `pick(7)` prints `99` (the `7` wraps to a present `?i64`), and +`pick(null)` compiles (the `null` lowers to an absent `?i64`), matching the +top-level `norm`. + +## Root cause (hypothesis) + +The closure-value call path in `src/ir/lower/call.zig` lowers each argument and +passes it to the closure's trampoline WITHOUT running the `coerceToType` step +that the normal sx-to-sx call path applies against the callee's declared +parameter types. So a `T → ?T` wrap (and `null → ?T`) never happens for a +closure value's optional param. A top-level fn call coerces args to param types, +which is why `norm` works. + +## Investigation prompt + +In `src/ir/lower/call.zig`, find the closure-value / fn-pointer call lowering +(where `%cl.fn`/`%cl.env` trampolines are invoked — grep for `cl.fn` / the +closure-call branch). Confirm it lowers args without coercing to the closure +type's parameter types. The closure's parameter types are available from the +`Closure(...)`/closure TypeInfo. Coerce each lowered argument to the +corresponding parameter type via `self.coerceToType(arg, arg_ty, param_ty)` +before building the call — mirroring the sx-to-sx call path. Verify: +1. The repro above: `pick(7)` → `99`, `pick(null)` compiles and the body sees + absent. +2. No regression in existing closure examples (`examples/closures/`). +3. Add a regression `examples/closures/03xx-closure-optional-param.sx` covering + a concrete arg (wraps present) and a `null` arg (absent) into a closure-value + `?T` param. + +Note: this is purely an argument-coercion gap at the closure-value call site; +it is unrelated to the implicit-optional-unwrap family (issues 0179 / 0185), +which is already fixed. diff --git a/issues/0187-lambda-inferred-return-block-early-return.md b/issues/0187-lambda-inferred-return-block-early-return.md new file mode 100644 index 00000000..c9ff2e55 --- /dev/null +++ b/issues/0187-lambda-inferred-return-block-early-return.md @@ -0,0 +1,55 @@ +# 0187 — lambda with INFERRED return type + block body with early `return`s mis-infers its return type (LLVM verifier failure) + +## Symptom + +A `:=`-bound lambda (closure literal) that has NO explicit `-> T` return type +and whose body is a BLOCK containing `return` statements infers the WRONG +return type (apparently `void`). Calling it and using the value fails LLVM +verification (`Call parameter type does not match function signature! ... i64 +undef` / `Function arguments must have first-class types!`). Adding an explicit +`-> T` makes it work. No optionals or flow narrowing are involved — found while +verifying issue 0186. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + // inferred return type, block body with early returns — NO optionals + f := () => { if 1 > 0 { return 11; } return 22; }; + print("f: {}\n", f()); // LLVM verification failed (return type inferred void/undef) +} +``` + +Workaround / contrast (works): annotate the return type — +`f := () -> i64 => { if 1 > 0 { return 11; } return 22; };` + +## Root cause (hypothesis) + +The lambda return-type inference in `lowerLambda` (`src/ir/lower/closure.zig`, +the `ret_ty` computation around line 164: `const inferred = +self.inferExprType(lam.body);`) does not infer the type from the body's +`return` statements when the body is a block. For a block whose value is +produced only via early `return`s (not a trailing tail expression), +`inferExprType` likely yields `.void`, so the lambda is built with a void +return while the body actually returns `i64` — the mismatch surfaces at the +call site / LLVM verifier. + +## Investigation prompt + +In `src/ir/lower/closure.zig`, the lambda return-type inference path +(`inferExprType(lam.body)` ~line 164) must, for a block body, infer the return +type from the body's `return` statement operands (matching how +`lowerValueBody` / the function-decl return inference handles bodies with early +returns), not just the block's tail-expression value. Reuse the existing +return-type inference the top-level fn path uses (a top-level +`f :: () { if c { return 11; } return 22; }` with inferred return works — see +why, and apply the same to lambdas). Verify: +1. The repro prints `f: 11`. +2. `examples/optionals/0919`/`0921` and `examples/closures/0312` still pass + (0312 deliberately uses explicit `-> i64` to dodge this bug — once fixed, an + inferred-return variant should also work). +3. Add a regression `examples/closures/03xx-lambda-inferred-return-early.sx`. + +Unrelated to the optional-unwrap family (0179/0185) and the closure-arg +coercion fix (0186); purely lambda return-type inference. diff --git a/issues/0188-closure-value-call-arg-validation-gaps.md b/issues/0188-closure-value-call-arg-validation-gaps.md new file mode 100644 index 00000000..728111d1 --- /dev/null +++ b/issues/0188-closure-value-call-arg-validation-gaps.md @@ -0,0 +1,65 @@ +# 0188 — closure-VALUE calls skip argument validation: no arity check + runtime-tuple spread not expanded + +## Symptom + +Calling a closure VALUE (a `:=`-bound lambda, struct-field closure, fn-pointer +value) does NOT validate arguments the way a top-level function call does. Two +distinct gaps, both pre-existing (surfaced during the adversarial review of the +issue-0186 fix; 0186 fixed only arg COERCION for correctly-counted args): + +1. **No arity check.** A closure value called with the WRONG number of args + compiles and silently drops/ignores extras (or reads garbage for missing + ones), exit 0. A top-level fn call diagnoses arity. +2. **Runtime-tuple spread `f(..tuple)` is never expanded for a closure value.** + The spread leaves a `Ref.none` placeholder (`call.zig` ~line 404) that the + `call_closure` sites emit as `undef`, so the call passes garbage. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + // (1) arity: extra arg silently dropped + one := (a: i64) -> i64 => a; + print("arity: {}\n", one(1, 2)); // prints 1 (no error; `2` dropped) + + // (2) spread into a closure value → garbage + add := (a: i64, b: i64) -> i64 => a + b; + pair := (10, 20); + print("spread: {}\n", add(..pair)); // prints garbage (e.g. -17590042754976), not 30 +} +``` + +Expected: (1) an arity diagnostic (as for a top-level fn); (2) `add(..pair)` +expands to `add(10, 20)` → `30`, OR a clear diagnostic that spread into a +closure value is unsupported (never silent garbage). + +## Root cause (hypothesis) + +The closure-value call paths in `src/ir/lower/call.zig` (the three +`call_closure` emission sites + the local `call_indirect` fn-pointer path) build +the arg list and emit directly without (a) an arity check against +`closure.params.len` / `function.params.len`, and (b) without running the +runtime-slice/tuple spread expansion that the normal call path uses +(`packVariadicCallArgs` / the `Ref.none` spread placeholder is never resolved +for closures). The pack-spread `..xs` path (`packSpreadRefs`) handles comptime +packs but not a runtime tuple value spread into a closure. + +## Investigation prompt + +In `src/ir/lower/call.zig`, for each closure-value / fn-pointer-value call site +(grep `call_closure` and the local `call_indirect` path ~line 655): +1. Add an arity check against the callee value's `closure.params` / + `function.params` length (mirror `checkCallArity` used for top-level fns), + accounting for the implicit `__sx_ctx` slot. +2. Either expand a runtime-tuple/slice spread argument into positional args for + closure values (as the normal call path does), or emit a located diagnostic + that spread into a callable value is unsupported — never emit the `Ref.none` + placeholder as `undef`. +3. Regression: extend `examples/closures/0312-...` or add + `examples/closures/03xx-closure-value-arity.sx` covering both the arity + diagnostic and the spread behavior. + +Unrelated to the arg-COERCION fix (issue 0186, already landed) — that fix +correctly coerces a correctly-COUNTED arg; these gaps are about COUNT and +spread expansion.