fix: struct-literal → optional coercion + #get through optional chain (issue 0160)
Two fixes for optional interactions surfaced by the #set/#get review. The
original issue 0160 mis-diagnosed (A) as an optional-chain bug; the chain works
fine for real fields. The actual bugs:
(A) A bare struct literal `.{ ... }` against an optional target `?T` was built
into the optional's {payload, has_value} layout instead of the inner T, then
re-wrapped — corrupting the value (a multi-field payload's first field clobbered
by the has_value flag, or a `?T` arg silently null) or failing LLVM
verification. lowerStructLiteral now builds the inner T, materializes it, and
wraps via coerceToType; lowerVarDecl's previously-UNCONDITIONAL optional wrap is
guarded so an already-`?T` value isn't double-wrapped. Fixed across var-decl,
arg, return, nested field, reassignment, and array-element contexts.
(B) `#get` accessors are now reachable through an optional chain (`obj?.getter`):
lowerOptionalChain dispatches the getter via a synthetic receiver, and
expr_typer types `obj?.getter` through a shared getterReturnTypeOnDeref helper
(handles `?T` and `?*T`, value and pointer optionals, and generic-instance
getters like List.len). The `#set` write side through `?.` is intentionally left
matching real-field behavior (optional-chain assignment unsupported).
Regression tests: examples/optionals/0906 (struct-literal → optional) and 0907
(accessor through chain). issues/0160 marked RESOLVED with the corrected root
cause.
This commit is contained in:
@@ -0,0 +1,41 @@
|
||||
// A bare struct literal `.{ ... }` against an optional target `?T` builds the
|
||||
// inner `T` and wraps it once — in every caller context: var-decl, function
|
||||
// argument, return value, a nested `?T` struct field, reassignment, and an
|
||||
// array element. Previously this filled the optional's {payload, has_value}
|
||||
// layout directly, corrupting the value (a multi-field payload's first field
|
||||
// was clobbered by the has_value flag, or a `?T` arg silently read as null) or
|
||||
// failing LLVM verification on a double wrap.
|
||||
// Regression (issue 0160).
|
||||
#import "modules/std.sx";
|
||||
|
||||
T :: struct { a: i64 = 0; b: i64 = 0; }
|
||||
Outer :: struct { opt: ?T = null; tag: i64 = 0; }
|
||||
|
||||
take :: (o: ?T) -> i64 { if o != null { return o!.a * 10 + o!.b; } return -1; }
|
||||
make :: () -> ?T { return .{ a = 5, b = 6 }; }
|
||||
|
||||
main :: () -> i64 {
|
||||
// var-decl
|
||||
v : ?T = .{ a = 3, b = 9 };
|
||||
if v != null { print("var: {} {}\n", v!.a, v!.b); } // 3 9
|
||||
|
||||
// function argument
|
||||
print("arg: {}\n", take(.{ a = 4, b = 7 })); // 47
|
||||
|
||||
// return value
|
||||
r := make();
|
||||
if r != null { print("ret: {} {}\n", r!.a, r!.b); } // 5 6
|
||||
|
||||
// nested ?T struct field
|
||||
o : Outer = .{ opt = .{ a = 1, b = 2 }, tag = 8 };
|
||||
if o.opt != null { print("nested: {} {} {}\n", o.opt!.a, o.opt!.b, o.tag); } // 1 2 8
|
||||
|
||||
// reassignment
|
||||
v = .{ a = 11, b = 12 };
|
||||
if v != null { print("reassign: {} {}\n", v!.a, v!.b); } // 11 12
|
||||
|
||||
// array element
|
||||
arr : [2]?T = .[ .{ a = 20 }, .{ a = 21 } ];
|
||||
if arr[0] != null { if arr[1] != null { print("arr: {} {}\n", arr[0]!.a, arr[1]!.a); } } // 20 21
|
||||
return 0;
|
||||
}
|
||||
39
examples/optionals/0907-optionals-accessor-through-chain.sx
Normal file
39
examples/optionals/0907-optionals-accessor-through-chain.sx
Normal file
@@ -0,0 +1,39 @@
|
||||
// A `#get` property accessor is reachable through an optional chain
|
||||
// (`obj?.getter`): the some-branch dispatches the getter and the result is
|
||||
// re-wrapped as `?R`; a null receiver short-circuits to null. Works for a value
|
||||
// optional (`?T`), a pointer optional (`?*T`), and a generic-instance getter
|
||||
// (`List.len`), and types correctly without an explicit annotation. Real fields
|
||||
// through `?.` keep working unchanged.
|
||||
// Regression (issue 0160).
|
||||
#import "modules/std.sx";
|
||||
|
||||
Temp :: struct {
|
||||
raw: i64 = 0;
|
||||
doubled :: (self: *Temp) -> i64 #get => self.raw * 2;
|
||||
}
|
||||
|
||||
main :: () -> i64 {
|
||||
t : Temp = .{ raw = 4 };
|
||||
|
||||
// value optional ?T — getter through chain
|
||||
ot : ?Temp = t;
|
||||
print("?T getter: {}\n", ot?.doubled ?? -1); // 8
|
||||
|
||||
// pointer optional ?*T — getter through chain
|
||||
pt : ?*Temp = @t;
|
||||
print("?*T getter: {}\n", pt?.doubled ?? -1); // 8
|
||||
|
||||
// null receiver short-circuits
|
||||
nope : ?Temp = null;
|
||||
print("null getter: {}\n", nope?.doubled ?? -1); // -1
|
||||
|
||||
// real field through chain still works
|
||||
print("?T field: {}\n", ot?.raw ?? -1); // 4
|
||||
|
||||
// generic-instance getter (List.len) through chain
|
||||
xs : List(i64) = .{};
|
||||
xs.append(10); xs.append(20); xs.append(30);
|
||||
pxs : ?*List(i64) = @xs;
|
||||
print("?*List len: {}\n", pxs?.len ?? -1); // 3
|
||||
return 0;
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
var: 3 9
|
||||
arg: 47
|
||||
ret: 5 6
|
||||
nested: 1 2 8
|
||||
reassign: 11 12
|
||||
arr: 20 21
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
?T getter: 8
|
||||
?*T getter: 8
|
||||
null getter: -1
|
||||
?T field: 4
|
||||
?*List len: 3
|
||||
@@ -1,5 +1,23 @@
|
||||
# 0160 — optional-chain field access: `?T` value-optional read miscompiles, and `#get`/`#set` accessors aren't reached through `?.`
|
||||
|
||||
> **RESOLVED.** Root cause was MIS-DIAGNOSED in the original writeup below: (A)
|
||||
> was NOT an optional-chain bug — the chain works fine for real fields. The real
|
||||
> bug was **struct-literal → optional coercion**: a bare `.{ ... }` against a
|
||||
> `?T` target was built into the optional's `{payload, has_value}` layout
|
||||
> instead of building the inner `T` and wrapping it. Fix:
|
||||
> `lowerStructLiteral` (`src/ir/lower/expr.zig`) now builds the inner `T`,
|
||||
> materializes it, and wraps via `coerceToType`; and `lowerVarDecl`
|
||||
> (`src/ir/lower/stmt.zig`) only wraps when the value is not already the target
|
||||
> optional (its previous UNCONDITIONAL wrap double-wrapped any already-`?T`
|
||||
> value). (B) the `#get`-through-`?.` gap was real and is fixed:
|
||||
> `lowerOptionalChain` dispatches the getter via a synthetic receiver, and
|
||||
> `expr_typer` types `obj?.getter` through a shared `getterReturnTypeOnDeref`
|
||||
> helper (handles `?T` and `?*T`). The `#set` write side through `?.` is
|
||||
> intentionally left matching real-field behavior (optional-chain assignment is
|
||||
> unsupported for all fields) and was never part of this bug. Regression tests:
|
||||
> `examples/optionals/0906-optionals-struct-literal-into-optional.sx` and
|
||||
> `examples/optionals/0907-optionals-accessor-through-chain.sx`.
|
||||
|
||||
## Symptom
|
||||
|
||||
Two related gaps in optional-chain field access (`obj?.field`), surfaced while
|
||||
|
||||
@@ -259,6 +259,19 @@ pub const ExprTyper = struct {
|
||||
for (fields) |f| {
|
||||
if (f.name == field_name_id) return if (is_opt_chain) self.l.optionalOfFlattened(f.ty) else f.ty;
|
||||
}
|
||||
// `#get` property accessor through an optional chain
|
||||
// (`obj?.getter`): resolve the getter on the dereferenced
|
||||
// inner type via a synthetic `*T` receiver, so a value
|
||||
// optional (`?T`) — whose receiver would otherwise be the
|
||||
// optional itself — and a pointer optional (`?*T`) both type
|
||||
// exactly as `lowerOptionalChain` emits (issue 0160).
|
||||
if (is_opt_chain) {
|
||||
var deref_inner = obj_ty;
|
||||
if (!deref_inner.isBuiltin() and self.l.module.types.get(deref_inner) == .pointer)
|
||||
deref_inner = self.l.module.types.get(deref_inner).pointer.pointee;
|
||||
if (self.l.getterReturnTypeOnDeref(deref_inner, fa.field)) |rt|
|
||||
return self.l.optionalOfFlattened(rt);
|
||||
}
|
||||
// `#get` property accessor: type as the accessor's return
|
||||
// type. Resolve via the synthesized no-arg call so generic
|
||||
// bindings (e.g. a `List(T)` getter returning `T`) resolve
|
||||
|
||||
@@ -1972,6 +1972,7 @@ pub const Lowering = struct {
|
||||
pub const getStructFields = lower_expr.getStructFields;
|
||||
pub const getAccessorFor = lower_expr.getAccessorFor;
|
||||
pub const getSetterFor = lower_expr.getSetterFor;
|
||||
pub const getterReturnTypeOnDeref = lower_expr.getterReturnTypeOnDeref;
|
||||
pub const fixupMethodReceiver = lower_expr.fixupMethodReceiver;
|
||||
pub const getStructTypeName = lower_expr.getStructTypeName;
|
||||
pub const builtinTypeName = lower_expr.builtinTypeName;
|
||||
|
||||
@@ -95,6 +95,34 @@ pub fn lowerStructLiteral(self: *Lowering, sl: *const ast.StructLiteral, span: a
|
||||
return self.lowerUnionLiteral(sl, ty, span);
|
||||
}
|
||||
|
||||
// A bare struct literal against an optional target `?T` builds the INNER
|
||||
// `T` and wraps it once. Without this `ty` is the optional itself, so the
|
||||
// literal is lowered into the optional's `{payload, has_value}` layout and
|
||||
// then re-wrapped — corrupting the value (a `?T` arg silently reads as
|
||||
// null) or failing LLVM verification on the double wrap (issue 0160).
|
||||
// Only the bare `.{ ... }` form reaches here with an optional `ty` (a named
|
||||
// / generic literal resolves `ty` from its own type, never the target).
|
||||
if (!ty.isBuiltin() and self.module.types.get(ty) == .optional) {
|
||||
const child = self.module.types.get(ty).optional.child;
|
||||
// Build the inner `T` (targeting it so nested literals resolve), then
|
||||
// wrap to `?T`. Building into `ty` (the optional) directly would fill
|
||||
// its {payload, has_value} layout and corrupt the value / fail LLVM
|
||||
// verification. Wrapping the raw struct_init SSA aggregate ALSO mislays
|
||||
// a multi-field payload, so round-trip through memory first — the wrap
|
||||
// then sees a loaded value, the same shape the working `T -> ?T` value
|
||||
// coercion wraps. Returning a fully-built `?T` makes EVERY caller
|
||||
// context correct, including array/struct-literal element slots that
|
||||
// don't re-coerce (issue 0160).
|
||||
const saved_tt = self.target_type;
|
||||
self.target_type = child;
|
||||
const inner = self.lowerStructLiteral(sl, span);
|
||||
self.target_type = saved_tt;
|
||||
const slot = self.builder.alloca(child);
|
||||
self.builder.store(slot, inner);
|
||||
const reloaded = self.builder.load(slot, child);
|
||||
return self.coerceToType(reloaded, child, ty);
|
||||
}
|
||||
|
||||
// Get struct field types for coercion and ordering
|
||||
const struct_fields = self.getStructFields(ty);
|
||||
|
||||
@@ -802,8 +830,37 @@ pub fn lowerOptionalChain(self: *Lowering, obj: Ref, fa: *const ast.FieldAccess,
|
||||
break :blk if (info == .optional) info.optional.child else obj_ty;
|
||||
} else obj_ty;
|
||||
|
||||
// Get the field type on the inner type
|
||||
const field_ty = self.resolveFieldType(inner_ty, fa.field);
|
||||
// `#get` accessor through `?.`: if the unwrapped (and pointer-deref'd)
|
||||
// receiver has a getter for this field, the some-branch dispatches the
|
||||
// getter instead of a struct-field read. A synthetic receiver local (typed
|
||||
// `inner_ty`) lets the existing getter intercept in `lowerFieldAccess` do
|
||||
// the deref / address-of; we bind it type-only here for the return-type
|
||||
// query, then fill its ref in the some-branch (issue 0160).
|
||||
var deref_inner = inner_ty;
|
||||
if (!deref_inner.isBuiltin() and self.module.types.get(deref_inner) == .pointer)
|
||||
deref_inner = self.module.types.get(deref_inner).pointer.pointee;
|
||||
const getter_recv: ?[]const u8 = if (self.scope != null and self.getAccessorFor(deref_inner, fa.field) != null) blk: {
|
||||
var buf: [40]u8 = undefined;
|
||||
const nm = std.fmt.bufPrint(&buf, "$oc_recv_{d}", .{self.block_counter}) catch "$oc_recv";
|
||||
self.block_counter += 1;
|
||||
const owned = self.alloc.dupe(u8, nm) catch break :blk null;
|
||||
// Type-only binding for the return-type query: type it as a POINTER to
|
||||
// the struct so inference routes through the (working) pointer-deref
|
||||
// getter path for both `?T` and `?*T`. The some-branch re-binds it to
|
||||
// the actual unwrapped receiver before lowering.
|
||||
self.scope.?.put(owned, .{ .ref = Ref.none, .ty = self.module.types.ptrTo(deref_inner), .is_alloca = false });
|
||||
break :blk owned;
|
||||
} else null;
|
||||
const read_node: ?*Node = if (getter_recv) |nm| blk: {
|
||||
const id = self.alloc.create(Node) catch break :blk null;
|
||||
id.* = .{ .span = span, .data = .{ .identifier = .{ .name = nm } } };
|
||||
const rn = self.alloc.create(Node) catch break :blk null;
|
||||
rn.* = .{ .span = span, .data = .{ .field_access = .{ .object = id, .field = fa.field } } };
|
||||
break :blk rn;
|
||||
} else null;
|
||||
|
||||
// Get the field type on the inner type (the getter's return type, if any).
|
||||
const field_ty = if (read_node) |rn| self.inferExprType(rn) else self.resolveFieldType(inner_ty, fa.field);
|
||||
// If field is already optional, flatten (don't double-wrap)
|
||||
const field_already_optional = if (!field_ty.isBuiltin()) self.module.types.get(field_ty) == .optional else false;
|
||||
const result_ty = if (field_already_optional) field_ty else self.module.types.optionalOf(field_ty);
|
||||
@@ -821,7 +878,24 @@ pub fn lowerOptionalChain(self: *Lowering, obj: Ref, fa: *const ast.FieldAccess,
|
||||
// Some: unwrap, access field (already ?FieldType if flattened, else wrap)
|
||||
self.builder.switchToBlock(some_bb);
|
||||
const unwrapped = self.builder.emit(.{ .optional_unwrap = .{ .operand = obj } }, inner_ty);
|
||||
const field_val = self.lowerFieldAccessOnType(unwrapped, inner_ty, fa.field, span);
|
||||
const field_val = if (read_node) |rn| blk: {
|
||||
// Re-bind the synthetic receiver to the unwrapped value, then dispatch
|
||||
// the getter through the normal field-access intercept. A `?*T` unwraps
|
||||
// to the pointer receiver directly; a `?T` unwraps to a value that the
|
||||
// getter (`self: *T`) needs to address, so materialize it into an alloca
|
||||
// and bind it like an ordinary `T` local (is_alloca).
|
||||
if (!inner_ty.isBuiltin() and self.module.types.get(inner_ty) == .pointer) {
|
||||
self.scope.?.put(getter_recv.?, .{ .ref = unwrapped, .ty = inner_ty, .is_alloca = false });
|
||||
} else {
|
||||
// Materialize the value and bind the alloca POINTER as a `*T` value
|
||||
// (not an is_alloca `T`), so the receiver path is identical to the
|
||||
// `?*T` case — a plain pointer receiver the getter intercept derefs.
|
||||
const slot = self.builder.alloca(inner_ty);
|
||||
self.builder.store(slot, unwrapped);
|
||||
self.scope.?.put(getter_recv.?, .{ .ref = slot, .ty = self.module.types.ptrTo(inner_ty), .is_alloca = false });
|
||||
}
|
||||
break :blk self.lowerExpr(rn);
|
||||
} else self.lowerFieldAccessOnType(unwrapped, inner_ty, fa.field, span);
|
||||
const some_result = if (field_already_optional) field_val else self.builder.emit(.{ .optional_wrap = .{ .operand = field_val } }, result_ty);
|
||||
self.builder.br(merge_bb, &.{some_result});
|
||||
|
||||
@@ -886,6 +960,29 @@ pub fn getAccessorFor(self: *Lowering, ty: TypeId, field: []const u8) ?*const as
|
||||
/// effective `field$set` name (so a same-name `#get` keeps the plain `field`),
|
||||
/// and a REAL field of the same name wins over it (parallels the `#get` rule).
|
||||
/// `ty` must be the dereferenced (non-pointer) receiver type.
|
||||
/// The return type of a `#get` accessor named `field` on `deref_ty` (a
|
||||
/// dereferenced struct type), or null when there is no such getter (or no scope
|
||||
/// to resolve through). Resolves the type the SAME way a real read does — via a
|
||||
/// synthetic `*deref_ty` receiver local routed through inference — so a generic
|
||||
/// instance getter (`List(T).len`) binds its type parameter exactly as the call
|
||||
/// path would. Shared by `lowerOptionalChain` and the optional-chain inference
|
||||
/// in `expr_typer`, so `obj?.getter` types identically to how it lowers.
|
||||
pub fn getterReturnTypeOnDeref(self: *Lowering, deref_ty: TypeId, field: []const u8) ?TypeId {
|
||||
if (deref_ty.isBuiltin()) return null;
|
||||
if (self.getAccessorFor(deref_ty, field) == null) return null;
|
||||
const s = self.scope orelse return null;
|
||||
var buf: [40]u8 = undefined;
|
||||
const nm = std.fmt.bufPrint(&buf, "$oc_ty_{d}", .{self.block_counter}) catch "$oc_ty";
|
||||
self.block_counter += 1;
|
||||
const owned = self.alloc.dupe(u8, nm) catch return null;
|
||||
s.put(owned, .{ .ref = Ref.none, .ty = self.module.types.ptrTo(deref_ty), .is_alloca = false });
|
||||
const id = self.alloc.create(Node) catch return null;
|
||||
id.* = .{ .span = .{ .start = 0, .end = 0 }, .data = .{ .identifier = .{ .name = owned } } };
|
||||
const rn = self.alloc.create(Node) catch return null;
|
||||
rn.* = .{ .span = .{ .start = 0, .end = 0 }, .data = .{ .field_access = .{ .object = id, .field = field } } };
|
||||
return self.inferExprType(rn);
|
||||
}
|
||||
|
||||
pub fn getSetterFor(self: *Lowering, ty: TypeId, field: []const u8) ?*const ast.FnDecl {
|
||||
if (ty.isBuiltin()) return null;
|
||||
// A REAL field of this name wins over a same-name `#set` (a setter must not
|
||||
|
||||
@@ -308,9 +308,13 @@ pub fn lowerVarDecl(self: *Lowering, vd: *const ast.VarDecl) void {
|
||||
self.target_type = saved_target;
|
||||
self.force_block_value = saved_fbv;
|
||||
// If target is optional and value isn't null, wrap with optional_wrap
|
||||
// — UNLESS the value is already that optional (e.g. a `?T`-returning
|
||||
// call, or a struct literal that lowered straight to `?T`); wrapping
|
||||
// again would build a `??`-shaped value typed `?T` and corrupt it /
|
||||
// fail LLVM verification (issue 0160).
|
||||
if (!ty.isBuiltin()) {
|
||||
const ty_info = self.module.types.get(ty);
|
||||
if (ty_info == .optional and val.data != .null_literal) {
|
||||
if (ty_info == .optional and val.data != .null_literal and self.builder.getRefType(ref) != ty) {
|
||||
ref = self.builder.optionalWrap(ref, ty);
|
||||
} else if (ty_info == .slice) {
|
||||
// Array → slice promotion: if value is an array, convert to slice
|
||||
|
||||
Reference in New Issue
Block a user