diff --git a/examples/0166-types-union-promoted-member-lvalue.sx b/examples/0166-types-union-promoted-member-lvalue.sx new file mode 100644 index 0000000..d48dfff --- /dev/null +++ b/examples/0166-types-union-promoted-member-lvalue.sx @@ -0,0 +1,32 @@ +// Taking the address of a promoted anonymous-struct union member yields a +// pointer to that member's slot, so mutating through the pointer is visible +// through the member. The write path (`v.x = 41`) and the read path already +// resolve promoted members; the lvalue-pointer path (`@v.x`) now resolves them +// too, via the shared field-lvalue resolver. +// +// Regression (issue 0094, attempt 2): lowerExprAsPtr's union branch handled +// only DIRECT union field names, so `@v.x` on a promoted member reported +// "field 'x' not found on type 'Vec2'" even though `v.x = 41` worked. The +// over-rejection is gone, and a member that is NOT at offset 0 (`v.y`) resolves +// to its own slot — not a default field 0. + +#import "modules/std.sx"; + +Vec2 :: union { + data: [2]s64; + struct { x: s64; y: s64; }; +} + +bump :: (p: *s64) { + p.* = p.* + 1; +} + +main :: () { + v : Vec2 = ---; + v.x = 41; + v.y = 100; + bump(@v.x); // promoted member at offset 0 → 42 + bump(@v.y); // promoted member at offset 8 → 101 (its own slot) + print("x={}\n", v.x); + print("y={}\n", v.y); +} diff --git a/examples/1145-diagnostics-missing-struct-field-assign.sx b/examples/1145-diagnostics-missing-struct-field-assign.sx index 5740eb6..b8dffa8 100644 --- a/examples/1145-diagnostics-missing-struct-field-assign.sx +++ b/examples/1145-diagnostics-missing-struct-field-assign.sx @@ -1,13 +1,15 @@ // Assigning to a field that does not exist on a struct produces the same // `field 'X' not found on type 'Y'` diagnostic as the read path (1100), and -// exits 1 — never the `.unresolved` LLVM-emission panic. +// exits 1 — never the `.unresolved` LLVM-emission panic, never a silent store +// into a neighbouring field. // // Regression (issue 0094): the lvalue field lookup left `field_ty = .unresolved` // (lowerAssignment's assignment-target path) or silently GEP'd field 0 as `.s64` -// (lowerExprAsPtr's fallback), so a missing-field store built a -// pointer-to-`.unresolved` that panicked at LLVM emission. Both the -// assignment-target path (`p.q`) and the nested lvalue-pointer path -// (`o.missing.a`) now emit the field-not-found diagnostic. +// (lowerExprAsPtr's fallback / lowerMultiAssign's struct loop), so a missing-field +// store either built a pointer-to-`.unresolved` that panicked at LLVM emission or +// silently wrote field 0. All three lvalue sites now emit the field-not-found +// diagnostic: the assignment-target path (`p.q`), the nested lvalue-pointer path +// (`o.missing.a`), and the multi-target store path (`p.r, y`). Point :: struct { x: s64; } Inner :: struct { a: s64; } @@ -20,5 +22,8 @@ main :: () -> s32 { o := Outer.{ inner = Inner.{ a = 1 } }; o.missing.a = 5; // site 2: lowerExprAsPtr fallback + y : s64 = 0; + p.r, y = 3, 4; // site 3: lowerMultiAssign field path + return 0; } diff --git a/examples/expected/0166-types-union-promoted-member-lvalue.exit b/examples/expected/0166-types-union-promoted-member-lvalue.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/examples/expected/0166-types-union-promoted-member-lvalue.exit @@ -0,0 +1 @@ +0 diff --git a/examples/expected/0166-types-union-promoted-member-lvalue.stderr b/examples/expected/0166-types-union-promoted-member-lvalue.stderr new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/0166-types-union-promoted-member-lvalue.stderr @@ -0,0 +1 @@ + diff --git a/examples/expected/0166-types-union-promoted-member-lvalue.stdout b/examples/expected/0166-types-union-promoted-member-lvalue.stdout new file mode 100644 index 0000000..29da7e4 --- /dev/null +++ b/examples/expected/0166-types-union-promoted-member-lvalue.stdout @@ -0,0 +1,2 @@ +x=42 +y=101 diff --git a/examples/expected/1145-diagnostics-missing-struct-field-assign.stderr b/examples/expected/1145-diagnostics-missing-struct-field-assign.stderr index 239852e..c561556 100644 --- a/examples/expected/1145-diagnostics-missing-struct-field-assign.stderr +++ b/examples/expected/1145-diagnostics-missing-struct-field-assign.stderr @@ -1,11 +1,17 @@ error: field 'q' not found on type 'Point' - --> examples/1145-diagnostics-missing-struct-field-assign.sx:18:5 + --> examples/1145-diagnostics-missing-struct-field-assign.sx:20:5 | -18 | p.q = 2; // site 1: lowerAssignment target path +20 | p.q = 2; // site 1: lowerAssignment target path | ^^^ error: field 'missing' not found on type 'Outer' - --> examples/1145-diagnostics-missing-struct-field-assign.sx:21:5 + --> examples/1145-diagnostics-missing-struct-field-assign.sx:23:5 | -21 | o.missing.a = 5; // site 2: lowerExprAsPtr fallback +23 | o.missing.a = 5; // site 2: lowerExprAsPtr fallback | ^^^^^^^^^ + +error: field 'r' not found on type 'Point' + --> examples/1145-diagnostics-missing-struct-field-assign.sx:26:5 + | +26 | p.r, y = 3, 4; // site 3: lowerMultiAssign field path + | ^^^ diff --git a/issues/0094-missing-struct-field-assignment-unresolved-panic.md b/issues/0094-missing-struct-field-assignment-unresolved-panic.md index 73f8793..e2bb586 100644 --- a/issues/0094-missing-struct-field-assignment-unresolved-panic.md +++ b/issues/0094-missing-struct-field-assignment-unresolved-panic.md @@ -10,26 +10,36 @@ > on a miss it returned `structGepTyped(obj_ptr, 0, .s64, obj_ty)` — a silent > field-0/`.s64` default. > -> **Fix (`src/ir/lower.zig`):** -> 1. `lowerAssignment` `.field_access` target — track a `found` flag over the -> struct-field loop; on a miss, emit the read path's field-not-found -> diagnostic (`emitFieldError`) and bail, never constructing -> `ptrTo(.unresolved)`. -> 2. `lowerExprAsPtr` `.field_access` — resolve union/tagged-union fields via -> `union_gep` (mirroring the write path; the old `.s64` fallback was silently -> standing in for union field access), then the struct-field loop, then -> `emitFieldError` on a genuine miss. The `.s64` sentinel is gone. +> **Fix (`src/ir/lower.zig`):** all three lvalue field-store sites — single +> 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. +> 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. +> 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. > -> Both sites now reuse `emitFieldError` (the exact facility the read path +> All sites reuse `emitFieldError` (the exact facility the read path > `lowerFieldAccessOnType` uses), so the read and write paths reject identically. -> The `types.zig` tripwire is untouched — the fix is to never produce -> `.unresolved` for a missing-field store. +> The `src/backend/llvm/types.zig` tripwire is untouched — the fix is to never +> produce `.unresolved` for a missing-field store. > > **Regression tests:** `examples/1145-diagnostics-missing-struct-field-assign.sx` -> (negative — both sites error, exit 1), `examples/0165-types-nested-struct-field-assign.sx` -> (positive — nested struct field write + address-of a matched field still work), -> and a lowering unit test in `src/ir/lower.test.zig` -> ("assigning to a missing struct field emits field-not-found, no panic"). +> (negative — single-assign, address-of, AND multi-assign missing-field all error, +> 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). ## 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 7fec684..78ab31f 100644 --- a/src/ir/lower.test.zig +++ b/src/ir/lower.test.zig @@ -1121,6 +1121,59 @@ test "lower: assigning to a missing struct field emits field-not-found, no panic try std.testing.expect(found); } +test "lower: multi-assign to a missing struct field emits field-not-found, no corruption (issue 0094)" { + 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(); + var diags = errors.DiagnosticList.init(alloc, "", "test.sx"); + defer diags.deinit(); + + // Register `Point :: struct { x: s64; }` so the struct literal resolves. + const fields = [_]ir_mod.types.TypeInfo.StructInfo.Field{ + .{ .name = module.types.internString("x"), .ty = .s64 }, + }; + _ = module.types.intern(.{ .@"struct" = .{ .name = module.types.internString("Point"), .fields = &fields } }); + + const span = ast.Span{ .start = 0, .end = 0 }; + + // main :: () { p := Point.{ x = 1 }; y := 0; p.r, y = 3, 4; } — `r` is not a field of Point. + var x_val = Node{ .span = span, .data = .{ .int_literal = .{ .value = 1 } } }; + const field_inits = [_]ast.StructFieldInit{.{ .name = "x", .value = &x_val }}; + var lit = Node{ .span = span, .data = .{ .struct_literal = .{ .struct_name = "Point", .field_inits = &field_inits } } }; + var decl = Node{ .span = span, .data = .{ .var_decl = .{ .name = "p", .name_span = span, .type_annotation = null, .value = &lit } } }; + + var y_init = Node{ .span = span, .data = .{ .int_literal = .{ .value = 0 } } }; + var y_decl = Node{ .span = span, .data = .{ .var_decl = .{ .name = "y", .name_span = span, .type_annotation = null, .value = &y_init } } }; + + var p_ident = Node{ .span = span, .data = .{ .identifier = .{ .name = "p" } } }; + var target0 = Node{ .span = span, .data = .{ .field_access = .{ .object = &p_ident, .field = "r" } } }; + var target1 = Node{ .span = span, .data = .{ .identifier = .{ .name = "y" } } }; + var v0 = Node{ .span = span, .data = .{ .int_literal = .{ .value = 3 } } }; + var v1 = Node{ .span = span, .data = .{ .int_literal = .{ .value = 4 } } }; + const targets = [_]*Node{ &target0, &target1 }; + const values = [_]*Node{ &v0, &v1 }; + var massign = Node{ .span = span, .data = .{ .multi_assign = .{ .targets = &targets, .values = &values } } }; + + const stmts = [_]*Node{ &decl, &y_decl, &massign }; + var body = Node{ .span = span, .data = .{ .block = .{ .stmts = &stmts } } }; + const fd = ast.FnDecl{ .name = "main", .params = &.{}, .return_type = null, .body = &body }; + + var lowering = Lowering.init(&module); + lowering.diagnostics = &diags; + // Pre-fix the struct-only loop defaulted field_idx 0 / field_ty .unresolved on + // a miss, silently storing into field 0 (no diagnostic); the fix resolves the + // target via the shared fieldLvaluePtr and bails with field-not-found. + lowering.lowerFunction(&fd, "main", false); + + var found = false; + for (diags.items.items) |d| { + if (d.level == .err and std.mem.indexOf(u8, d.message, "field 'r' not found on type 'Point'") != null) 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 88b24c9..3907ef3 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2564,6 +2564,86 @@ pub const Lowering = struct { } } + const FieldLvalue = struct { ptr: Ref, ty: TypeId }; + + /// 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 + /// 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; + const field_name_id = self.module.types.internString(field); + const type_info = self.module.types.get(obj_ty); + + // Union / tagged-union: variants overlay at offset 0. A direct field is + // a union_gep; a promoted anonymous-struct member is a union_gep into + // the variant followed by a struct_gep into the member. + 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 ptr = self.builder.emit(.{ .union_gep = .{ .base = obj_ptr, .field_index = @intCast(i), .base_type = obj_ty } }, self.module.types.ptrTo(f.ty)); + return .{ .ptr = ptr, .ty = f.ty }; + } + 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) { + 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); + return .{ .ptr = ptr, .ty = sf.ty }; + } + } + } + } + } + return null; + } + + // Tuple element: `.0` (numeric) or `.name`. + if (type_info == .tuple) { + const tup = type_info.tuple; + var elem_idx: ?usize = null; + if (std.fmt.parseInt(usize, 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 ptr = self.builder.structGepTyped(obj_ptr, @intCast(idx), elem_ty, obj_ty); + return .{ .ptr = ptr, .ty = elem_ty }; + } + return null; + } + + // 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); + return .{ .ptr = ptr, .ty = f.ty }; + } + } + return null; + } + /// Get the pointer (alloca ref) for an lvalue expression, without loading. fn lowerExprAsPtr(self: *Lowering, node: *const Node) Ref { switch (node.data) { @@ -2608,40 +2688,15 @@ pub const Lowering = struct { obj_ty = info.pointer.pointee; } } - const field_name_id = self.module.types.internString(fa.field); - - // Union / tagged-union field address: all variants overlay at - // offset 0, so the lvalue pointer is a union_gep — mirrors the - // write path (lowerAssignment) so the lvalue-pointer and the store - // resolve the same field index. A non-struct aggregate would - // otherwise miss the struct-field loop below and fall through. - 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) { - return self.builder.emit(.{ .union_gep = .{ .base = obj_ptr, .field_index = @intCast(i), .base_type = obj_ty } }, self.module.types.ptrTo(f.ty)); - } - } - } - } - - const struct_fields = self.getStructFields(obj_ty); - for (struct_fields, 0..) |f, i| { - if (f.name == field_name_id) { - return self.builder.structGepTyped(obj_ptr, @intCast(i), f.ty, obj_ty); - } - } - // No struct/union field matches — emit the read path's - // field-not-found diagnostic (lowerFieldAccessOnType → emitFieldError) - // instead of silently GEPing field 0 as .s64. That bogus pointer - // mislowers the lvalue and reaches LLVM emission as - // ptrTo(.unresolved), panicking (issue 0094). + // Resolve the field lvalue (struct / union direct / promoted + // anonymous-struct member / tuple element) via the shared + // resolver so address-of and the multi-target store path never + // disagree on the slot. No match → emit the read path's + // field-not-found diagnostic (lowerFieldAccessOnType → + // emitFieldError) instead of silently GEPing field 0 as .s64; + // that bogus pointer reaches LLVM emission as ptrTo(.unresolved) + // and panics (issue 0094). + if (self.fieldLvaluePtr(obj_ptr, obj_ty, fa.field)) |r| return r.ptr; return self.emitFieldError(obj_ty, fa.field, node.span); }, .index_expr => |ie| { @@ -8776,24 +8831,21 @@ pub const Lowering = struct { .field_access => |fa| { const obj_ptr = self.lowerExprAsPtr(fa.object); const obj_ty = self.inferExprType(fa.object); - const field_name_id = self.module.types.internString(fa.field); - const struct_fields = self.getStructFields(obj_ty); - var field_idx: u32 = 0; - var field_ty: TypeId = .unresolved; - for (struct_fields, 0..) |f, fi| { - if (f.name == field_name_id) { - field_idx = @intCast(fi); - field_ty = f.ty; - break; - } + // Resolve the target field via the shared lvalue resolver — + // the same one address-of uses — so a missing field emits a + // diagnostic instead of defaulting to field 0 / field_ty + // .unresolved, which silently corrupted a neighbouring field + // (or panicked at LLVM emission) (issue 0094). + if (self.fieldLvaluePtr(obj_ptr, obj_ty, fa.field)) |r| { + const val_ty = self.builder.getRefType(val); + const store_val = if (val_ty != r.ty and val_ty != .void and r.ty != .void) + self.coerceToType(val, val_ty, r.ty) + else + val; + self.builder.store(r.ptr, store_val); + } else { + _ = self.emitFieldError(obj_ty, fa.field, target.span); } - const gep = self.builder.structGepTyped(obj_ptr, field_idx, field_ty, obj_ty); - const val_ty = self.builder.getRefType(val); - const store_val = if (val_ty != field_ty and val_ty != .void and field_ty != .void) - self.coerceToType(val, val_ty, field_ty) - else - val; - self.builder.store(gep, store_val); }, .deref_expr => |de| { const ptr = self.lowerExpr(de.operand);