diff --git a/examples/0167-types-ptr-to-aggregate-field-store.sx b/examples/0167-types-ptr-to-aggregate-field-store.sx new file mode 100644 index 0000000..351e5eb --- /dev/null +++ b/examples/0167-types-ptr-to-aggregate-field-store.sx @@ -0,0 +1,42 @@ +// Storing a struct field whose own type is a pointer to a two-pointer struct +// (`*Pair` — the shape `coerceArg` would closure-auto-promote into `{ptr,null}`) +// must store an 8-byte pointer, never a 16-byte struct that overruns the slot +// and clobbers the neighbouring field. The shared field-lvalue resolver types +// the GEP as `*field_ty`, so `emitStore` unwraps exactly one pointer level to +// the field's own type (`*Pair`, an opaque pointer) instead of the pointee +// aggregate (`Pair`). +// +// Regression (issue 0094, attempt-3 consolidation): the shared `fieldLvaluePtr` +// resolver used to type struct/tuple GEPs with the bare field value type, so a +// `*Pair` field stored via the multi-assign / address-of paths promoted the +// pointer to a 16-byte struct and clobbered the next field (`sentinel` read 0). +// The single-assign path already used the `*field_ty` convention; folding all +// three lvalue sites onto the one resolver applies it uniformly. + +#import "modules/std.sx"; + +Pair :: struct { a: *s64; b: *s64; } +Holder :: struct { pr: *Pair; sentinel: s64; } + +main :: () { + x : s64 = 7; + y : s64 = 9; + pair : Pair = .{ a = @x, b = @y }; + other : Pair = .{ a = @y, b = @x }; + + h : Holder = .{ pr = @pair, sentinel = 99 }; + + // single-assign: store a *Pair into h.pr; sentinel must stay 99. + h.pr = @other; + print("single: a={} b={} sentinel={}\n", h.pr.a.*, h.pr.b.*, h.sentinel); + + // multi-assign: store into h.pr alongside a plain local; sentinel untouched. + dummy : s64 = 0; + h.pr, dummy = @pair, 1; + print("multi: a={} b={} sentinel={}\n", h.pr.a.*, h.pr.b.*, h.sentinel); + + // address-of the *Pair field, store through it; sentinel untouched. + ppr := @h.pr; + ppr.* = @other; + print("addr: a={} b={} sentinel={}\n", h.pr.a.*, h.pr.b.*, h.sentinel); +} diff --git a/examples/expected/0167-types-ptr-to-aggregate-field-store.exit b/examples/expected/0167-types-ptr-to-aggregate-field-store.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/examples/expected/0167-types-ptr-to-aggregate-field-store.exit @@ -0,0 +1 @@ +0 diff --git a/examples/expected/0167-types-ptr-to-aggregate-field-store.stderr b/examples/expected/0167-types-ptr-to-aggregate-field-store.stderr new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/0167-types-ptr-to-aggregate-field-store.stderr @@ -0,0 +1 @@ + diff --git a/examples/expected/0167-types-ptr-to-aggregate-field-store.stdout b/examples/expected/0167-types-ptr-to-aggregate-field-store.stdout new file mode 100644 index 0000000..08de3ba --- /dev/null +++ b/examples/expected/0167-types-ptr-to-aggregate-field-store.stdout @@ -0,0 +1,3 @@ +single: a=9 b=7 sentinel=99 +multi: a=7 b=9 sentinel=99 +addr: a=9 b=7 sentinel=99 diff --git a/issues/0094-missing-struct-field-assignment-unresolved-panic.md b/issues/0094-missing-struct-field-assignment-unresolved-panic.md index e2bb586..6049c49 100644 --- a/issues/0094-missing-struct-field-assignment-unresolved-panic.md +++ b/issues/0094-missing-struct-field-assignment-unresolved-panic.md @@ -14,20 +14,34 @@ > assignment, address-of, and multi-target assignment — route field resolution > through one shared helper, `fieldLvaluePtr(obj_ptr, obj_ty, field)`, which > resolves struct fields, union/tagged-union direct fields, promoted -> anonymous-struct union members, and tuple elements, and returns `null` (no -> field 0 / `.unresolved` default) when nothing matches. Each caller emits the -> read path's field-not-found diagnostic (`emitFieldError`) on a `null` result: -> 1. `lowerAssignment` `.field_access` target — `found`-flag bail on the struct -> loop, with union direct + promoted handled before it. +> anonymous-struct union members, tuple elements, and vector lanes (reusing +> `vectorLaneIndex`), and returns `null` (no field 0 / `.unresolved` /`.s64` +> default) when nothing matches. Each caller emits the read path's +> field-not-found diagnostic (`emitFieldError`) on a `null` result: +> 1. `lowerAssignment` `.field_access` target — delegates to `fieldLvaluePtr`; +> its own duplicated union / promoted / tuple / vector / struct walk is +> deleted (issue-0083 two-resolver divergence removed). > 2. `lowerExprAsPtr` `.field_access` — delegates to `fieldLvaluePtr`, so the -> address-of path now resolves promoted union members (`@v.x`) — not only -> direct union fields — and a genuine miss errors. The `.s64` sentinel is gone. +> address-of path resolves promoted union members (`@v.x`) — not only direct +> union fields — and a genuine miss errors. The `.s64` sentinel is gone. > 3. `lowerMultiAssign` `.field_access` target — replaced its struct-only loop > (which defaulted `field_idx 0` / `field_ty .unresolved` on a miss, silently > storing into field 0 — `p.q, y = 2, 3` printed `x=2 y=3`) with the shared > `fieldLvaluePtr`; a missing field now errors, and a valid promoted-union / > tuple member at a non-zero offset stores into its own slot, not field 0. > +> `fieldLvaluePtr` types every GEP `*field_ty` (a pointer to the field), the +> convention the single-assign path always used: `emitStore` reads the +> store-target pointer's IR type and unwraps exactly one `.pointer` level to +> find the stored value's type. The earlier `lowerExprAsPtr` / `lowerMultiAssign` +> walks typed the GEP with the *bare* field value type, so a field whose own +> type is a pointer-to-aggregate (`*Pair`, a two-pointer struct) made `emitStore` +> unwrap to the aggregate and `coerceArg`'s closure auto-promotion store a +> 16-byte `{ptr,null}` struct over the 8-byte slot — clobbering the neighbouring +> field. Consolidating all three sites onto the one `*field_ty` resolver +> preserves single-assign and fixes that pre-existing multi-assign / address-of +> clobber. +> > All sites reuse `emitFieldError` (the exact facility the read path > `lowerFieldAccessOnType` uses), so the read and write paths reject identically. > The `src/backend/llvm/types.zig` tripwire is untouched — the fix is to never @@ -38,8 +52,10 @@ > exit 1), `examples/0165-types-nested-struct-field-assign.sx` (positive — nested > struct field write + address-of a matched field), `examples/0166-types-union-promoted-member-lvalue.sx` > (positive — promoted union member written and address-of'd, including a non-zero -> offset member), and two lowering unit tests in `src/ir/lower.test.zig` -> (single- and multi-assign missing-field field-not-found). +> offset member), `examples/0167-types-ptr-to-aggregate-field-store.sx` (positive — +> a `*Pair` field stored via all three lvalue sites leaves the neighbour intact), +> and three lowering unit tests in `src/ir/lower.test.zig` (single- and +> multi-assign missing-field field-not-found, plus the `*field_ty` GEP convention). ## Symptom Assigning to a nonexistent struct field (`p.q = ...`) panics during LLVM emission instead of reporting a source diagnostic. diff --git a/src/ir/lower.test.zig b/src/ir/lower.test.zig index 78ab31f..765e36d 100644 --- a/src/ir/lower.test.zig +++ b/src/ir/lower.test.zig @@ -1174,6 +1174,74 @@ test "lower: multi-assign to a missing struct field emits field-not-found, no co try std.testing.expect(found); } +test "lower: shared resolver types a pointer-typed field GEP as *field_ty, not field_ty (issue 0094 clobber)" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + var module = ir_mod.Module.init(alloc); + defer module.deinit(); + + const span = ast.Span{ .start = 0, .end = 0 }; + + // Register `S :: struct { p: *s64; }` — the field's own type is a pointer. + const ptr_s64 = module.types.ptrTo(.s64); + const fields = [_]ir_mod.types.TypeInfo.StructInfo.Field{ + .{ .name = module.types.internString("p"), .ty = ptr_s64 }, + }; + _ = module.types.intern(.{ .@"struct" = .{ .name = module.types.internString("S"), .fields = &fields } }); + + // mutate :: (s: *S, q: *s64) { d := 0; s.p, d = q, 1; } + // The multi-assign target routes `s.p` through the shared fieldLvaluePtr + // resolver. Pre-fix that resolver typed the field GEP with the bare field + // value type (`*s64`), so emitStore unwrapped one level to `s64` and + // coerceArg's closure auto-promotion stored a 16-byte struct over the + // 8-byte field, clobbering the neighbour. The resolver now types the GEP + // `*(*s64)` so emitStore stops at the field's own pointer type. + var s_pointee = Node{ .span = span, .data = .{ .type_expr = .{ .name = "S", .is_generic = false } } }; + var s_ty = Node{ .span = span, .data = .{ .pointer_type_expr = .{ .pointee_type = &s_pointee } } }; + var q_pointee = Node{ .span = span, .data = .{ .type_expr = .{ .name = "s64", .is_generic = false } } }; + var q_ty = Node{ .span = span, .data = .{ .pointer_type_expr = .{ .pointee_type = &q_pointee } } }; + + var d_init = Node{ .span = span, .data = .{ .int_literal = .{ .value = 0 } } }; + var d_decl = Node{ .span = span, .data = .{ .var_decl = .{ .name = "d", .name_span = span, .type_annotation = null, .value = &d_init } } }; + + var s_ident = Node{ .span = span, .data = .{ .identifier = .{ .name = "s" } } }; + var target0 = Node{ .span = span, .data = .{ .field_access = .{ .object = &s_ident, .field = "p" } } }; + var target1 = Node{ .span = span, .data = .{ .identifier = .{ .name = "d" } } }; + var q_rhs = Node{ .span = span, .data = .{ .identifier = .{ .name = "q" } } }; + var v1 = Node{ .span = span, .data = .{ .int_literal = .{ .value = 1 } } }; + const targets = [_]*Node{ &target0, &target1 }; + const values = [_]*Node{ &q_rhs, &v1 }; + var massign = Node{ .span = span, .data = .{ .multi_assign = .{ .targets = &targets, .values = &values } } }; + + const stmts = [_]*Node{ &d_decl, &massign }; + var body = Node{ .span = span, .data = .{ .block = .{ .stmts = &stmts } } }; + const params = [_]ast.Param{ + .{ .name = "s", .name_span = span, .type_expr = &s_ty }, + .{ .name = "q", .name_span = span, .type_expr = &q_ty }, + }; + const fd = ast.FnDecl{ .name = "mutate", .params = ¶ms, .return_type = null, .body = &body }; + + var lowering = Lowering.init(&module); + lowering.lowerFunction(&fd, "mutate", false); + + // The field-store GEP must be typed `*(*s64)`: its pointee is the field's + // own type (`*s64`), not the field's pointee (`s64`). + const func = module.getFunction(FuncId.fromIndex(0)); + var found = false; + for (func.blocks.items) |blk| { + for (blk.insts.items) |inst| { + if (inst.op == .struct_gep) { + const info = module.types.get(inst.ty); + try std.testing.expect(info == .pointer); + try std.testing.expectEqual(ptr_s64, info.pointer.pointee); + found = true; + } + } + } + try std.testing.expect(found); +} + test "lower: reflectionArgIsType accepts spelled types, rejects plain values (issue 0090)" { const alloc = std.testing.allocator; var module = ir_mod.Module.init(alloc); diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 3907ef3..4a32877 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2372,136 +2372,26 @@ pub const Lowering = struct { } else if (is_special_container and std.mem.eql(u8, fa.field, "ptr")) { const gep = self.builder.structGepTyped(obj_ptr, 0, .s64, obj_ty); self.storeOrCompound(gep, val, asgn.op, .s64); - } else { - const field_name_id = self.module.types.internString(fa.field); - - // Check if this is a union field assignment - if (!obj_ty.isBuiltin()) { - const type_info = self.module.types.get(obj_ty); - const union_fields: ?[]const types.TypeInfo.StructInfo.Field = switch (type_info) { - .@"union" => |u| u.fields, - .tagged_union => |u| u.fields, - else => null, - }; - if (union_fields) |fields| { - for (fields, 0..) |f, i| { - if (f.name == field_name_id) { - const gep = self.builder.emit(.{ .union_gep = .{ .base = obj_ptr, .field_index = @intCast(i), .base_type = obj_ty } }, self.module.types.ptrTo(f.ty)); - const src_ty = self.builder.getRefType(val); - const coerced = self.coerceToType(val, src_ty, f.ty); - self.storeOrCompound(gep, coerced, asgn.op, f.ty); - return; - } - // Check promoted fields from anonymous struct variants - if (!f.ty.isBuiltin()) { - const fi = self.module.types.get(f.ty); - if (fi == .@"struct") { - for (fi.@"struct".fields, 0..) |sf, si| { - if (sf.name == field_name_id) { - // GEP into union payload area, then into the struct field - const union_gep = self.builder.emit(.{ .union_gep = .{ .base = obj_ptr, .field_index = @intCast(i), .base_type = obj_ty } }, self.module.types.ptrTo(f.ty)); - const field_gep = self.builder.structGepTyped(union_gep, @intCast(si), sf.ty, f.ty); - const src_ty = self.builder.getRefType(val); - const coerced = self.coerceToType(val, src_ty, sf.ty); - self.storeOrCompound(field_gep, coerced, asgn.op, sf.ty); - return; - } - } - } - } - } - } - } - - // Tuple field element: `t.0 = v` / `t.x = v` — indexed by - // position (numeric) or by name, not via `getStructFields` - // (a tuple's elements aren't named-struct fields). Mirrors - // the read path's tuple handling. - if (!obj_ty.isBuiltin()) { - const ti = self.module.types.get(obj_ty); - if (ti == .tuple) { - const tup = ti.tuple; - var elem_idx: ?usize = null; - if (std.fmt.parseInt(usize, fa.field, 10)) |n| { - if (n < tup.fields.len) elem_idx = n; - } else |_| { - if (tup.names) |names| { - for (names, 0..) |nm, i| { - if (nm == field_name_id and i < tup.fields.len) { - elem_idx = i; - break; - } - } - } - } - if (elem_idx) |idx| { - const elem_ty = tup.fields[idx]; - const gep = self.builder.structGepTyped(obj_ptr, @intCast(idx), self.module.types.ptrTo(elem_ty), obj_ty); - const src_ty = self.builder.getRefType(val); - const coerced = self.coerceToType(val, src_ty, elem_ty); - self.storeOrCompound(gep, coerced, asgn.op, elem_ty); - return; - } - } - } - - // Vector lane assignment: `v.x = val` (also .y/.z/.w and the - // colour aliases .r/.g/.b/.a). GEP a pointer to the lane and - // store with the vector element type — mirrors the read path - // (lowerFieldAccessOnType) and shares vectorLaneIndex so the - // two never diverge. Without this the field falls through to - // the struct-field lookup below, where no field matches, - // leaving field_ty = .unresolved and a pointer-to-.unresolved - // that panics at LLVM emission (issue 0086). - if (!obj_ty.isBuiltin()) { - const vinfo = self.module.types.get(obj_ty); - if (vinfo == .vector) { - const vidx = Lowering.vectorLaneIndex(fa.field) orelse { - _ = self.emitFieldError(obj_ty, fa.field, asgn.target.span); - return; - }; - const elem_ty = vinfo.vector.element; - const gep = self.builder.structGepTyped(obj_ptr, vidx, self.module.types.ptrTo(elem_ty), obj_ty); - const src_ty = self.builder.getRefType(val); - const coerced = self.coerceToType(val, src_ty, elem_ty); - self.storeOrCompound(gep, coerced, asgn.op, elem_ty); - return; - } - } - - const struct_fields = self.getStructFields(obj_ty); - var field_idx: u32 = 0; - var field_ty: TypeId = .unresolved; - var found = false; - for (struct_fields, 0..) |f, i| { - if (f.name == field_name_id) { - field_idx = @intCast(i); - field_ty = f.ty; - found = true; - break; - } - } - if (!found) { - // No struct field matches the assignment target. Emit the - // same field-not-found diagnostic the read path uses - // (lowerFieldAccessOnType → emitFieldError) and bail; building - // ptrTo(field_ty) with field_ty = .unresolved would otherwise - // store through a pointer-to-.unresolved that panics at LLVM - // emission (issue 0094). - _ = self.emitFieldError(obj_ty, fa.field, asgn.target.span); - return; - } - // Wrap in ptrTo so the store handler sees *field_ty (consistent - // with index_gep which uses ptrTo(elem_ty)). Without this, a - // [*]BigNode field makes the store handler extract BigNode as the - // target type, storing element-sized bytes instead of a pointer. - const gep_ty = self.module.types.ptrTo(field_ty); - const gep = self.builder.structGepTyped(obj_ptr, field_idx, gep_ty, obj_ty); - // Coerce value to field type — use the lowered value's actual type - // (not inferExprType, which can re-read target_type after restore). + } else if (self.fieldLvaluePtr(obj_ptr, obj_ty, fa.field)) |fl| { + // Resolve the target field (struct / union direct / promoted + // anonymous-struct member / tuple element / vector lane) via + // the shared lvalue resolver — the same one the address-of + // and multi-target store paths use — so the three never + // resolve a field to a different slot or default field 0 + // (issue 0094 / issue-0083 two-resolver class). fl.ptr is + // *field_ty (the store handler unwraps one pointer level); + // fl.ty is the value type to coerce the rhs to. const src_ty = self.builder.getRefType(val); - const coerced = self.coerceToType(val, src_ty, field_ty); - self.storeOrCompound(gep, coerced, asgn.op, field_ty); + const coerced = self.coerceToType(val, src_ty, fl.ty); + self.storeOrCompound(fl.ptr, coerced, asgn.op, fl.ty); + } else { + // No struct / union / tuple / vector field matches the + // assignment target. Emit the same field-not-found + // diagnostic the read path uses (emitFieldError) and bail; + // building a pointer with field_ty = .unresolved would + // otherwise store through a pointer-to-.unresolved that + // panics at LLVM emission (issue 0094). + _ = self.emitFieldError(obj_ty, fa.field, asgn.target.span); } }, .index_expr => |ie| { @@ -2569,10 +2459,22 @@ pub const Lowering = struct { /// Resolve `obj.field` — where `obj_ptr` already points at the aggregate — /// to a typed pointer into the field's storage plus the field's value type. /// Handles union direct fields, promoted anonymous-struct union members, - /// tuple elements (numeric or named), and plain struct fields. Returns null - /// when no field matches; the caller emits the field-not-found diagnostic. - /// Single source of lvalue field resolution shared by lowerExprAsPtr - /// (address-of) and lowerMultiAssign (multi-target store) so the two never + /// tuple elements (numeric or named), vector lanes (`.x`/`.y`/`.z`/`.w` and + /// the colour aliases), and plain struct fields. Returns null when no field + /// matches; the caller emits the field-not-found diagnostic. + /// + /// `ptr`'s IR type is `*field_ty` (a pointer to the field), NOT the field + /// value type: `emitStore` reads the store-target pointer's IR type and + /// unwraps one `.pointer` level to find the stored value's type. Labelling + /// the GEP with the bare field type instead would make a field whose own + /// type is a pointer-to-aggregate (`*Pair`) coerce the stored pointer into + /// the aggregate (closure auto-promotion in `coerceArg`), storing an + /// oversized struct that clobbers the neighbouring field. `.ty` carries the + /// field's value type for the caller's coercion. + /// + /// Single source of lvalue field resolution shared by all three store/ + /// address-of sites — lowerAssignment (single-target store), lowerExprAsPtr + /// (address-of), and lowerMultiAssign (multi-target store) — so they never /// resolve a field to a different slot or default field 0 (issue 0094). fn fieldLvaluePtr(self: *Lowering, obj_ptr: Ref, obj_ty: TypeId, field: []const u8) ?FieldLvalue { if (obj_ty.isBuiltin()) return null; @@ -2599,7 +2501,7 @@ pub const Lowering = struct { for (fi.@"struct".fields, 0..) |sf, si| { if (sf.name == field_name_id) { const ug = self.builder.emit(.{ .union_gep = .{ .base = obj_ptr, .field_index = @intCast(i), .base_type = obj_ty } }, self.module.types.ptrTo(f.ty)); - const ptr = self.builder.structGepTyped(ug, @intCast(si), sf.ty, f.ty); + const ptr = self.builder.structGepTyped(ug, @intCast(si), self.module.types.ptrTo(sf.ty), f.ty); return .{ .ptr = ptr, .ty = sf.ty }; } } @@ -2627,17 +2529,27 @@ pub const Lowering = struct { } if (elem_idx) |idx| { const elem_ty = tup.fields[idx]; - const ptr = self.builder.structGepTyped(obj_ptr, @intCast(idx), elem_ty, obj_ty); + const ptr = self.builder.structGepTyped(obj_ptr, @intCast(idx), self.module.types.ptrTo(elem_ty), obj_ty); return .{ .ptr = ptr, .ty = elem_ty }; } return null; } + // Vector lane: `.x`/`.y`/`.z`/`.w` (or colour aliases `.r`/`.g`/`.b`/`.a`) + // → lane 0/1/2/3 via the same vectorLaneIndex the read path uses. A + // non-lane field on a vector is a genuine miss (caller diagnoses). + if (type_info == .vector) { + const vidx = Lowering.vectorLaneIndex(field) orelse return null; + const elem_ty = type_info.vector.element; + const ptr = self.builder.structGepTyped(obj_ptr, vidx, self.module.types.ptrTo(elem_ty), obj_ty); + return .{ .ptr = ptr, .ty = elem_ty }; + } + // Plain struct field. const struct_fields = self.getStructFields(obj_ty); for (struct_fields, 0..) |f, i| { if (f.name == field_name_id) { - const ptr = self.builder.structGepTyped(obj_ptr, @intCast(i), f.ty, obj_ty); + const ptr = self.builder.structGepTyped(obj_ptr, @intCast(i), self.module.types.ptrTo(f.ty), obj_ty); return .{ .ptr = ptr, .ty = f.ty }; } }