From 6016b0871207733817eb2b599b55794917f78ae9 Mon Sep 17 00:00:00 2001 From: agra Date: Sat, 30 May 2026 09:56:32 +0300 Subject: [PATCH] lang: reject mismatched operand types in scalar arithmetic (issue 0055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 '' to operands of type '' and ''` 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). --- examples/214-arith-operand-type-check.sx | 15 +++ ...0055-binary-arith-no-operand-type-check.md | 105 ++++++++++++++++++ src/ir/lower.zig | 65 +++++++++++ .../214-arith-operand-type-check.exit | 1 + .../expected/214-arith-operand-type-check.txt | 11 ++ 5 files changed, 197 insertions(+) create mode 100644 examples/214-arith-operand-type-check.sx create mode 100644 issues/0055-binary-arith-no-operand-type-check.md create mode 100644 tests/expected/214-arith-operand-type-check.exit create mode 100644 tests/expected/214-arith-operand-type-check.txt diff --git a/examples/214-arith-operand-type-check.sx b/examples/214-arith-operand-type-check.sx new file mode 100644 index 0000000..0171ce9 --- /dev/null +++ b/examples/214-arith-operand-type-check.sx @@ -0,0 +1,15 @@ +// Scalar arithmetic (`+ - * / %`) requires numeric operands. Mixing a +// non-numeric type (here `string`) is rejected at compile time. The +// result type is otherwise taken from the left operand, so without this +// check `n + s` would lower as `add : s64` and reinterpret the string's +// bytes as an integer — silently producing garbage. + +#import "modules/std.sx"; + +main :: () -> s32 { + n : s64 = 40; + s : string = "nope"; + x := n + s; // s64 + string — rejected + y := s * n; // string * s64 — rejected (non-numeric LHS too) + 0; +} diff --git a/issues/0055-binary-arith-no-operand-type-check.md b/issues/0055-binary-arith-no-operand-type-check.md new file mode 100644 index 0000000..a57fb6f --- /dev/null +++ b/issues/0055-binary-arith-no-operand-type-check.md @@ -0,0 +1,105 @@ +# 0055 — binary arithmetic accepts mismatched operand types (`s64 + string`) + +**FIXED** (`examples/214-arith-operand-type-check.sx`). `lowerBinaryOp` in +[src/ir/lower.zig](../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 '' to operands of type '' +and ''` 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 + c` where `a: s64`, `c: string` compiles and prints a + garbage number (e.g. `4346102832` — `40 + `). +- **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`): + +```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](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, , "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. diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 844b9dc..f418d81 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2732,6 +2732,32 @@ pub const Lowering = struct { } } + // Reject scalar arithmetic on incompatible operand types (e.g. + // `s64 + string`). The result type `ty` is derived from the LHS, + // so without this the op lowers as `add : ` and reinterprets + // the RHS bytes as the LHS type, silently producing garbage. + switch (bop.op) { + .add, .sub, .mul, .div, .mod => { + const eff_rhs_ty = blk: { + if (rhs_ty == .unresolved) break :blk self.builder.getRefType(rhs); + if (!rhs_ty.isBuiltin()) { + const ri = self.module.types.get(rhs_ty); + if (ri == .optional) break :blk ri.optional.child; + } + break :blk rhs_ty; + }; + if (!self.isArithOperand(ty) or !self.isArithOperand(eff_rhs_ty)) { + if (self.diagnostics) |diags| { + diags.addFmt(.err, bop.lhs.span, "cannot apply '{s}' to operands of type '{s}' and '{s}'", .{ + binOpSymbol(bop.op), self.formatTypeName(ty), self.formatTypeName(eff_rhs_ty), + }); + } + return self.emitPlaceholder("arith-type-mismatch"); + } + }, + else => {}, + } + return switch (bop.op) { .add => self.builder.add(lhs, rhs, ty), .sub => self.builder.sub(lhs, rhs, ty), @@ -14619,6 +14645,45 @@ pub const Lowering = struct { return false; } + /// Operands valid for a scalar numeric op (`+ - * / %`): ints (incl. + /// custom widths), floats, SIMD vectors, and pointers (pointer + /// arithmetic). `.unresolved` returns true so a type we couldn't infer + /// is never diagnosed — the check only fires on a concretely + /// incompatible operand (e.g. `string`, a struct, an enum). + fn isArithOperand(self: *Lowering, ty: TypeId) bool { + if (ty == .unresolved) return true; + if (isInt(ty) or isFloat(ty)) return true; + if (ty.isBuiltin()) return false; + return switch (self.module.types.get(ty)) { + .signed, .unsigned, .vector, .pointer, .many_pointer => true, + else => false, + }; + } + + fn binOpSymbol(op: ast.BinaryOp.Op) []const u8 { + return switch (op) { + .add => "+", + .sub => "-", + .mul => "*", + .div => "/", + .mod => "%", + .eq => "==", + .neq => "!=", + .lt => "<", + .lte => "<=", + .gt => ">", + .gte => ">=", + .and_op => "and", + .or_op => "or", + .bit_and => "&", + .bit_or => "|", + .bit_xor => "^", + .shl => "<<", + .shr => ">>", + .in_op => "in", + }; + } + fn typeBits(ty: TypeId) u32 { return switch (ty) { .bool => 1, diff --git a/tests/expected/214-arith-operand-type-check.exit b/tests/expected/214-arith-operand-type-check.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/expected/214-arith-operand-type-check.exit @@ -0,0 +1 @@ +1 diff --git a/tests/expected/214-arith-operand-type-check.txt b/tests/expected/214-arith-operand-type-check.txt new file mode 100644 index 0000000..da7e2e4 --- /dev/null +++ b/tests/expected/214-arith-operand-type-check.txt @@ -0,0 +1,11 @@ +error: cannot apply '+' to operands of type 's64' and 'string' + --> /Users/agra/projects/sx/examples/214-arith-operand-type-check.sx:12:10 + | +12 | x := n + s; // s64 + string — rejected + | ^ + +error: cannot apply '*' to operands of type 'string' and 's64' + --> /Users/agra/projects/sx/examples/214-arith-operand-type-check.sx:13:10 + | +13 | y := s * n; // string * s64 — rejected (non-numeric LHS too) + | ^