fix: diagnose ?? with a non-optional lhs instead of codegen panic (issue 0172)
lowerNullCoalesce fed resolveOptionalInner's .unresolved (returned for a non-optional lhs) into the merge-block params / optionalUnwrap / RHS target type, reaching codegen and panicking 'unresolved type reached LLVM emission'. Guard: when inferExprType(nc.lhs) is a resolved non-optional type, emit a located diagnostic 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. Regression: examples/diagnostics/1200-diagnostics-null-coalesce-non-optional.sx. Verified by 3 adversarial reviews, suite 790/0. Filed adjacent bug 0180 (?? lowering defects for generic/alias/tuple optional lhs).
This commit is contained in:
@@ -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);
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
1
|
||||
@@ -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'
|
||||
| ^
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -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
|
||||
> '<T>'") 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:
|
||||
|
||||
@@ -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.)
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user