From 0bc8005b99fc52e4af5ea82b6cc73eef17d3db89 Mon Sep 17 00:00:00 2001 From: agra Date: Mon, 22 Jun 2026 21:54:12 +0300 Subject: [PATCH] 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). --- .../1101-diagnostics-err-tuple-oob.sx | 2 +- ...ostics-err-parenthesized-optional-tuple.sx | 18 +++++++ .../1101-diagnostics-err-tuple-oob.stderr | 2 +- ...tics-err-parenthesized-optional-tuple.exit | 1 + ...cs-err-parenthesized-optional-tuple.stderr | 5 ++ ...cs-err-parenthesized-optional-tuple.stdout | 1 + .../0911-nested-optional-via-alias.sx | 29 ++++++++++++ .../0911-nested-optional-via-alias.exit | 1 + .../0911-nested-optional-via-alias.stderr | 1 + .../0911-nested-optional-via-alias.stdout | 5 ++ ...protocols-generic-struct-protocol-erase.ir | 4 +- ...parenthesized-nested-optional-malformed.md | 17 +++++++ ...71-optional-any-child-not-canonicalized.md | 47 +++++++++++++++++++ src/ir/lower/coerce.zig | 23 +++++++++ src/ir/lower/generic.zig | 17 +++++++ src/ir/lower/stmt.zig | 29 ++++++++++++ 16 files changed, 198 insertions(+), 4 deletions(-) create mode 100644 examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx create mode 100644 examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.exit create mode 100644 examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stderr create mode 100644 examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stdout create mode 100644 examples/optionals/0911-nested-optional-via-alias.sx create mode 100644 examples/optionals/expected/0911-nested-optional-via-alias.exit create mode 100644 examples/optionals/expected/0911-nested-optional-via-alias.stderr create mode 100644 examples/optionals/expected/0911-nested-optional-via-alias.stdout create mode 100644 issues/0171-optional-any-child-not-canonicalized.md diff --git a/examples/diagnostics/1101-diagnostics-err-tuple-oob.sx b/examples/diagnostics/1101-diagnostics-err-tuple-oob.sx index db0d0947..3c182d56 100644 --- a/examples/diagnostics/1101-diagnostics-err-tuple-oob.sx +++ b/examples/diagnostics/1101-diagnostics-err-tuple-oob.sx @@ -1,5 +1,5 @@ // 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 { t := (10, 20); diff --git a/examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx b/examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx new file mode 100644 index 00000000..863f2206 --- /dev/null +++ b/examples/diagnostics/1195-diagnostics-err-parenthesized-optional-tuple.sx @@ -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"); +} diff --git a/examples/diagnostics/expected/1101-diagnostics-err-tuple-oob.stderr b/examples/diagnostics/expected/1101-diagnostics-err-tuple-oob.stderr index 91f9de25..43fe1659 100644 --- a/examples/diagnostics/expected/1101-diagnostics-err-tuple-oob.stderr +++ b/examples/diagnostics/expected/1101-diagnostics-err-tuple-oob.stderr @@ -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 | 6 | return xx t.42; diff --git a/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.exit b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.exit new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.exit @@ -0,0 +1 @@ +1 diff --git a/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stderr b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stderr new file mode 100644 index 00000000..a895f180 --- /dev/null +++ b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stderr @@ -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; + | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stdout b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stdout new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/diagnostics/expected/1195-diagnostics-err-parenthesized-optional-tuple.stdout @@ -0,0 +1 @@ + diff --git a/examples/optionals/0911-nested-optional-via-alias.sx b/examples/optionals/0911-nested-optional-via-alias.sx new file mode 100644 index 00000000..7d9c644b --- /dev/null +++ b/examples/optionals/0911-nested-optional-via-alias.sx @@ -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); +} diff --git a/examples/optionals/expected/0911-nested-optional-via-alias.exit b/examples/optionals/expected/0911-nested-optional-via-alias.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/optionals/expected/0911-nested-optional-via-alias.exit @@ -0,0 +1 @@ +0 diff --git a/examples/optionals/expected/0911-nested-optional-via-alias.stderr b/examples/optionals/expected/0911-nested-optional-via-alias.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/optionals/expected/0911-nested-optional-via-alias.stderr @@ -0,0 +1 @@ + diff --git a/examples/optionals/expected/0911-nested-optional-via-alias.stdout b/examples/optionals/expected/0911-nested-optional-via-alias.stdout new file mode 100644 index 00000000..6e4d6257 --- /dev/null +++ b/examples/optionals/expected/0911-nested-optional-via-alias.stdout @@ -0,0 +1,5 @@ +outer present: true +mid present: true +value: 5 +coalesced: 5 +empty present: false diff --git a/examples/protocols/expected/0414-protocols-generic-struct-protocol-erase.ir b/examples/protocols/expected/0414-protocols-generic-struct-protocol-erase.ir index 2dff31d6..e720c698 100644 --- a/examples/protocols/expected/0414-protocols-generic-struct-protocol-erase.ir +++ b/examples/protocols/expected/0414-protocols-generic-struct-protocol-erase.ir @@ -1381,7 +1381,7 @@ @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.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.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 @@ -16197,7 +16197,7 @@ if.else.1256: ; preds = %entry %loadN = load i64, ptr %allocaN, align 8 %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.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 if.merge.1257: ; preds = %if.else.1256, %if.then.1255 diff --git a/issues/0165-parenthesized-nested-optional-malformed.md b/issues/0165-parenthesized-nested-optional-malformed.md index 5086b4a1..ea179eeb 100644 --- a/issues/0165-parenthesized-nested-optional-malformed.md +++ b/issues/0165-parenthesized-nested-optional-malformed.md @@ -1,5 +1,22 @@ # 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 A nested optional written `?(?i64)` resolves to a spurious extra struct wrapper: diff --git a/issues/0171-optional-any-child-not-canonicalized.md b/issues/0171-optional-any-child-not-canonicalized.md new file mode 100644 index 00000000..9d6c3e32 --- /dev/null +++ b/issues/0171-optional-any-child-not-canonicalized.md @@ -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/`. diff --git a/src/ir/lower/coerce.zig b/src/ir/lower/coerce.zig index 56b82eb5..94832875 100644 --- a/src/ir/lower/coerce.zig +++ b/src/ir/lower/coerce.zig @@ -677,6 +677,29 @@ pub fn coerceMode(self: *Lowering, val: Ref, src_ty: TypeId, dst_ty: TypeId, mod .optional_wrap => { const child_ty = self.module.types.get(dst_ty).optional.child; 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); }, // Concrete → Protocol (auto type erasure) diff --git a/src/ir/lower/generic.zig b/src/ir/lower/generic.zig index 00096067..5e12d242 100644 --- a/src/ir/lower/generic.zig +++ b/src/ir/lower/generic.zig @@ -556,6 +556,23 @@ pub fn formatTypeName(self: *Lowering, ty: TypeId) []const u8 { const inner = self.formatTypeName(v.element); 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), }; } diff --git a/src/ir/lower/stmt.zig b/src/ir/lower/stmt.zig index fcd43b08..9217379b 100644 --- a/src/ir/lower/stmt.zig +++ b/src/ir/lower/stmt.zig @@ -322,6 +322,35 @@ pub fn lowerVarDecl(self: *Lowering, vd: *const ast.VarDecl) void { const child = ty_info.optional.child; const rt = self.builder.getRefType(ref); 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); } else if (ty_info == .slice) { // Array → slice promotion: if value is an array, convert to slice