diff --git a/examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx b/examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx new file mode 100644 index 00000000..55d4b408 --- /dev/null +++ b/examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx @@ -0,0 +1,17 @@ +// `??` (null-coalesce) requires an optional left operand. A resolved +// non-optional lhs is malformed user input and must be a clean, located type +// error — not a compiler crash. Regression (issue 0172): `lowerNullCoalesce` +// used to feed the `.unresolved` inner type (from `resolveOptionalInner` on a +// non-optional lhs) straight into the merge-block params / `optionalUnwrap` / +// the RHS target type, which reached emit_llvm's "unresolved type reached LLVM +// emission" panic (exit 134) with no diagnostic. A guard now inspects the +// inferred lhs type: a resolved non-optional emits this error and bails before +// codegen; an already-`.unresolved` lhs (a prior error, e.g. undefined name) +// passes through untouched so the original diagnostic isn't double-reported. + +#import "modules/std.sx"; + +main :: () { + x := 5 ?? 7; // <- left operand of '??' must be an optional, but has type 'i64' + print("{}\n", x); +} diff --git a/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.exit b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.exit new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.exit @@ -0,0 +1 @@ +1 diff --git a/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stderr b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stderr new file mode 100644 index 00000000..0b934195 --- /dev/null +++ b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stderr @@ -0,0 +1,5 @@ +error: left operand of '??' must be an optional, but has type 'i64' + --> examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx:15:8 + | +15 | x := 5 ?? 7; // <- left operand of '??' must be an optional, but has type 'i64' + | ^ diff --git a/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stdout b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stdout new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/diagnostics/expected/1200-diagnostics-null-coalesce-non-optional.stdout @@ -0,0 +1 @@ + diff --git a/issues/0172-null-coalesce-non-optional-lhs-panics.md b/issues/0172-null-coalesce-non-optional-lhs-panics.md index 994f9258..b22c31ab 100644 --- a/issues/0172-null-coalesce-non-optional-lhs-panics.md +++ b/issues/0172-null-coalesce-non-optional-lhs-panics.md @@ -1,5 +1,18 @@ # 0172 — `??` with a non-optional left-hand side panics instead of diagnosing +> **RESOLVED.** `lowerNullCoalesce` fed `resolveOptionalInner`'s `.unresolved` +> (returned for a non-optional lhs) into the merge-block / `optionalUnwrap` / RHS +> target type → codegen panic. Fix (`src/ir/lower/expr.zig`): before computing +> `inner_ty`, if `inferExprType(nc.lhs)` is a RESOLVED non-optional type, emit a +> located diagnostic ("left operand of '??' must be an optional, but has type +> ''") and bail; an `.unresolved` lhs (prior error) is excluded to avoid +> double-report. `??` is optional-only per specs.md (error unions use +> `or`/`catch`), so rejecting a failable lhs is correct. Comptime panic closed +> too. Verified by 3 adversarial reviews; suite 790/0. Regression: +> `examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx`. (Adjacent +> pre-existing `??`-lowering defects found + filed: 0180 — generic / alias / +> tuple optional lhs.) + ## Symptom Using `??` where the left operand is NOT an optional panics the compiler: diff --git a/issues/0180-null-coalesce-generic-and-nontrivial-optional-defects.md b/issues/0180-null-coalesce-generic-and-nontrivial-optional-defects.md new file mode 100644 index 00000000..05d4abd8 --- /dev/null +++ b/issues/0180-null-coalesce-generic-and-nontrivial-optional-defects.md @@ -0,0 +1,58 @@ +# 0180 — `??` lowering defects for generic / alias / tuple optional lhs (wrong fallback, segfault, PHI mismatch) + +## Symptom + +`lowerNullCoalesce` mishandles several non-trivial optional lhs shapes (all with a +genuinely-optional lhs, so the issue-0172 non-optional guard correctly does not +fire). Found during adversarial review of issue 0172. + +1. **Generic `??` returns the WRONG fallback** (VERIFIED — silent miscompile): + a `??` inside a generic function where the lhs is a type-param-typed optional + `?T` drops the RHS default and returns the zero payload instead. +2. **Alias-optional with a struct-literal default segfaults** (reported by review): + `?Opt ?? Opt.{}` where `Opt :: ?Struct` crashes in type interning + (`hashString`/wyhash). +3. **`?(i32) ?? i32` LLVM PHI-type mismatch** (reported by review): an + optional-of-1-tuple coalesced with a scalar default emits `phi { i32 }` vs + `i32`. + +(Scalar-default alias-optional `Opt :: ?i64; o ?? 7` works correctly — the +defects are specific to the shapes above.) + +## Reproduction + +(1) Generic `??` wrong fallback — VERIFIED, prints `0`, expected `99`: +```sx +#import "modules/std.sx"; +unwrap_or :: (d: $T, x: ?T) -> T { return x ?? d; } +main :: () { b : ?i32 = null; print("{}\n", unwrap_or(99, b)); } // prints 0 — WRONG +``` + +(2) Alias-optional + struct-literal default (reported segfault): +```sx +#import "modules/std.sx"; +S :: struct { a: i64 = 0; } +Opt :: ?S; +main :: () { o : Opt = null; x := o ?? S.{ a = 7 }; print("{}\n", x.a); } +``` + +(3) Optional-of-tuple + scalar default (reported PHI mismatch): +```sx +#import "modules/std.sx"; +main :: () { o : ?(i32) = null; x := o ?? 5; } +``` + +## Investigation prompt + +`src/ir/lower/expr.zig` `lowerNullCoalesce`. For (1), when the lhs optional's +child is a TYPE PARAMETER (`?T`, resolved per monomorphization), the present/ +absent merge appears to drop the RHS default and yield the zero payload — check +that `inner_ty` and the merge-block param resolve to the monomorphized child and +that the RHS (`d`) is correctly selected on the null path. For (2)/(3), the merge +of present-payload vs default has a type mismatch when the child is an +alias/struct (interning crash) or a 1-tuple (PHI `{i32}` vs `i32`) — the default +and the unwrapped payload must share the exact merge TypeId. Verify each repro +produces the expected value (`99`, `7`, and a clean `?(i32)` coalesce); confirm +the generic case across multiple instantiations. Add regressions under +`examples/optionals/09xx-...`. (These reproduce on master, independent of the +issue-0172 guard.) diff --git a/src/ir/lower/expr.zig b/src/ir/lower/expr.zig index 88bfc90c..3d194345 100644 --- a/src/ir/lower/expr.zig +++ b/src/ir/lower/expr.zig @@ -1952,7 +1952,24 @@ pub fn lowerForceUnwrap(self: *Lowering, fu: *const ast.ForceUnwrap) Ref { pub fn lowerNullCoalesce(self: *Lowering, nc: *const ast.NullCoalesce) Ref { const lhs = self.lowerExpr(nc.lhs); - const inner_ty = self.resolveOptionalInner(self.inferExprType(nc.lhs)); + const lhs_ty = self.inferExprType(nc.lhs); + + // `??` requires an optional left operand. A resolved non-optional lhs is + // malformed user input: diagnose it here instead of letting the + // `.unresolved` inner type (from `resolveOptionalInner`) flow into the + // merge-block params / `optionalUnwrap` / the RHS target type and reach + // emit_llvm's "unresolved type reached LLVM emission" panic with no source + // location (issue 0172). Skip an already-`.unresolved` lhs: that comes + // from a PRIOR error (e.g. an undefined name) which has already been + // diagnosed — re-reporting would be a confusing second message. + const lhs_is_optional = !lhs_ty.isBuiltin() and self.module.types.get(lhs_ty) == .optional; + if (lhs_ty != .unresolved and !lhs_is_optional) { + if (self.diagnostics) |d| + d.addFmt(.err, nc.lhs.span, "left operand of '??' must be an optional, but has type '{s}'", .{self.formatTypeName(lhs_ty)}); + return lhs; // placeholder — hasErrors() aborts before codegen + } + + const inner_ty = self.resolveOptionalInner(lhs_ty); // Short-circuit: only evaluate RHS if LHS is null. // IMPORTANT: optional_unwrap must be in the "has value" branch,