From 5a436eddb1e5d047c63a3e719d40fe3cd496af29 Mon Sep 17 00:00:00 2001 From: agra Date: Mon, 22 Jun 2026 22:50:20 +0300 Subject: [PATCH] fix: coerce array/vector literal elements to element type (issue 0168) [N]?T arrays were corrupted: a positional literal .{ null, 7 } stored bare T/null elements into {T,i1} optional slots because array elements were never coerced (getStructFields is empty for an array, so the i{T,i1}, pointer-sentinel->one-word, array->slice, concrete->protocol). Verified by 3 adversarial reviews, suite 780/0. Regression: examples/optionals/0913-optionals-array-of-optionals.sx. Filed adjacent pre-existing bugs: 0173 (typed .[null,..] element), 0174 (tuple positional-element coercion), 0175 (positional struct literal variable element zeroed). --- .../0913-optionals-array-of-optionals.sx | 33 +++++++++ .../0913-optionals-array-of-optionals.exit | 1 + .../0913-optionals-array-of-optionals.stderr | 1 + .../0913-optionals-array-of-optionals.stdout | 7 ++ ...-array-of-optionals-element-load-broken.md | 17 +++++ ...d-array-literal-null-element-unresolved.md | 69 +++++++++++++++++++ ...-positional-literal-element-not-coerced.md | 38 ++++++++++ ...-struct-literal-variable-element-zeroed.md | 46 +++++++++++++ src/ir/lower/expr.zig | 58 +++++++++++----- 9 files changed, 253 insertions(+), 17 deletions(-) create mode 100644 examples/optionals/0913-optionals-array-of-optionals.sx create mode 100644 examples/optionals/expected/0913-optionals-array-of-optionals.exit create mode 100644 examples/optionals/expected/0913-optionals-array-of-optionals.stderr create mode 100644 examples/optionals/expected/0913-optionals-array-of-optionals.stdout create mode 100644 issues/0173-typed-array-literal-null-element-unresolved.md create mode 100644 issues/0174-tuple-positional-literal-element-not-coerced.md create mode 100644 issues/0175-positional-struct-literal-variable-element-zeroed.md diff --git a/examples/optionals/0913-optionals-array-of-optionals.sx b/examples/optionals/0913-optionals-array-of-optionals.sx new file mode 100644 index 00000000..42154fb4 --- /dev/null +++ b/examples/optionals/0913-optionals-array-of-optionals.sx @@ -0,0 +1,33 @@ +// Indexing an array whose element type is an optional `[N]?T` must load the +// full `{T,i1}` element aggregate: a positional `.{ ... }` literal coerces each +// element to the optional element type, so a present element reads back present +// and an absent one absent. Covers a scalar payload (`?i64`), a struct payload +// (`?Pt`), write-then-read, and direct-use of the indexed value (`??`, `if`). +// +// Regression (issue 0168): the positional struct-literal path treated array +// targets as having no named fields and skipped per-element coercion, storing a +// bare `T`/`null` into a `{T,i1}` slot — corrupting the array (a present +// element read back as absent; indexing it segfaulted). +#import "modules/std.sx"; + +Pt :: struct { x: i64; y: i64; } + +main :: () { + // Scalar payload — read present + absent. + arr : [2]?i64 = .{ null, 7 }; + print("idx ?? : {}\n", arr[1] ?? -1); // 7 + e := arr[1]; + if e { print("e present\n"); } else { print("e absent\n"); } // present + if arr[0] { print("a0 present\n"); } else { print("a0 absent\n"); } // absent + + // Write an absent slot, then read it back. + w : [2]?i64 = .{ null, null }; + w[0] = 99; + print("w0: {}\n", w[0] ?? -1); // 99 + print("w1: {}\n", w[1] ?? -1); // -1 + + // Struct payload `?Pt`. + pts : [2]?Pt = .{ null, .{ x = 5, y = 6 } }; + if pts[0] { print("pt0 present\n"); } else { print("pt0 absent\n"); } // absent + if p := pts[1] { print("pt1: {} {}\n", p.x, p.y); } // 5 6 +} diff --git a/examples/optionals/expected/0913-optionals-array-of-optionals.exit b/examples/optionals/expected/0913-optionals-array-of-optionals.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/optionals/expected/0913-optionals-array-of-optionals.exit @@ -0,0 +1 @@ +0 diff --git a/examples/optionals/expected/0913-optionals-array-of-optionals.stderr b/examples/optionals/expected/0913-optionals-array-of-optionals.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/optionals/expected/0913-optionals-array-of-optionals.stderr @@ -0,0 +1 @@ + diff --git a/examples/optionals/expected/0913-optionals-array-of-optionals.stdout b/examples/optionals/expected/0913-optionals-array-of-optionals.stdout new file mode 100644 index 00000000..e672677d --- /dev/null +++ b/examples/optionals/expected/0913-optionals-array-of-optionals.stdout @@ -0,0 +1,7 @@ +idx ?? : 7 +e present +a0 absent +w0: 99 +w1: -1 +pt0 absent +pt1: 5 6 diff --git a/issues/0168-array-of-optionals-element-load-broken.md b/issues/0168-array-of-optionals-element-load-broken.md index bb8228eb..90a38546 100644 --- a/issues/0168-array-of-optionals-element-load-broken.md +++ b/issues/0168-array-of-optionals-element-load-broken.md @@ -1,5 +1,22 @@ # 0168 — indexing an array of optionals `[N]?T` produces a wrong/garbage element (segfault or wrong value) +> **RESOLVED.** Not a GEP/stride bug — a positional literal `.{ null, 7 }` for an +> array target was storing bare `T`/`null` elements into the `{T,i1}` optional +> slots because array elements were never coerced to the element type +> (`getStructFields` is empty for an array, so the per-field coercion gate +> `i < struct_fields.len` never fired). Fix (`src/ir/lower/expr.zig`): in +> `lowerStructLiteral`'s positional branch, compute `array_elem_ty` for +> array/vector targets and coerce each positional element to it; in +> `lowerArrayLiteral`, generalize the previous slice-only coercion to coerce +> every element via `coerceToType` (which is layout-aware — scalar→`{T,i1}`, +> pointer-sentinel→one-word, array→slice, concrete→protocol). Verified across +> scalar/struct/pointer-sentinel optional elements, int→float/widening/erasure/ +> array→slice element coercions, nested + vector arrays, by 3 adversarial +> reviews; suite 780/0. Regression: +> `examples/optionals/0913-optionals-array-of-optionals.sx`. (Adjacent +> pre-existing bugs found + filed: 0173 typed `.[null,…]` element, 0174 tuple +> positional-element coercion, 0175 positional struct literal variable element.) + ## Symptom Reading an element of an array whose element type is an optional (`[N]?T`) is diff --git a/issues/0173-typed-array-literal-null-element-unresolved.md b/issues/0173-typed-array-literal-null-element-unresolved.md new file mode 100644 index 00000000..0b6878a3 --- /dev/null +++ b/issues/0173-typed-array-literal-null-element-unresolved.md @@ -0,0 +1,69 @@ +# 0173 — `(T).[ ... ]` typed array-literal with a `null` element panics (unresolved type at LLVM emission) + +## Symptom + +An explicit-type array literal of the `(T).[ elems ]` form (the `.[...]` +`array_literal` AST node, NOT the `.{ ... }` positional struct literal) whose +element type is an optional and one of whose elements is the bare `null` +literal reaches LLVM emission with an `.unresolved` type and panics: + +``` +thread … panic: unresolved type reached LLVM emission — a type resolution +failure was not diagnosed/aborted + src/backend/llvm/types.zig:196 toLLVMTypeInfo (.unresolved arm) + src/backend/llvm/ops.zig:120 emitConstNull (instruction.ty == .unresolved) +``` + +Observed: hard panic (exit 134). +Expected: the `null` element lowers to a null `?i64`, the literal builds a +`[2]?i64`, and `arr[1] ?? -1` prints `7`. + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + arr := ([2]?i64).[ null, 7 ]; // typed `.[...]` form with a null element + print("{}\n", arr[1] ?? -1); // expected: 7 +} +``` + +Notes: +- The equivalent **positional** form works (fixed under issue 0168): + `arr : [2]?i64 = .{ null, 7 };` correctly prints `7`. +- The `.[...]` form works fine when no element is `null` + (`(i64).[10,20]`, `([2]?i64).[ 1, 7 ]`), and nested slices via `.[...]` + (`rows : [][]i64 = .[ .[1,2], .[3,4,5] ]`) also work. The panic is specific + to a **`null` element under the typed `.[...]` array-literal path**. +- Pre-existing: reproduces on clean HEAD (commit + `2ea25e84`, before the 0168 fix). Independent of issue 0168. + +## Investigation prompt + +`lowerArrayLiteral` (`src/ir/lower/expr.zig`, the `.[...]` `array_literal` +node, NOT `lowerStructLiteral`) resolves the literal's element type from +`al.type_expr` via `resolveArrayLiteralType`. For `([2]?i64).[ ... ]` the +element type should resolve to `?i64`, and each element is lowered with +`target_type = elem_ty`. A bare `null` element lowers via the `null_literal` +path, which produces a `const_null` whose type is `.unresolved` — the +`target_type` (`?i64`) is apparently not being consulted when lowering the bare +`null` element inside the `.[...]` form (whereas it IS in the `.{ ... }` +positional / scalar-assignment paths, which wrap correctly). + +Likely fix area: the `null_literal` lowering in `src/ir/lower/expr.zig` (search +for `.null_literal` / `constNull`) — it must honor `self.target_type` (the +optional element/dest type) so `null` becomes `const_null(?i64)` rather than +`const_null(.unresolved)`. Alternatively, the per-element coercion in +`lowerArrayLiteral` (around the `elem_ty != .unresolved` coercion added for +0168) could coerce the `.unresolved`-typed null to `elem_ty` — but the cleaner +fix is for the bare `null` to resolve its type from `target_type` at lowering +time (matching how `x : ?i64 = null` already works). + +Per the no-silent-fallback rule: do NOT default the unresolved null to a +guessed type; resolve it from the contextual `target_type`, and if that is +itself unresolved, emit a diagnostic rather than letting `.unresolved` reach +LLVM emission. + +Verify: the repro above prints `7`. Also check `([3]?i64).[ null, null, 5 ]` +and a struct-payload `([2]?Pt).[ null, .{x=1,y=2} ]`. Add an +`examples/optionals/09xx-typed-array-literal-null.sx` regression. diff --git a/issues/0174-tuple-positional-literal-element-not-coerced.md b/issues/0174-tuple-positional-literal-element-not-coerced.md new file mode 100644 index 00000000..fcba8e3a --- /dev/null +++ b/issues/0174-tuple-positional-literal-element-not-coerced.md @@ -0,0 +1,38 @@ +# 0174 — positional literal for a TUPLE target does not coerce elements (same corruption class as 0168) + +## Symptom + +A positional literal `.{ a, b }` whose target is a TUPLE does not coerce its +elements to the tuple's field types. When a field type is an optional (or any +type the element doesn't already match), the raw element is stored into the +field slot with the wrong shape — e.g. a bare `i64` into a `{i64,i1}` optional +slot — so the value reads back wrong (a present optional reads as absent). Silent +miscompile. This is the tuple analogue of issue 0168 (which fixed the +array/vector case). + +## Reproduction + +```sx +#import "modules/std.sx"; +main :: () { + t : (?i64, f64) = .{ 7, 3.0 }; + print("{}\n", t.0 ?? -1); // prints "-1" — WRONG, expected 7 +} +``` + +Expected: `7` (field 0 is a present `?i64`). Observed: `-1` (read as absent). + +## Investigation prompt + +`src/ir/lower/expr.zig` `lowerStructLiteral` positional branch. Issue 0168 added +element coercion for array/vector targets via `array_elem_ty`, but a TUPLE target +is neither a struct (so `getStructFields` returns empty → the `i < struct_fields.len` +field-coercion path doesn't fire) nor array/vector (so `array_elem_ty` is +`.unresolved`). Extend the positional branch to recognize a `.tuple` target: +fetch the tuple's per-field types (`TupleInfo.fields`) and coerce element `i` to +`fields[i]` (mirroring the struct-field path, which uses `struct_fields[i].ty`). +Set `target_type` per element so a nested untyped literal element resolves too. +Follow the no-silent-fallback rule. Verify: the repro prints `7`; a tuple with +mixed element coercions (int→float, concrete→protocol, array→slice) initializes +correctly; named tuples `(x: ?i64, y: f64)` too. Add an +`examples/types/01xx-tuple-positional-optional-element.sx` regression. diff --git a/issues/0175-positional-struct-literal-variable-element-zeroed.md b/issues/0175-positional-struct-literal-variable-element-zeroed.md new file mode 100644 index 00000000..eb221269 --- /dev/null +++ b/issues/0175-positional-struct-literal-variable-element-zeroed.md @@ -0,0 +1,46 @@ +# 0175 — positional struct literal with a VARIABLE element silently zeroes the field + +## Symptom + +A positional struct literal `S.{ x, ... }` whose element is a VARIABLE reference +(not a literal constant) silently stores `0` instead of the variable's value. The +NAMED form `S.{ a = x, ... }` works correctly. Silent miscompile. Pre-existing +(reproduces on clean master; surfaced while fixing issue 0168). + +## Reproduction + +```sx +#import "modules/std.sx"; +P :: struct { a: i64 = 0; b: i64 = 0; } +main :: () { + x := 5; + p : P = .{ x, 2 }; // positional, variable first element + print("{} {}\n", p.a, p.b); // prints "0 0" — WRONG, expected "5 2" +} +``` + +Expected: `5 2`. Observed: `0 0` (the variable element zeroed; note even the +`2` literal field is wrong here — the whole positional path mis-coerces once a +variable element is present). The named form `P.{ a = x, b = 2 }` prints `5 2`. + +A related crash: `[2]P = .{ .{ x, 2 }, .{ 3, 4 } }` with an i32 variable `x` +aborted the LLVM verifier on master (`Invalid InsertValueInst operands`); after +the issue 0168 fix it no longer crashes but still prints the residual `0 …` for +the variable element — confirming the root cause is the positional-element +coercion, independent of 0168. + +## Investigation prompt + +`src/ir/lower/expr.zig` `lowerStructLiteral` positional branch, the field +coercion at the `i < struct_fields.len` path (~expr.zig:235-237): it computes +`src_ty = self.inferExprType(fi.value)` then `coerceToType(val, src_ty, +struct_fields[i].ty)`. For a variable reference element, `inferExprType` appears +to return a wrong/narrower type, causing `coerceToType` to mis-narrow/zero the +value. Investigate why `inferExprType(variable_ref)` disagrees with the value's +actual `getRefType`, and prefer coercing from the lowered value's real type +(`self.builder.getRefType(val)`) rather than a re-inferred source type — or fix +the inference for a bare variable element. Verify: the repro prints `5 2`; a +positional literal mixing variable + literal + expression elements; with field +types needing real coercion (i32 var → i64 field, concrete var → protocol field). +Add an `examples/types/01xx-positional-struct-literal-variable-element.sx` +regression. diff --git a/src/ir/lower/expr.zig b/src/ir/lower/expr.zig index 5184725e..b5941ee8 100644 --- a/src/ir/lower/expr.zig +++ b/src/ir/lower/expr.zig @@ -207,16 +207,39 @@ pub fn lowerStructLiteral(self: *Lowering, sl: *const ast.StructLiteral, span: a return result; } - // Positional literal: use source order + // Positional literal: use source order. + // + // For an ARRAY / VECTOR target the literal `.{ a, b, ... }` has no named + // fields — `getStructFields` returns empty for these, so the per-field + // coercion below (`i < struct_fields.len`) never fires. Each positional + // element must still be coerced to the homogeneous element type, or a + // scalar element flowing into an aggregate element slot stores the wrong + // shape. Concretely `[N]?T` would store a bare `T`/`null` into a `{T,i1}` + // slot — corrupting the array (a present element reads back as absent; + // indexing it segfaults). Issue 0168. `coerceToType` is a no-op when the + // element already matches (the common `[N]i64`/`[N]Struct` case). + const array_elem_ty: TypeId = if (!ty.isBuiltin()) switch (self.module.types.get(ty)) { + .array, .vector => self.getElementType(ty), + else => .unresolved, + } else .unresolved; + var fields = std.ArrayList(Ref).empty; defer fields.deinit(self.alloc); for (sl.field_inits, 0..) |fi, i| { + const saved_tt = self.target_type; + if (array_elem_ty != .unresolved) self.target_type = array_elem_ty; var val = self.lowerExpr(fi.value); + self.target_type = saved_tt; // Coerce field value to match struct field type if (i < struct_fields.len) { const src_ty = self.inferExprType(fi.value); val = self.coerceToType(val, src_ty, struct_fields[i].ty); + } else if (array_elem_ty != .unresolved) { + const src_ty = self.builder.getRefType(val); + if (src_ty != array_elem_ty) { + val = self.coerceToType(val, src_ty, array_elem_ty); + } } fields.append(self.alloc, val) catch unreachable; } @@ -1529,22 +1552,23 @@ pub fn lowerArrayLiteral(self: *Lowering, al: *const ast.ArrayLiteral) Ref { self.target_type = elem_ty; var val = self.lowerExpr(elem); self.target_type = old_tt; - // A nested `.[...]` element at a slice element type lowers to an - // aggregate array `[N]U` (lowerArrayLiteral always yields an array - // value); materialize it into a `[]U` slice so the element is a real - // {ptr,len} header rather than a raw array the callee would read its - // header off of. This per-element coercion recurses with - // the literal nesting, so `[][]T` and deeper coerce at every level. - if (!elem_ty.isBuiltin()) { - const ei = self.module.types.get(elem_ty); - if (ei == .slice) { - const val_ty = self.builder.getRefType(val); - if (!val_ty.isBuiltin()) { - const vi = self.module.types.get(val_ty); - if (vi == .array and vi.array.element == ei.slice.element) { - val = self.coerceToType(val, val_ty, elem_ty); - } - } + // Coerce each element to the declared element type. Setting + // `target_type` above steers literal lowering, but the actual + // wrap/erase (scalar → optional `{T,i1}`, array → slice header, + // concrete → protocol, etc.) lives in `coerceToType`. Without this, + // an `[N]?T` literal stores bare `T`/`null` elements into a slot whose + // stride is the optional's `{T,i1}` size — corrupting the aggregate + // (a present element reads back as absent; indexing segfaults). + // Issue 0168. + // + // `coerceToType` classifies a same-type element as `.no_op`/`.none` + // and returns `val` unchanged, so this is a no-op for elements already + // at `elem_ty` (the common `[N]i64`/`[N]Struct` case). The earlier + // slice special-case is now subsumed by this general coercion. + if (elem_ty != .unresolved) { + const val_ty = self.builder.getRefType(val); + if (val_ty != elem_ty) { + val = self.coerceToType(val, val_ty, elem_ty); } } elems.append(self.alloc, val) catch unreachable;