Files
sx/issues/0055-binary-arith-no-operand-type-check.md
agra ac7f1d10e5 lang: extend operand-type check to ordering + bitwise/shift (issue 0055 follow-up)
The arithmetic-only check from the previous commit shared a hole with the
comparison and bitwise/shift ops: lowerBinaryOp derives the result type
from the LHS, so `s64 < string` fed mismatched types to `icmp` (LLVM
verifier failure) and `s64 & string` reinterpreted the string's bytes.

Add isOrderingOperand (numeric / enum / pointer / bool / vector) and
isBitwiseOperand (integer / enum / bool / vector), and route `< <= > >=`
and `& | ^ << >>` through them alongside the existing arithmetic check, all
sharing one diagnostic + placeholder-sentinel path. Flags-enum bitwise
(`.read | .write`, `perm & .read`), enum/pointer comparison, and int
literals stay legal (50-smoke unaffected).

Equality `== / !=` is deliberately left unchecked — its path is heavily
special-cased (str_eq, Any unbox, optional == null); folding a check in
without regressing those is a separate change, noted in the issue.

Regression test renamed arith→binop and broadened to cover `+ * < & <<`
against a string operand: examples/214-binop-operand-type-check.sx.
2026-05-30 10:30:57 +03:00

5.6 KiB
Raw Blame History

0055 — binary arithmetic accepts mismatched operand types (s64 + string)

FIXED (examples/214-binop-operand-type-check.sx). lowerBinaryOp in src/ir/lower.zig now checks operand-type compatibility for every scalar binary-op group before emitting, via three predicates that pass .unresolved through (so a type we couldn't infer is never falsely diagnosed) but reject a concretely incompatible operand:

  • arithmetic + - * / %isArithOperand (numeric / vector / pointer). Without it s64 + string lowered as add : s64 and reinterpreted the string's bytes — garbage.
  • ordering < <= > >=isOrderingOperand (numeric / enum / pointer / bool / vector). Without it s64 < string fed mismatched LLVM types to icmp and tripped the verifier.
  • bitwise / shift & | ^ << >>isBitwiseOperand (integer / enum / bool / vector). Without it s64 & string reinterpreted the bytes.

On mismatch it emits cannot apply '<op>' to operands of type '<lhs>' and '<rhs>' and returns a placeholder sentinel instead of the corrupting op. The existing optional-unwrap and int×float promotion are applied before the check. Legitimate mixes — flags-enum bitwise (.read | .write, perm & .read), enum/pointer comparison, int literals — are unaffected (covered by examples/50-smoke.sx). Regression test: examples/214 (rejects + * < & << against a string operand).

Still NOT covered (left deliberately): equality == / !=, whose path is heavily special-cased (string str_eq, Any unbox, optional == null). s64 == string still slips through. Folding a compatibility check into that path without regressing the special cases is a separate change — open a fresh issue 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 + c where a: s64, c: string compiles and prints a garbage number (e.g. 434610283240 + <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/213 also contains a separate typo — Closure(..sources.Target) instead of ..sources.T. .Target is an unknown projection name and produces "cannot infer type of lambda parameter" errors that mask this bug. Restoring .T exposes 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. lowerBinaryOp in src/ir/lower.zig (starts ~L2545) computes the result type ty purely from the LHS operand (var ty = lhs_ty; ~L2680), then at the final switch (bop.op) (~L2735) emits .add => self.builder.add(lhs, rhs, ty) (and .sub/.mul/.div/.mod) with that LHS-derived ty — never verifying rhs_ty is compatible. For s64 + string, ty = .s64 and the string rhs Ref is fed to an add : s64, reinterpreting its bytes as an integer.

The fix: before the arithmetic switch arms, for the arithmetic ops (add/sub/mul/div/mod) check that lhs_ty and rhs_ty are 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 existing if (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-wrong add.

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 + string should also reject (currently untested here).

Span: confirm how to get the operator/expression span for the diagnostic — lowerBinaryOp takes bop: *const ast.BinaryOp; check whether ast.BinaryOp carries a span or whether the enclosing node's span must be threaded in (the other addFmt sites use node.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.T and confirm (a, b, c) => a + b + c with a string third source now errors at the + rather than printing garbage. Run bash tests/run_examples.sh to confirm no legitimate arithmetic regressed.