Optional (?T) operands were implicitly unwrapped without proof of
presence, silently miscompiling a NULL ?T to garbage. Unwraps in
binary ops and other expression positions are now gated on flow
narrowing: a ?T value is only auto-unwrapped where control flow has
established it is non-null (the narrowed_refs set). Outside a narrowed
region, an implicit unwrap is rejected rather than producing garbage.
Touches the lowering pipeline (lower.zig + lower/{call,closure,coerce,
comptime,control_flow,expr,ffi,generic,pack,stmt}.zig). Adds optionals
examples 0919-0923 and closures example 0312 covering flow narrowing,
binop narrowing, no-implicit-unwrap rejection, and no closure leak of
narrowed state. Updates specs.md and readme.md.
5.1 KiB
0179 — implicit ?T → <concrete> unwrap silently miscompiles a NULL optional to garbage (whole family)
RESOLVED. Root cause:
coerceMode's.optional_unwraparm (src/ir/lower/coerce.zig) unwrapped any?T → concreteUNCONDITIONALLY, never reading the has_value flag — a null optional yielded its zero payload with no diagnostic. Fix took path (a) from the investigation prompt: real flow-sensitive narrowing.?T → concreteis now REJECTED at the coercion site (loud diagnostic listing!/??/ binding /!= null) UNLESS the source is a local proven present by flow narrowing. Narrowing is tracked by name (Lowering.narrowed, region-scoped bylowerBlock/ the if-then arm / a divergent== nullguard / theelsearm, killed on reassignment) and bridged tocoerceModevianarrowed_refs(the loadedRefof a narrowed identifier, tagged inlowerIdentifier). Closes the?bool → boolhole too (issue 0169's carve-out). specs.md §Optional Types + readme updated. Regressions:examples/optionals/0919-optionals-flow-narrowing.sx(narrowing works) +examples/optionals/0920-optionals-no-implicit-unwrap.sx(rejection);0900-optionals-optionals.sxnow exercises genuine narrowing.Out of scope / follow-up: the binary-op operand auto-unwrap (
src/ir/lower/expr.zig~line 3211) is a SEPARATE silent-unwrap path that does NOT route throughclassify/coerceMode—a + bwith a null?Tstill yields garbage with no diagnostic. Filed separately as issue 0185.
Symptom
Passing an optional ?T where a concrete T/other builtin is expected (function
arg, field initializer, -> T return) — WITHOUT any explicit !/??/binding —
compiles silently and unconditionally unwraps the payload. For a PRESENT optional
it yields the value; for a NULL optional it yields the zero/garbage payload with
NO diagnostic. Silent miscompile across the whole ?T → concrete family. (Issue
0169 fixed only the ?T → bool cell where child != bool; the rest of the
family — and the ?bool → bool cell — remain.)
Per specs.md §Optional Types, the ONLY legal ways to extract T from ?T are
! (force unwrap), ?? (coalesce), if v := opt / while-binding, pattern
match, and flow-sensitive narrowing after a != null guard. There is NO implicit
unwrap-at-a-value-position. The T → ?T direction is the only implicit optional
conversion sanctioned.
Reproduction
#import "modules/std.sx";
takes_i32 :: (x: i32) { print("got {}\n", x); }
main :: () {
n : ?i64 = null;
takes_i32(n); // prints "got 0", exit 0 — silent miscompile (no diagnostic)
}
Confirmed silently wrong for the NULL case across: ?i64 → i32, ?i64 → f64,
?f64 → i64, ?i64 → u8, ?i32 → i64 (widen), ?i64 → i64 (same type), and
?bool → bool (yields false). Present optionals unwrap the payload; null
optionals yield 0/garbage. (Found during adversarial review of issue 0169.)
Root cause
src/ir/conversions.zig (CoercionResolver.classify, the "Optional → Concrete
unwrap" rule, ~line 114): if (child_ty == dst_ty or (dst_ty.isBuiltin() and child_ty.isBuiltin())) classifies any ?builtin → builtin (and ?T → T) as
.optional_unwrap. The emitter emitOptionalUnwrap
(src/backend/llvm/ops.zig ~2212) does an UNCONDITIONAL ExtractValue of the
payload field — it never reads the has_value flag — so a null optional yields its
zero payload. (The comptime VM's optHas path traps, so runtime and comptime
diverge.)
Investigation prompt — NOTE: this is design-touching, resolve the semantics first
Per spec, implicit ?T → concrete should be REJECTED entirely (require explicit
!/??/binding). Broadening issue 0169's optional_to_bool_reject to all
?T → concrete is the mechanical change (in classify), BUT it interacts with
flow-sensitive narrowing:
- Flow narrowing is not actually implemented as type refinement. The
non-binding
if x != null { use(x) }spec example (specs.md §Flow-Sensitive Narrowing) currently "works" only because the broken unconditional unwrap fires everywhere, guard or not — there is no real narrowed type. The genuine narrowing mechanism is the BINDING formif v := opt {}(src/ir/lower/control_flow.zig), which emits an explicitoptional_unwrapinto a freshinner_tyscope binding and does NOT route throughclassify. So a broad reject would reject the non-bindingif x != null { use(x) }pattern. Decide: (a) implement real flow-sensitive type refinement soxis genuinelyTinside the guarded branch (then the broad reject is safe), or (b) require the binding form /!/??and update specs.md §Flow-Sensitive Narrowing accordingly. - Close the
?bool → boolhole — issue 0169'schild_ty != .boolcarve-out still silently unwraps a null?booltofalse; per spec there's no implicit?bool → booleither.
Whichever path, the NULL case must never silently yield garbage. Verify the present cases still work via the legal explicit forms; add regressions. This likely warrants its own focused session given the flow-narrowing decision.