lowerBinaryOp derived the result type from the LHS alone and emitted add/sub/mul/div/mod without checking the RHS, so `s64 + string` lowered as `add : s64` and reinterpreted the string's bytes — printing garbage instead of erroring. Add isArithOperand (int / float / vector / pointer, plus custom int widths) and, for `+ - * / %`, diagnose `cannot apply '<op>' to operands of type '<lhs>' and '<rhs>'` and return a placeholder sentinel instead of the corrupting op. `.unresolved` operands pass through so a type we couldn't infer is never falsely rejected; the existing optional-unwrap and int×float promotion are accounted for before the check. Ordering (`< <= > >=`) and bitwise/shift (`& | ^ << >>`) ops share the same LHS-derived-type hole and are left as a noted follow-up in the issue. Regression: examples/214-arith-operand-type-check.sx (s64 + string, and non-numeric LHS string * s64).
5.0 KiB
0055 — binary arithmetic accepts mismatched operand types (s64 + string)
FIXED (examples/214-arith-operand-type-check.sx). lowerBinaryOp in
src/ir/lower.zig now checks operand-type
compatibility for the scalar arithmetic ops (+ - * / %) before emitting:
if either operand (after the existing optional-unwrap / int×float
promotion) is not a numeric / vector / pointer type via the new
isArithOperand, it emits cannot apply '<op>' to operands of type '<lhs>' and '<rhs>' and returns a placeholder sentinel instead of an add that
reinterprets the RHS bytes. .unresolved operands are passed through so a
type we couldn't infer is never falsely diagnosed. Regression test:
examples/214 (covers s64 + string and non-numeric LHS string * s64).
Not covered by this fix (same LHS-derived-type shape, separate ops): the
ordering comparisons (< <= > >=) and bitwise/shift ops (& | ^ << >>)
have the same hole — s64 < string / s64 & string still lower without an
operand check. Left out to avoid touching enum/flags comparison + bitwise
paths in the same change; flag as a follow-up if it bites.
Symptom
Binary arithmetic operators (+, -, *, /, %) perform no
operand-type compatibility check. s64 + string compiles cleanly and
runs, reinterpreting the string operand's bytes (pointer/len) as an
integer.
- Observed:
a + cwherea: s64,c: stringcompiles and prints a garbage number (e.g.4346102832—40 + <string data pointer>). - Expected: a type-error diagnostic, e.g.
cannot apply '+' to operands of type 's64' and 'string'.
Surfaced in examples/213-canonical-map.sx: a third source v3: StrCell
(VL(string)) was added so the mapper became (a, b, c) => a + b + c
with a, b: s64 and c: string. The canonical map infers the closure
params from the projected pack element types, so a + b + c is
s64 + s64 + string — which should reject. Instead r.get() prints
garbage (4312977714).
Note: the working-tree copy of
examples/213also contains a separate typo —Closure(..sources.Target)instead of..sources.T..Targetis an unknown projection name and produces "cannot infer type of lambda parameter" errors that mask this bug. Restoring.Texposes the real hole. The two are independent; this issue is about the missing arithmetic operand-type check, reproducible standalone with no pack machinery.
Reproduction
Minimal, standalone (only modules/std.sx):
#import "modules/std.sx";
main :: () -> s32 {
a : s64 = 40;
c : string = "it should error";
r := a + c; // expected: type error (s64 + string)
print("{}\n", r); // actual: prints garbage, e.g. 4346102832
0;
}
$ ./zig-out/bin/sx run repro.sx
4346102832 # should be a compile error, not a number
Investigation prompt
Binary arithmetic in the sx compiler does no operand-type compatibility check.
lowerBinaryOpin src/ir/lower.zig (starts ~L2545) computes the result typetypurely from the LHS operand (var ty = lhs_ty;~L2680), then at the finalswitch (bop.op)(~L2735) emits.add => self.builder.add(lhs, rhs, ty)(and.sub/.mul/.div/.mod) with that LHS-derivedty— never verifyingrhs_tyis compatible. Fors64 + string,ty = .s64and thestringrhs Ref is fed to anadd : s64, reinterpreting its bytes as an integer.The fix: before the arithmetic
switcharms, for the arithmetic ops (add/sub/mul/div/mod) check thatlhs_tyandrhs_tyare arithmetic-compatible (both numeric — int/float — modulo the existing int×float promotion at ~L2682, or otherwise an exact match). When they are not, emit a diagnostic via the existingif (self.diagnostics) |diags| diags.addFmt(.err, <span>, "cannot apply '{s}' to operands of type '{...}' and '{...}'", .{...})pattern (see the diagnostics calls already in this file, e.g. ~L2175, ~L2193) and return a non-corrupting sentinel rather than a silently-wrongadd.Watch the legitimate non-numeric
+paths so the check doesn't false-positive: tuple ops (lowerTupleOp, handled earlier ~L2718), string==/!=(handled ~L2710), optional auto-unwrap (~L2693), and the int×float promotion (~L2682). Decide whether string concatenation via+is intended to be supported at all — if not,string + stringshould also reject (currently untested here).Span: confirm how to get the operator/expression span for the diagnostic —
lowerBinaryOptakesbop: *const ast.BinaryOp; check whetherast.BinaryOpcarries a span or whether the enclosing node's span must be threaded in (the otheraddFmtsites usenode.span).Verify: re-run the repro above — expect a type-error diagnostic and a non-zero exit instead of a printed integer. Then restore
examples/213's.Target→.Tand confirm(a, b, c) => a + b + cwith astringthird source now errors at the+rather than printing garbage. Runbash tests/run_examples.shto confirm no legitimate arithmetic regressed.