fix: presence-preserving optional->optional coercion (issue 0180)
The generic-?? wrong-fallback was not in lowerNullCoalesce: coercing ?A -> ?B (differing payload, e.g. the ?i32->?i64 call-arg coercion when instantiating unwrap_or(99, ?i32)) routed through .optional_wrap, which unconditionally unwrapped the source and re-wrapped as ALWAYS-PRESENT, so a null became present-zero everywhere (args, returns, field init, var-decl, ??). Add a CoercionPlan.optional_to_optional (conversions.zig) + a presence-preserving arm in coerceMode (coerce.zig): has_value -> present: unwrap+coerce-child+wrap-present; absent: constNull(dst); merge via a dst_ty block param. lowerVarDecl gains a !src_is_optional guard so an annotated x : ?B = <?A> routes through the same arm (also makes aggregate-payload var-decl ?[3]i64->?[]i64 / ?Concrete->?Protocol work). Alias-optional struct-literal default already works (grouping + 0166); a 1-tuple default ?(i32,) ?? 5 now emits a clean diagnostic instead of an LLVM PHI abort (no implicit scalar->1-tuple coercion per spec). Regressions: optionals/0916 (generic ??), 0917 (alias struct default), 0918 (var-decl optional->optional), diagnostics/1202 (1-tuple default) + a conversions.test.zig unit test. Verified by 3 adversarial reviews, suite 798/0.
This commit is contained in:
@@ -48,6 +48,13 @@ test "conversions: classify covers the built-in coercion ladder" {
|
||||
try std.testing.expectEqual(Plan.optional_unwrap, cr.classify(opt_i64, .i64));
|
||||
try std.testing.expectEqual(Plan.void_to_optional, cr.classify(.void, opt_i64));
|
||||
|
||||
// `?A → ?B` (differing payloads) is a presence-preserving payload coercion,
|
||||
// NOT the always-present unwrap-then-rewrap that the `.optional_wrap` arm
|
||||
// produced (issue 0180). Same-payload optionals are `.no_op`.
|
||||
const opt_i32 = tt.optionalOf(.i32);
|
||||
try std.testing.expectEqual(Plan.optional_to_optional, cr.classify(opt_i32, opt_i64));
|
||||
try std.testing.expectEqual(Plan.no_op, cr.classify(opt_i64, opt_i64));
|
||||
|
||||
// `?T → bool` is NOT an unwrap-then-narrow presence test (issue 0169):
|
||||
// it must reject, never silently produce `false`. But `?bool → bool`
|
||||
// is a genuine unwrap of a bool payload.
|
||||
|
||||
@@ -36,6 +36,7 @@ pub const CoercionResolver = struct {
|
||||
optional_unwrap, // ?T → concrete (narrowing)
|
||||
optional_to_bool_reject, // ?T → bool (no presence-test coercion; diagnostic)
|
||||
void_to_optional, // void (null literal) → ?T
|
||||
optional_to_optional, // ?A → ?B (presence-preserving payload coercion)
|
||||
optional_wrap, // concrete → ?T
|
||||
erase_protocol, // concrete → protocol value
|
||||
int_to_float,
|
||||
@@ -114,6 +115,16 @@ pub const CoercionResolver = struct {
|
||||
if (child_ty == dst_ty or (dst_ty.isBuiltin() and child_ty.isBuiltin())) {
|
||||
return .optional_unwrap;
|
||||
}
|
||||
// ?A → ?B: a presence-preserving payload coercion. Without a
|
||||
// dedicated arm this fell to `.optional_wrap` (dst is optional),
|
||||
// which unwrapped the SOURCE optional unconditionally and re-
|
||||
// wrapped it as always-present — turning a null `?i32` into a
|
||||
// present `?i64` carrying the zero payload (issue 0180: generic
|
||||
// `??` returning the wrong fallback). Only meaningful when the
|
||||
// children differ (same-type optionals are `.no_op` already).
|
||||
if (!dst_ty.isBuiltin() and self.l.module.types.get(dst_ty) == .optional) {
|
||||
return .optional_to_optional;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -683,6 +683,39 @@ pub fn coerceMode(self: *Lowering, val: Ref, src_ty: TypeId, dst_ty: TypeId, mod
|
||||
},
|
||||
// void → Optional: produce null (void is the type of null_literal)
|
||||
.void_to_optional => return self.builder.constNull(dst_ty),
|
||||
// ?A → ?B: presence-preserving payload coercion. The naive route
|
||||
// (fall through to `.optional_wrap`) unwrapped the SOURCE optional
|
||||
// unconditionally and re-wrapped as always-present, dropping the
|
||||
// has-bit — a null `?i32` became a present `?i64` carrying zero
|
||||
// (issue 0180: generic `??` returning the wrong fallback). Branch on
|
||||
// the source's presence so the payload is only unwrapped when it is
|
||||
// actually present (the interp errors on unwrapping a null optional,
|
||||
// so a branchless select is not an option):
|
||||
// present → unwrap src child, coerce to dst child, wrap as present
|
||||
// absent → null of the destination optional
|
||||
.optional_to_optional => {
|
||||
const src_child = self.module.types.get(src_ty).optional.child;
|
||||
const dst_child = self.module.types.get(dst_ty).optional.child;
|
||||
|
||||
const has_val = self.builder.optionalHasValue(val);
|
||||
const present_bb = self.freshBlock("opt2opt.present");
|
||||
const absent_bb = self.freshBlock("opt2opt.absent");
|
||||
const merge_bb = self.freshBlockWithParams("opt2opt.merge", &.{dst_ty});
|
||||
self.builder.condBr(has_val, present_bb, &.{}, absent_bb, &.{});
|
||||
|
||||
self.builder.switchToBlock(present_bb);
|
||||
const unwrapped = self.builder.optionalUnwrap(val, src_child);
|
||||
const coerced_child = self.coerceMode(unwrapped, src_child, dst_child, mode);
|
||||
const wrapped = self.builder.optionalWrap(coerced_child, dst_ty);
|
||||
self.builder.br(merge_bb, &.{wrapped});
|
||||
|
||||
self.builder.switchToBlock(absent_bb);
|
||||
const null_dst = self.builder.constNull(dst_ty);
|
||||
self.builder.br(merge_bb, &.{null_dst});
|
||||
|
||||
self.builder.switchToBlock(merge_bb);
|
||||
return self.builder.blockParam(merge_bb, 0, dst_ty);
|
||||
},
|
||||
// Concrete → Optional wrapping (coerce to the inner type first)
|
||||
.optional_wrap => {
|
||||
const child_ty = self.module.types.get(dst_ty).optional.child;
|
||||
|
||||
@@ -2065,6 +2065,25 @@ pub fn lowerNullCoalesce(self: *Lowering, nc: *const ast.NullCoalesce) Ref {
|
||||
if (rhs_ty != inner_ty and rhs_ty != .void and inner_ty != .void) {
|
||||
rhs = self.coerceToType(rhs, rhs_ty, inner_ty);
|
||||
}
|
||||
// The merge-block param, the unwrapped LHS, and the coerced RHS must all
|
||||
// share `inner_ty` — they feed the same PHI. If the RHS default still does
|
||||
// not match after coercion (e.g. a scalar `5` default against an optional
|
||||
// whose payload is a 1-tuple `(i32,)`: there is no implicit scalar→1-tuple
|
||||
// coercion — a 1-tuple value is written `(5,)`), branching with the
|
||||
// mismatched type emits a `phi {i32}` vs `i32` that aborts the LLVM
|
||||
// verifier (issue 0180). Diagnose loudly and br with a typed placeholder so
|
||||
// the PHI stays well-formed; `hasErrors()` aborts before codegen anyway.
|
||||
const coerced_ty = self.builder.getRefType(rhs);
|
||||
if (coerced_ty != inner_ty and coerced_ty != .void and inner_ty != .void) {
|
||||
if (self.diagnostics) |d| {
|
||||
const note: []const u8 = if (!inner_ty.isBuiltin() and self.module.types.get(inner_ty) == .tuple)
|
||||
" (note: a 1-tuple value is written '(x,)' with a trailing comma)"
|
||||
else
|
||||
"";
|
||||
d.addFmt(.err, nc.rhs.span, "'??' default has type '{s}', but the optional's payload is '{s}'{s}", .{ self.formatTypeName(coerced_ty), self.formatTypeName(inner_ty), note });
|
||||
}
|
||||
rhs = self.builder.constNull(inner_ty); // typed placeholder — keeps the PHI well-formed
|
||||
}
|
||||
self.builder.br(merge_bb, &.{rhs});
|
||||
|
||||
// Continue at merge
|
||||
|
||||
@@ -314,7 +314,18 @@ pub fn lowerVarDecl(self: *Lowering, vd: *const ast.VarDecl) void {
|
||||
// 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 and self.builder.getRefType(ref) != ty) {
|
||||
// Is the initializer value ITSELF an optional (`?A`)? If so the
|
||||
// target `?B` is a presence-PRESERVING coercion (`.optional_to_optional`),
|
||||
// NOT a wrap-present. The manual unwrap-to-child + `optionalWrap(present)`
|
||||
// below classifies `?A → child_B` as an unconditional unwrap (→ 0 for a
|
||||
// null source) then wraps it as always-present, so a null `?A` becomes a
|
||||
// present `?B` carrying zero (issue 0180). Route an optional source through
|
||||
// the trailing general `coerceToType(?A → ?B)` instead, which dispatches to
|
||||
// the presence-preserving arm. The wrap-present path below stays correct for
|
||||
// a non-optional source `T → ?T`.
|
||||
const ref_ty0 = self.builder.getRefType(ref);
|
||||
const src_is_optional = !ref_ty0.isBuiltin() and self.module.types.get(ref_ty0) == .optional;
|
||||
if (ty_info == .optional and val.data != .null_literal and ref_ty0 != ty and !src_is_optional) {
|
||||
// Coerce to the optional's CHILD first (e.g. an array value
|
||||
// into a `?[]T` promotes array→slice), THEN wrap — wrapping
|
||||
// the raw value would store e.g. array bits into the slice
|
||||
|
||||
Reference in New Issue
Block a user