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<struct_fields.len field-coercion gate never fired). A present element
then read back as absent and direct indexing segfaulted.
lowerStructLiteral's positional branch now computes array_elem_ty for
array/vector targets and coerces each element to it; lowerArrayLiteral
generalizes its slice-only coercion to coerce every element via
coerceToType (layout-aware: scalar->{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).
This commit is contained in:
33
examples/optionals/0913-optionals-array-of-optionals.sx
Normal file
33
examples/optionals/0913-optionals-array-of-optionals.sx
Normal file
@@ -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
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
0
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
|
||||||
@@ -0,0 +1,7 @@
|
|||||||
|
idx ?? : 7
|
||||||
|
e present
|
||||||
|
a0 absent
|
||||||
|
w0: 99
|
||||||
|
w1: -1
|
||||||
|
pt0 absent
|
||||||
|
pt1: 5 6
|
||||||
@@ -1,5 +1,22 @@
|
|||||||
# 0168 — indexing an array of optionals `[N]?T` produces a wrong/garbage element (segfault or wrong value)
|
# 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
|
## Symptom
|
||||||
|
|
||||||
Reading an element of an array whose element type is an optional (`[N]?T`) is
|
Reading an element of an array whose element type is an optional (`[N]?T`) is
|
||||||
|
|||||||
69
issues/0173-typed-array-literal-null-element-unresolved.md
Normal file
69
issues/0173-typed-array-literal-null-element-unresolved.md
Normal file
@@ -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.
|
||||||
38
issues/0174-tuple-positional-literal-element-not-coerced.md
Normal file
38
issues/0174-tuple-positional-literal-element-not-coerced.md
Normal file
@@ -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.
|
||||||
@@ -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.
|
||||||
@@ -207,16 +207,39 @@ pub fn lowerStructLiteral(self: *Lowering, sl: *const ast.StructLiteral, span: a
|
|||||||
return result;
|
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;
|
var fields = std.ArrayList(Ref).empty;
|
||||||
defer fields.deinit(self.alloc);
|
defer fields.deinit(self.alloc);
|
||||||
|
|
||||||
for (sl.field_inits, 0..) |fi, i| {
|
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);
|
var val = self.lowerExpr(fi.value);
|
||||||
|
self.target_type = saved_tt;
|
||||||
// Coerce field value to match struct field type
|
// Coerce field value to match struct field type
|
||||||
if (i < struct_fields.len) {
|
if (i < struct_fields.len) {
|
||||||
const src_ty = self.inferExprType(fi.value);
|
const src_ty = self.inferExprType(fi.value);
|
||||||
val = self.coerceToType(val, src_ty, struct_fields[i].ty);
|
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;
|
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;
|
self.target_type = elem_ty;
|
||||||
var val = self.lowerExpr(elem);
|
var val = self.lowerExpr(elem);
|
||||||
self.target_type = old_tt;
|
self.target_type = old_tt;
|
||||||
// A nested `.[...]` element at a slice element type lowers to an
|
// Coerce each element to the declared element type. Setting
|
||||||
// aggregate array `[N]U` (lowerArrayLiteral always yields an array
|
// `target_type` above steers literal lowering, but the actual
|
||||||
// value); materialize it into a `[]U` slice so the element is a real
|
// wrap/erase (scalar → optional `{T,i1}`, array → slice header,
|
||||||
// {ptr,len} header rather than a raw array the callee would read its
|
// concrete → protocol, etc.) lives in `coerceToType`. Without this,
|
||||||
// header off of. This per-element coercion recurses with
|
// an `[N]?T` literal stores bare `T`/`null` elements into a slot whose
|
||||||
// the literal nesting, so `[][]T` and deeper coerce at every level.
|
// stride is the optional's `{T,i1}` size — corrupting the aggregate
|
||||||
if (!elem_ty.isBuiltin()) {
|
// (a present element reads back as absent; indexing segfaults).
|
||||||
const ei = self.module.types.get(elem_ty);
|
// Issue 0168.
|
||||||
if (ei == .slice) {
|
//
|
||||||
const val_ty = self.builder.getRefType(val);
|
// `coerceToType` classifies a same-type element as `.no_op`/`.none`
|
||||||
if (!val_ty.isBuiltin()) {
|
// and returns `val` unchanged, so this is a no-op for elements already
|
||||||
const vi = self.module.types.get(val_ty);
|
// at `elem_ty` (the common `[N]i64`/`[N]Struct` case). The earlier
|
||||||
if (vi == .array and vi.array.element == ei.slice.element) {
|
// slice special-case is now subsumed by this general coercion.
|
||||||
val = self.coerceToType(val, val_ty, elem_ty);
|
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;
|
elems.append(self.alloc, val) catch unreachable;
|
||||||
|
|||||||
Reference in New Issue
Block a user