diff --git a/examples/214-arith-operand-type-check.sx b/examples/214-arith-operand-type-check.sx deleted file mode 100644 index 0171ce9..0000000 --- a/examples/214-arith-operand-type-check.sx +++ /dev/null @@ -1,15 +0,0 @@ -// 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/examples/214-binop-operand-type-check.sx b/examples/214-binop-operand-type-check.sx new file mode 100644 index 0000000..77d1455 --- /dev/null +++ b/examples/214-binop-operand-type-check.sx @@ -0,0 +1,25 @@ +// Scalar binary operators check operand-type compatibility. The result +// type is otherwise taken from the left operand, so mixing a non-numeric +// type (here `string`) would lower as ` : s64` and either reinterpret +// the string's bytes (arithmetic / bitwise → garbage) or feed mismatched +// types to `icmp` (ordering → LLVM verifier failure). All such mismatches +// are now rejected at compile time: +// - arithmetic `+ - * / %` (numeric / vector / pointer operands) +// - ordering `< <= > >=` (numeric / enum / pointer operands) +// - bitwise `& | ^` (integer / enum operands) +// - shift `<< >>` (integer / enum operands) +// Legitimate mixes (int+float promotion, flags-enum bitwise, enum/pointer +// comparison) are unaffected — see `examples/50-smoke.sx`. + +#import "modules/std.sx"; + +main :: () -> s32 { + n : s64 = 40; + s : string = "nope"; + a := n + s; // arithmetic: s64 + string + b := s * n; // arithmetic: non-numeric LHS (string * s64) + c := n < s; // ordering: s64 < string + d := n & s; // bitwise: s64 & string + e := n << s; // shift: s64 << string + 0; +} diff --git a/issues/0055-binary-arith-no-operand-type-check.md b/issues/0055-binary-arith-no-operand-type-check.md index a57fb6f..2dba552 100644 --- a/issues/0055-binary-arith-no-operand-type-check.md +++ b/issues/0055-binary-arith-no-operand-type-check.md @@ -1,21 +1,33 @@ # 0055 — binary arithmetic accepts mismatched operand types (`s64 + string`) -**FIXED** (`examples/214-arith-operand-type-check.sx`). `lowerBinaryOp` in +**FIXED** (`examples/214-binop-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`). +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: -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. +- **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 '' to operands of type '' and +''` 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 diff --git a/src/ir/lower.zig b/src/ir/lower.zig index f418d81..31c92da 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2732,12 +2732,20 @@ 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 => { + // Reject scalar ops on incompatible operand types (e.g. + // `s64 + string`, `s64 < string`, `s64 & string`). The result type + // `ty` is derived from the LHS, so without this the op lowers as + // ` : ` and either reinterprets the RHS bytes (arithmetic + // / bitwise → garbage) or feeds mismatched LLVM types to `icmp` + // (ordering → verifier failure). + { + const group: enum { none, arith, ordering, bitwise } = switch (bop.op) { + .add, .sub, .mul, .div, .mod => .arith, + .lt, .lte, .gt, .gte => .ordering, + .bit_and, .bit_or, .bit_xor, .shl, .shr => .bitwise, + else => .none, + }; + if (group != .none) { const eff_rhs_ty = blk: { if (rhs_ty == .unresolved) break :blk self.builder.getRefType(rhs); if (!rhs_ty.isBuiltin()) { @@ -2746,16 +2754,21 @@ pub const Lowering = struct { } break :blk rhs_ty; }; - if (!self.isArithOperand(ty) or !self.isArithOperand(eff_rhs_ty)) { + const ok = switch (group) { + .arith => self.isArithOperand(ty) and self.isArithOperand(eff_rhs_ty), + .ordering => self.isOrderingOperand(ty) and self.isOrderingOperand(eff_rhs_ty), + .bitwise => self.isBitwiseOperand(ty) and self.isBitwiseOperand(eff_rhs_ty), + .none => true, + }; + if (!ok) { 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"); + return self.emitPlaceholder("operand-type-mismatch"); } - }, - else => {}, + } } return switch (bop.op) { @@ -14660,6 +14673,35 @@ pub const Lowering = struct { }; } + /// Operands valid for ordering comparisons (`< <= > >=`): numbers + /// (incl. custom int widths), enums (ordinal), pointers (address + /// order), bool, and SIMD vectors. NOT strings (no lexicographic `<` + /// lowering exists) or any other aggregate. `.unresolved` passes so an + /// un-inferable operand is never falsely diagnosed. + fn isOrderingOperand(self: *Lowering, ty: TypeId) bool { + if (ty == .unresolved) return true; + if (isInt(ty) or isFloat(ty) or ty == .bool) return true; + if (ty.isBuiltin()) return false; + return switch (self.module.types.get(ty)) { + .signed, .unsigned, .@"enum", .pointer, .many_pointer, .vector => true, + else => false, + }; + } + + /// Operands valid for bitwise/shift ops (`& | ^ << >>`): integers + /// (incl. custom widths), enums (flags are int-backed), bool, and SIMD + /// vectors. NOT floats, strings, pointers, or aggregates. `.unresolved` + /// passes (see `isOrderingOperand`). + fn isBitwiseOperand(self: *Lowering, ty: TypeId) bool { + if (ty == .unresolved) return true; + if (isInt(ty) or ty == .bool) return true; + if (ty.isBuiltin()) return false; + return switch (self.module.types.get(ty)) { + .signed, .unsigned, .@"enum", .vector => true, + else => false, + }; + } + fn binOpSymbol(op: ast.BinaryOp.Op) []const u8 { return switch (op) { .add => "+", diff --git a/tests/expected/214-arith-operand-type-check.txt b/tests/expected/214-arith-operand-type-check.txt deleted file mode 100644 index da7e2e4..0000000 --- a/tests/expected/214-arith-operand-type-check.txt +++ /dev/null @@ -1,11 +0,0 @@ -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) - | ^ diff --git a/tests/expected/214-arith-operand-type-check.exit b/tests/expected/214-binop-operand-type-check.exit similarity index 100% rename from tests/expected/214-arith-operand-type-check.exit rename to tests/expected/214-binop-operand-type-check.exit diff --git a/tests/expected/214-binop-operand-type-check.txt b/tests/expected/214-binop-operand-type-check.txt new file mode 100644 index 0000000..f9655d4 --- /dev/null +++ b/tests/expected/214-binop-operand-type-check.txt @@ -0,0 +1,29 @@ +error: cannot apply '+' to operands of type 's64' and 'string' + --> /Users/agra/projects/sx/examples/214-binop-operand-type-check.sx:19:10 + | +19 | a := n + s; // arithmetic: s64 + string + | ^ + +error: cannot apply '*' to operands of type 'string' and 's64' + --> /Users/agra/projects/sx/examples/214-binop-operand-type-check.sx:20:10 + | +20 | b := s * n; // arithmetic: non-numeric LHS (string * s64) + | ^ + +error: cannot apply '<' to operands of type 's64' and 'string' + --> /Users/agra/projects/sx/examples/214-binop-operand-type-check.sx:21:10 + | +21 | c := n < s; // ordering: s64 < string + | ^ + +error: cannot apply '&' to operands of type 's64' and 'string' + --> /Users/agra/projects/sx/examples/214-binop-operand-type-check.sx:22:10 + | +22 | d := n & s; // bitwise: s64 & string + | ^ + +error: cannot apply '<<' to operands of type 's64' and 'string' + --> /Users/agra/projects/sx/examples/214-binop-operand-type-check.sx:23:10 + | +23 | e := n << s; // shift: s64 << string + | ^