fix: diagnose ?(?T) tuple-payload mismatch instead of malformed IR (issue 0165)
In type position (T) is a 1-tuple (specs.md:843), so ?(?i64) is optional(tuple(?i64)); assigning a bare ?i64 had coerceToType classify .none and pass the value through, then optionalWrap built a corrupt insertvalue that aborted the LLVM verifier. After coercing toward an optional's child, verify the coerced type equals the child type (stmt.zig decl-init + coerce.zig .optional_wrap); on mismatch emit a located diagnostic (tuple-specific note only when the child is a tuple). formatTypeName now renders tuples as (x: i64, y: i64). Regressions: optionals/0911 (nested optional via alias, round-trip), diagnostics/1195 (the mismatch diagnostic). Updated diagnostics/1101 + protocols/0414 goldens for the improved tuple type-name rendering. Verified by 3 adversarial reviews. Filed adjacent bug 0171 (?any child not canonicalized).
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
// Out-of-range tuple index produces a clear
|
// Out-of-range tuple index produces a clear
|
||||||
// `error: field 'N' not found on type 'tuple'` diagnostic and exit 1.
|
// `error: field 'N' not found on type '(i64, i64)'` diagnostic and exit 1.
|
||||||
|
|
||||||
main :: () -> i32 {
|
main :: () -> i32 {
|
||||||
t := (10, 20);
|
t := (10, 20);
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
// `?(?i64)` is `optional` wrapping the SINGLE-FIELD TUPLE `(?i64)` — in type
|
||||||
|
// position `(T)` is a 1-tuple, not a grouping (specs.md §"Tuple Types"). So
|
||||||
|
// assigning a bare `?i64` value to a `?(?i64)` slot is a type mismatch: the
|
||||||
|
// optional's payload is the tuple `(?i64)`, not `?i64`.
|
||||||
|
//
|
||||||
|
// Regression (issue 0165): this used to silently lower to a malformed
|
||||||
|
// `insertvalue { {{i64,i1}}, i1 }` that aborted the LLVM verifier. It now
|
||||||
|
// produces a clean diagnostic naming the payload type and pointing at the
|
||||||
|
// parens-are-a-tuple gotcha. (To write a genuine nested optional, alias the
|
||||||
|
// inner one: `Opt :: ?i64; x : ?Opt = ...` — see
|
||||||
|
// examples/optionals/0911-nested-optional-via-alias.sx.)
|
||||||
|
#import "modules/std.sx";
|
||||||
|
|
||||||
|
main :: () {
|
||||||
|
inner : ?i64 = 5;
|
||||||
|
outer : ?(?i64) = inner;
|
||||||
|
print("unreachable\n");
|
||||||
|
}
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
error: field '42' not found on type 'tuple'
|
error: field '42' not found on type '(i64, i64)'
|
||||||
--> examples/diagnostics/1101-diagnostics-err-tuple-oob.sx:6:15
|
--> examples/diagnostics/1101-diagnostics-err-tuple-oob.sx:6:15
|
||||||
|
|
|
|
||||||
6 | return xx t.42;
|
6 | return xx t.42;
|
||||||
|
|||||||
@@ -0,0 +1 @@
|
|||||||
|
1
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
error: cannot assign a value of type '?i64' to optional '?(?i64)': its payload type is '(?i64)' (note: in type position '(T)' is a single-field tuple, not a grouping — write the inner optional without parentheses)
|
||||||
|
--> examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx:16:3
|
||||||
|
|
|
||||||
|
16 | outer : ?(?i64) = inner;
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
|
||||||
29
examples/optionals/0911-nested-optional-via-alias.sx
Normal file
29
examples/optionals/0911-nested-optional-via-alias.sx
Normal file
@@ -0,0 +1,29 @@
|
|||||||
|
// Nested optional `?(?i64)` written via a type alias — `Opt :: ?i64; ?Opt`.
|
||||||
|
// The outer optional's payload is the inner optional, so the layout is the
|
||||||
|
// well-formed double-wrap `{ {i64,i1}, i1 }`. Assigning an inner `?i64`,
|
||||||
|
// unwrapping the outer to recover the inner `?i64`, and unwrapping that all
|
||||||
|
// round-trip cleanly.
|
||||||
|
//
|
||||||
|
// Regression (issue 0165): the type-table interning always produced the
|
||||||
|
// correct `{ {i64,i1}, i1 }` for a genuine nested optional — this locks that in.
|
||||||
|
#import "modules/std.sx";
|
||||||
|
|
||||||
|
Opt :: ?i64;
|
||||||
|
|
||||||
|
main :: () {
|
||||||
|
inner : ?i64 = 5;
|
||||||
|
outer : ?Opt = inner; // ?(?i64) — payload is the inner optional
|
||||||
|
|
||||||
|
print("outer present: {}\n", outer != null);
|
||||||
|
|
||||||
|
mid := outer!; // unwrap outer -> ?i64
|
||||||
|
print("mid present: {}\n", mid != null);
|
||||||
|
print("value: {}\n", mid!); // unwrap inner -> 5
|
||||||
|
|
||||||
|
// null-coalesce through the recovered inner optional
|
||||||
|
print("coalesced: {}\n", mid ?? 0);
|
||||||
|
|
||||||
|
// a none outer
|
||||||
|
empty : ?Opt = null;
|
||||||
|
print("empty present: {}\n", empty != null);
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
0
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
outer present: true
|
||||||
|
mid present: true
|
||||||
|
value: 5
|
||||||
|
coalesced: 5
|
||||||
|
empty present: false
|
||||||
@@ -1381,7 +1381,7 @@
|
|||||||
@str.1477 = private unnamed_addr constant [19 x i8] c"*__VL__i64__Vtable\00", align 1
|
@str.1477 = private unnamed_addr constant [19 x i8] c"*__VL__i64__Vtable\00", align 1
|
||||||
@str.1478 = private unnamed_addr constant [4 x i8] c"@0x\00", align 1
|
@str.1478 = private unnamed_addr constant [4 x i8] c"@0x\00", align 1
|
||||||
@str.1479 = private unnamed_addr constant [5 x i8] c"null\00", align 1
|
@str.1479 = private unnamed_addr constant [5 x i8] c"null\00", align 1
|
||||||
@str.1480 = private unnamed_addr constant [7 x i8] c"*tuple\00", align 1
|
@str.1480 = private unnamed_addr constant [11 x i8] c"*(VL__i64)\00", align 1
|
||||||
@str.1481 = private unnamed_addr constant [4 x i8] c"@0x\00", align 1
|
@str.1481 = private unnamed_addr constant [4 x i8] c"@0x\00", align 1
|
||||||
@str.1482 = private unnamed_addr constant [5 x i8] c"null\00", align 1
|
@str.1482 = private unnamed_addr constant [5 x i8] c"null\00", align 1
|
||||||
@str.1483 = private unnamed_addr constant [21 x i8] c"**Combined__i64__i64\00", align 1
|
@str.1483 = private unnamed_addr constant [21 x i8] c"**Combined__i64__i64\00", align 1
|
||||||
@@ -16197,7 +16197,7 @@ if.else.1256: ; preds = %entry
|
|||||||
%loadN = load i64, ptr %allocaN, align 8
|
%loadN = load i64, ptr %allocaN, align 8
|
||||||
%call = call { ptr, i64 } @int_to_hex_string(ptr %0, i64 %loadN)
|
%call = call { ptr, i64 } @int_to_hex_string(ptr %0, i64 %loadN)
|
||||||
%callN = call { ptr, i64 } @concat(ptr %0, { ptr, i64 } { ptr @str.1481, i64 3 }, { ptr, i64 } %call)
|
%callN = call { ptr, i64 } @concat(ptr %0, { ptr, i64 } { ptr @str.1481, i64 3 }, { ptr, i64 } %call)
|
||||||
%callN = call { ptr, i64 } @concat(ptr %0, { ptr, i64 } { ptr @str.1480, i64 6 }, { ptr, i64 } %callN)
|
%callN = call { ptr, i64 } @concat(ptr %0, { ptr, i64 } { ptr @str.1480, i64 10 }, { ptr, i64 } %callN)
|
||||||
br label %if.merge.1257
|
br label %if.merge.1257
|
||||||
|
|
||||||
if.merge.1257: ; preds = %if.else.1256, %if.then.1255
|
if.merge.1257: ; preds = %if.else.1256, %if.then.1255
|
||||||
|
|||||||
@@ -1,5 +1,22 @@
|
|||||||
# 0165 — parenthesized nested optional `?(?T)` resolves to a malformed double-wrapped type
|
# 0165 — parenthesized nested optional `?(?T)` resolves to a malformed double-wrapped type
|
||||||
|
|
||||||
|
> **RESOLVED.** The issue's premise was partly wrong: per `specs.md:843`, in
|
||||||
|
> TYPE position `(T)` is a single-field TUPLE, not a grouping — so `?(?i64)` is
|
||||||
|
> `optional(tuple(?i64))` and the compiler's `{ {{i64,i1}}, i1 }` layout was
|
||||||
|
> CORRECT. The real bug was a silent malformed-IR path: assigning a bare `?i64`
|
||||||
|
> to it had `coerceToType` classify `.none` and pass the value through unchanged,
|
||||||
|
> then `optionalWrap` built a corrupt `insertvalue` that aborted the LLVM
|
||||||
|
> verifier. Fix: after coercing toward an optional's child, verify the coerced
|
||||||
|
> value's type equals the child type (`src/ir/lower/stmt.zig` decl-init +
|
||||||
|
> `src/ir/lower/coerce.zig` `.optional_wrap`); on mismatch emit a located
|
||||||
|
> diagnostic (with a tuple-specific note only when the child is a tuple) instead
|
||||||
|
> of corrupt IR. `formatTypeName` now renders tuples as `(x: i64, y: i64)`.
|
||||||
|
> Genuine nested optionals via alias (`Opt :: ?i64; ?Opt`) work and round-trip.
|
||||||
|
> Regressions: `examples/optionals/0911-nested-optional-via-alias.sx`,
|
||||||
|
> `examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx`.
|
||||||
|
> Verified by 3 adversarial reviews. (Adjacent pre-existing bug found + filed:
|
||||||
|
> 0171 `?any` child not canonicalized.)
|
||||||
|
|
||||||
## Symptom
|
## Symptom
|
||||||
|
|
||||||
A nested optional written `?(?i64)` resolves to a spurious extra struct wrapper:
|
A nested optional written `?(?i64)` resolves to a spurious extra struct wrapper:
|
||||||
|
|||||||
47
issues/0171-optional-any-child-not-canonicalized.md
Normal file
47
issues/0171-optional-any-child-not-canonicalized.md
Normal file
@@ -0,0 +1,47 @@
|
|||||||
|
# 0171 — `?any` optional child is a non-canonical `any` TypeId (box-into-any rule misses, value silently discarded)
|
||||||
|
|
||||||
|
## Symptom
|
||||||
|
|
||||||
|
An optional whose child is `any` (`?any`) is broken. Baseline (before the issue
|
||||||
|
0165 fix) silently DISCARDED the boxed value: `x : ?any = 42; v := x!` yields an
|
||||||
|
empty box `any{}`, not `42` — the payload is lowered as a zero-size `{}`. After
|
||||||
|
the 0165 fix the same code now produces a clean type-mismatch diagnostic
|
||||||
|
(`cannot assign a value of type 'i64' to optional '?any': its payload type is
|
||||||
|
'any'`), which is strictly better than silent corruption but still means `?any`
|
||||||
|
does not work.
|
||||||
|
|
||||||
|
## Root cause (from adversarial review of issue 0165)
|
||||||
|
|
||||||
|
The box-into-`any` coercion rule (`src/ir/conversions.zig` ~line 57) keys on the
|
||||||
|
BUILTIN `.any` enum TypeId. But an optional's child `any` is a SEPARATELY
|
||||||
|
interned TypeId (observed `@enumFromInt(246)`, type-name `"any"`) that is NOT
|
||||||
|
identity-equal to the builtin `.any`. So `classify(i64, child_any)` falls through
|
||||||
|
to `.none`, returns the value unchanged (`i64`), and the wrap is invalid. The
|
||||||
|
`any` type is not being canonicalized to the builtin TypeId when it appears as an
|
||||||
|
optional child.
|
||||||
|
|
||||||
|
## Reproduction
|
||||||
|
|
||||||
|
```sx
|
||||||
|
#import "modules/std.sx";
|
||||||
|
main :: () {
|
||||||
|
x : ?any = 42;
|
||||||
|
v := x!;
|
||||||
|
print("{}\n", v); // expected: a boxed 42; baseline yields empty any{}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Investigation prompt
|
||||||
|
|
||||||
|
Canonicalize `any` as an optional child (and likely any other compound position)
|
||||||
|
to the builtin `.any` TypeId at type-resolution/interning time, so the
|
||||||
|
box-into-any rule in `src/ir/conversions.zig` classifies correctly and `?any`
|
||||||
|
round-trips. Find where the optional child type is resolved/interned
|
||||||
|
(`src/ir/types.zig` `optionalOf` / the type resolver) and ensure an `any` child
|
||||||
|
maps to the canonical builtin TypeId rather than a fresh interned copy.
|
||||||
|
Alternatively, make the box-into-any classifier compare by type-KIND
|
||||||
|
(`info == .any`) rather than TypeId identity — but canonicalization is the more
|
||||||
|
robust fix (it also fixes `==`, `size_of`, and any other identity check on the
|
||||||
|
`any` child). Verify the repro round-trips a boxed value; add an
|
||||||
|
`examples/types/01xx-optional-any.sx` regression. Low priority — `?any` is used
|
||||||
|
nowhere in `library/` or `examples/`.
|
||||||
@@ -677,6 +677,29 @@ pub fn coerceMode(self: *Lowering, val: Ref, src_ty: TypeId, dst_ty: TypeId, mod
|
|||||||
.optional_wrap => {
|
.optional_wrap => {
|
||||||
const child_ty = self.module.types.get(dst_ty).optional.child;
|
const child_ty = self.module.types.get(dst_ty).optional.child;
|
||||||
const coerced = self.coerceMode(val, src_ty, child_ty, mode);
|
const coerced = self.coerceMode(val, src_ty, child_ty, mode);
|
||||||
|
// The inner coercion may classify as `.none` (no built-in applies)
|
||||||
|
// and pass `val` through UNCHANGED — e.g. wrapping a `?i64` value
|
||||||
|
// into a `?(?i64)` whose payload is the 1-tuple `(?i64)`. Building
|
||||||
|
// the optional then inserts a `{i64,i1}` into a `{{i64,i1}}` slot,
|
||||||
|
// producing malformed IR that aborts the LLVM verifier (issue 0165).
|
||||||
|
// If the coerced operand's type does not match the optional's child
|
||||||
|
// type, the wrap is invalid — diagnose loudly instead of emitting a
|
||||||
|
// corrupt InsertValue. (`hasErrors()` then aborts the build.)
|
||||||
|
const coerced_ty = self.builder.getRefType(coerced);
|
||||||
|
if (coerced_ty != child_ty) {
|
||||||
|
if (self.diagnostics) |d| {
|
||||||
|
const cs = self.builder.current_span;
|
||||||
|
// Only mention the `(T)`-is-a-1-tuple gotcha when the payload
|
||||||
|
// actually IS a tuple (the `?(?T)` typo); for any other
|
||||||
|
// mismatch the parens note would be misleading.
|
||||||
|
const note: []const u8 = if (self.module.types.get(child_ty) == .tuple)
|
||||||
|
" (note: in type position '(T)' is a single-field tuple, not a grouping — write the inner optional without parentheses)"
|
||||||
|
else
|
||||||
|
"";
|
||||||
|
d.addFmt(.err, ast.Span{ .start = cs.start, .end = cs.end }, "cannot wrap a value of type '{s}' into optional '{s}': its payload type is '{s}'{s}", .{ self.formatTypeName(src_ty), self.formatTypeName(dst_ty), self.formatTypeName(child_ty), note });
|
||||||
|
}
|
||||||
|
return val;
|
||||||
|
}
|
||||||
return self.builder.emit(.{ .optional_wrap = .{ .operand = coerced } }, dst_ty);
|
return self.builder.emit(.{ .optional_wrap = .{ .operand = coerced } }, dst_ty);
|
||||||
},
|
},
|
||||||
// Concrete → Protocol (auto type erasure)
|
// Concrete → Protocol (auto type erasure)
|
||||||
|
|||||||
@@ -556,6 +556,23 @@ pub fn formatTypeName(self: *Lowering, ty: TypeId) []const u8 {
|
|||||||
const inner = self.formatTypeName(v.element);
|
const inner = self.formatTypeName(v.element);
|
||||||
break :blk std.fmt.allocPrint(self.alloc, "Vector({d},{s})", .{ v.length, inner }) catch "vector";
|
break :blk std.fmt.allocPrint(self.alloc, "Vector({d},{s})", .{ v.length, inner }) catch "vector";
|
||||||
},
|
},
|
||||||
|
.tuple => |t| blk: {
|
||||||
|
var buf = std.ArrayList(u8).empty;
|
||||||
|
buf.append(self.alloc, '(') catch break :blk "tuple";
|
||||||
|
for (t.fields, 0..) |f, i| {
|
||||||
|
if (i > 0) buf.appendSlice(self.alloc, ", ") catch break :blk "tuple";
|
||||||
|
// Render the field name for named tuples: `(x: i64, y: i64)`.
|
||||||
|
if (t.names) |ns| {
|
||||||
|
if (i < ns.len) {
|
||||||
|
buf.appendSlice(self.alloc, self.module.types.getString(ns[i])) catch break :blk "tuple";
|
||||||
|
buf.appendSlice(self.alloc, ": ") catch break :blk "tuple";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
buf.appendSlice(self.alloc, self.formatTypeName(f)) catch break :blk "tuple";
|
||||||
|
}
|
||||||
|
buf.append(self.alloc, ')') catch break :blk "tuple";
|
||||||
|
break :blk buf.toOwnedSlice(self.alloc) catch "tuple";
|
||||||
|
},
|
||||||
else => @tagName(info),
|
else => @tagName(info),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -322,6 +322,35 @@ pub fn lowerVarDecl(self: *Lowering, vd: *const ast.VarDecl) void {
|
|||||||
const child = ty_info.optional.child;
|
const child = ty_info.optional.child;
|
||||||
const rt = self.builder.getRefType(ref);
|
const rt = self.builder.getRefType(ref);
|
||||||
if (rt != child and rt != .void and child != .void) ref = self.coerceToType(ref, rt, child);
|
if (rt != child and rt != .void and child != .void) ref = self.coerceToType(ref, rt, child);
|
||||||
|
// After coercion the value MUST be the optional's payload
|
||||||
|
// type. If it isn't (the coercion classified `.none` and
|
||||||
|
// passed the value through unchanged — e.g. a `?i64` value
|
||||||
|
// flowing into `?(?i64)`, whose payload is the 1-tuple
|
||||||
|
// `(?i64)`), wrapping anyway inserts a `{i64,i1}` into a
|
||||||
|
// `{{i64,i1}}` slot and builds malformed IR that aborts the
|
||||||
|
// LLVM verifier (issue 0165). Diagnose loudly instead.
|
||||||
|
const post_rt = self.builder.getRefType(ref);
|
||||||
|
if (post_rt != child and post_rt != .void and child != .void) {
|
||||||
|
if (self.diagnostics) |d| {
|
||||||
|
const cs = self.builder.current_span;
|
||||||
|
// Only mention the `(T)`-is-a-1-tuple gotcha when the
|
||||||
|
// payload actually IS a tuple (the `?(?T)` typo).
|
||||||
|
const note: []const u8 = if (self.module.types.get(child) == .tuple)
|
||||||
|
" (note: in type position '(T)' is a single-field tuple, not a grouping — write the inner optional without parentheses)"
|
||||||
|
else
|
||||||
|
"";
|
||||||
|
d.addFmt(.err, ast.Span{ .start = cs.start, .end = cs.end }, "cannot assign a value of type '{s}' to optional '{s}': its payload type is '{s}'{s}", .{ self.formatTypeName(post_rt), self.formatTypeName(ty), self.formatTypeName(child), note });
|
||||||
|
}
|
||||||
|
// Already diagnosed — store the value as-is and bail. The
|
||||||
|
// trailing coerce below would re-diagnose the same mismatch
|
||||||
|
// (via the `.optional_wrap` guard in coerce.zig); `hasErrors()`
|
||||||
|
// aborts the build regardless of the bytes we store.
|
||||||
|
self.builder.store(slot, ref);
|
||||||
|
if (self.scope) |scope| {
|
||||||
|
scope.put(vd.name, .{ .ref = slot, .ty = ty, .is_alloca = true });
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
ref = self.builder.optionalWrap(ref, ty);
|
ref = self.builder.optionalWrap(ref, ty);
|
||||||
} else if (ty_info == .slice) {
|
} else if (ty_info == .slice) {
|
||||||
// Array → slice promotion: if value is an array, convert to slice
|
// Array → slice promotion: if value is an array, convert to slice
|
||||||
|
|||||||
Reference in New Issue
Block a user