diff --git a/.kiro/specs/calculator/design.md b/.kiro/specs/calculator/design.md index a41768c..f0aab2c 100644 --- a/.kiro/specs/calculator/design.md +++ b/.kiro/specs/calculator/design.md @@ -65,6 +65,7 @@ build.zig (workspace root) | `ast.zig` | AST node definitions | | `evaluator.zig` | Walk AST, produce results | | `programmer.zig` | Integer operations, base conversion, bit manipulation | +| `bitwise.zig` | The fixed-width operators (`& \| xor ~ << >> >>> rol ror`), shared by both modes | | `struct_layout.zig` | Struct DSL parser, layout computation, ABI profiles | | `units.zig` | Unit conversion tables and resolver | | `financial.zig` | CAGR, TVM, compound interest, amortization | @@ -183,6 +184,17 @@ keyword forms: `&`/`and`, `|`/`or`, `~`/`not`. XOR is keyword-only (`xor`) since `^` is reserved for power. Every operator means the same thing in standard and programmer modes. +**One implementation of the fixed-width operators.** `bitwise.zig` implements the +eight fixed-width binary operators plus `~` and two's complement negation, over a +`Domain` of width plus signedness. Standard mode passes `Domain.standard`, which is +64-bit signed and not configurable; programmer mode passes its configured width and +signedness. Values are `u128` bit patterns masked to the width, the representation +`types.Integer` already used. Both callers dispatch through an `inline else` prong +that resolves the operator at comptime, so an operator added to `BinaryOp` that +belongs in this tier fails to compile rather than falling through to the rational +tier. FR-2.13 covers the distance rules: shifts run to completion, rotations are +cyclic, negative distances are domain errors. + **No implicit multiplication**: adjacency (e.g. `2pi`) is an error; use explicit `*`. Spaces within hex/oct/bin literals are separators (e.g. `0xFF FF`). diff --git a/.kiro/specs/calculator/requirements.md b/.kiro/specs/calculator/requirements.md index fc77fb2..192c98d 100644 --- a/.kiro/specs/calculator/requirements.md +++ b/.kiro/specs/calculator/requirements.md @@ -25,7 +25,8 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-2.1**: Accept input in decimal, hexadecimal (`0x`), octal (`0o`), and binary (`0b`) formats. - **FR-2.2**: Simultaneously display results in all four bases (dec, hex, oct, bin). - **FR-2.3**: Support configurable bit widths: 8, 16, 32, 64, 128-bit. Standard mode is fixed at 64-bit signed; a width other than 64 is what programmer mode is for. -- **FR-2.4**: Support bitwise operators: AND (`&` or `and`), OR (`|` or `or`), XOR (`xor` keyword), NOT (`~` or `not`), left shift (`<<`), right shift (logical `>>>`), arithmetic right shift (`>>`), rotate left (`rol`), rotate right (`ror`). Note: `^` is always exponentiation (never XOR) - see FR-2.12. `>>` fills the vacated high bits with copies of the sign bit, so `-8 >> 1` is -4; `>>>` fills them with zeros. Bits shifted past the width are discarded rather than wrapped (`0b1000 << 1` is 16); `rol` and `ror` are the operators that wrap. +- **FR-2.4**: Support bitwise operators: AND (`&` or `and`), OR (`|` or `or`), XOR (`xor` keyword), NOT (`~` or `not`), left shift (`<<`), right shift (logical `>>>`), arithmetic right shift (`>>`), rotate left (`rol`), rotate right (`ror`). Note: `^` is always exponentiation (never XOR) - see FR-2.12. `>>` fills the vacated high bits with copies of the sign bit, so `-8 >> 1` is -4; `>>>` fills them with zeros. In an unsigned width there is no sign to extend, so `>>` fills with zeros there too. Bits shifted past the width are discarded rather than wrapped (`0b1000 << 1` is 16); `rol` and `ror` are the operators that wrap. +- **FR-2.13**: A shift distance at or beyond the bit width shifts every original bit out: `1 << 64` is 0 in 64-bit, and 8-bit `0xFF >>> 20` is 0. For `>>` the result is the sign fill, so a negative value shifted that far is -1 and a non-negative one is 0. Rotation instead reduces the distance modulo the width, because a rotation is cyclic: `1 rol 65` is `1 rol 1`. A negative shift or rotate distance is a domain error rather than a very large one. - **FR-2.12**: The `^` operator means exponentiation in all modes (never XOR). This avoids mode-dependent operator overloading. XOR is available only via the `xor` keyword. Power is also available via `**`. This keeps every operator's meaning identical across standard and programmer modes. - **FR-2.5**: Display both signed (two's complement) and unsigned interpretations of the current value. - **FR-2.6**: Visualize the bit pattern as a grid (integer.exposed style) - bits individually addressable/toggleable in TUI and Android. diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index 80cacb5..035e5bd 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -867,34 +867,43 @@ the review would have spent its time on): could never run, because only formatter `raw` strings carry a prefix. STILL OPEN, in the order I would take them: -1. `>>` is logical in standard mode and arithmetic in programmer mode, shift +1. ~~`>>` is logical in standard mode and arithmetic in programmer mode, shift amounts wrap in one and clamp in the other, and `evaluator.zig` ignores the - configured bit width entirely. The two implementations of these nine operators - need to become one, parameterised by width (FR-2.12 promises they agree). + configured bit width entirely.~~ FIXED. `engine/src/bitwise.zig` is the one + implementation of the eight fixed-width binary operators plus `~` and unary + minus, parameterised by a `Domain` (width plus signedness). `evaluator.zig` and + `programmer.zig` both call it from an `inline else` prong, so an operator added + to `BinaryOp` that belongs there is a compile error rather than a silent fall + through. Decisions taken, and the behaviour changes they caused: - DECIDED: standard mode is 64-bit signed. A user who wants another width uses - programmer mode, which keeps its configurable width (FR-2.3). The shared - implementation therefore takes width and signedness as parameters, and standard - mode passes 64 and signed. Signedness is the status quo rather than a change: - `evaluator.zig` already converts results back through `@as(i64, @bitCast(...))`, - so standard-mode `~0` is -1. + - Standard mode is 64-bit signed, fixed. A different width is what programmer + mode is for (FR-2.3). `~` used to read `env.programmer_config.bit_width` while + the shifts beside it ignored the setting, so `tally --bits 8 '~0'` gave 255 for + the complement and 64-bit answers for everything else. + - `>>` fills with the sign bit in both modes, `>>>` with zeros. Standard-mode + `-8 >> 1` is now -4 rather than 9223372036854775804. + - A shift distance at or beyond the width shifts everything out rather than + wrapping modulo 64 (standard) or clamping to `width - 1` (programmer): + `1 << 64` is 0, 8-bit `0xFF >>> 20` is 0, and a negative value shifted all the + way out is all sign bits, so `-8 >> 64` is -1. + - A negative shift or rotate distance is a `DomainError`. It used to be reduced + modulo the width, so `8 >> -1` quietly meant `8 >> 63`. + - Rotation stays cyclic, reducing the distance modulo the width, because that is + what a rotation is: `1 rol 65` is 2. + - An unsigned domain has no sign to extend, so `>>` is a zero fill there. The old + programmer-mode code looked at the top bit regardless of the configured + signedness, so 8-bit unsigned `0xFF >> 1` gave 0xFF instead of 0x7F. + - Bits shifted past the width are still discarded rather than wrapped + (`0b1000 << 1` is 16); `rol`/`ror` remain the operators that wrap. Zig's + saturating `<<|` was considered as a meaning for `<<<` and rejected: `>>>` is + the zero-filling variant of `>>`, so `<<<` would have to be the zero-filling + variant of `<<`, which is `<<` itself. - DECIDED: `>>` fills with the sign bit (arithmetic) in both modes, so standard - `-8 >> 1` becomes -4 instead of 9223372036854775804. `>>>` fills with zeros - (logical) in both modes. - - Bits shifted past the width are discarded, not wrapped: `0b1000 << 1` is 16. - Wrap-around is what `rol`/`ror` are for, and both modes already agree on those. - - STILL TO DECIDE: a shift distance at or beyond the width. Standard mode reduces - the distance modulo 64 (`1 << 64` is 1) and programmer mode clamps it to - `width - 1` (8-bit `0xFF >>> 20` shifts by 7 and gives 1). Both turn "shift - everything out" into "shift a little". Recommendation: let the shift run to - completion, so `<<` and `>>>` yield 0 and `>>` yields 0 or -1 by sign. That makes - `1 << 64` yield 0, which is a visible change with test expectations attached. - Zig's saturating `<<|` was considered as a home for `<<<` and rejected: `>>>` - means "the zero-filling variant of `>>`", so `<<<` would have to mean the - zero-filling variant of `<<`, which is `<<` itself. + Standard mode still projects operands through f64 before operating and converts + the result back, so a result above 2^53 is rounded: `-8 >>> 1` prints + 9.223372036854776e18 rather than 9223372036854775804. Making the fixed-width path + read and return exact integers is a separate change, worth doing with item 3 + below, and it would alter how large results display. 2. Signed division and modulo in programmer mode use unsigned semantics: `-10 / 2` gives 9223372036854775803. 3. Multi-base detail lines are computed through an f64 round trip, so diff --git a/engine/src/bitwise.zig b/engine/src/bitwise.zig new file mode 100644 index 0000000..1e244d7 --- /dev/null +++ b/engine/src/bitwise.zig @@ -0,0 +1,348 @@ +//! Fixed-width integer operations: the bitwise operators, the shifts and the +//! rotations, for both modes. +//! +//! These eight binary operators and two unary ones used to be implemented twice, +//! once in `evaluator.zig` over a 64-bit projection and once in `programmer.zig` +//! over the configured width, and the two disagreed: +//! +//! - `>>` was logical in standard mode and arithmetic in programmer mode, so +//! `-8 >> 1` was 9223372036854775804 in one and -4 in the other. +//! - A shift distance at or beyond the width wrapped modulo 64 in standard mode +//! (`1 << 64` was 1) and clamped to `width - 1` in programmer mode (8-bit +//! `0xFF >>> 20` shifted by 7 and gave 1). Both turned "shift everything out" +//! into "shift a little". +//! - Standard mode ignored the configured width for shifts while honouring it for +//! `~`. +//! +//! FR-2.12 promises that every operator means the same thing in both modes, so +//! there is one implementation, parameterised by a `Domain` (width plus +//! signedness). Standard mode is fixed at 64-bit signed; a different width is what +//! programmer mode is for (FR-2.3). +//! +//! Values are carried as a `u128` holding the two's complement bit pattern masked +//! to the width, which is the same representation `types.Integer` uses. + +const std = @import("std"); +const ast = @import("ast.zig"); +const BinaryOp = ast.BinaryOp; +const types = @import("types.zig"); +const BitWidth = types.BitWidth; +const Signedness = types.Signedness; +const ProgrammerConfig = types.ProgrammerConfig; +const CalcError = types.CalcError; + +/// The operators this module implements. +/// +/// A narrower set than `ast.BinaryOp`: the arithmetic operators are rational in +/// standard mode and wrapping in programmer mode, so they have nothing to share. +pub const Op = enum { + bit_and, + bit_or, + bit_xor, + shift_left, + /// Arithmetic right shift: vacated high bits take the value of the sign bit. + shift_right, + /// Logical right shift: vacated high bits are zero. + shift_right_logical, + rotate_left, + rotate_right, +}; + +/// The `Op` for a `BinaryOp`, or null for the arithmetic operators. +/// +/// Callers use this at comptime from an `inline else` prong, so an operator added +/// to `BinaryOp` that belongs here is a compile error rather than a silent fall +/// through to the wrong tier. +pub fn fromBinaryOp(op: BinaryOp) ?Op { + return switch (op) { + .bit_and => .bit_and, + .bit_or => .bit_or, + .bit_xor => .bit_xor, + .shift_left => .shift_left, + .shift_right => .shift_right, + .shift_right_logical => .shift_right_logical, + .rotate_left => .rotate_left, + .rotate_right => .rotate_right, + .add, .sub, .mul, .div, .mod, .pow => null, + }; +} + +/// The integer type an operation is carried out in: how wide, and whether the top +/// bit is a sign. +pub const Domain = struct { + bit_width: BitWidth, + signedness: Signedness, + + /// Standard mode: 64-bit two's complement. Fixed, not configurable; the + /// programmer-mode width setting does not reach standard mode. + pub const standard: Domain = .{ .bit_width = .bits64, .signedness = .signed }; + + pub fn fromConfig(config: ProgrammerConfig) Domain { + return .{ .bit_width = config.bit_width, .signedness = config.signedness }; + } + + pub fn mask(self: Domain) u128 { + return self.bit_width.mask(); + } + + pub fn bits(self: Domain) u8 { + return self.bit_width.bits(); + } + + /// The sign bit position, whether or not this domain treats it as a sign. + pub fn topBit(self: Domain) u128 { + return @as(u128, 1) << @intCast(self.bits() - 1); + } + + /// True when the pattern denotes a negative number in this domain. An unsigned + /// domain has no negative values, so `>>` is a zero fill there and a large + /// shift distance is large rather than negative. + pub fn isNegative(self: Domain, value: u128) bool { + return self.signedness == .signed and (value & self.mask()) & self.topBit() != 0; + } + + /// Sign-extend the pattern to a full i128. + pub fn signExtend(self: Domain, value: u128) i128 { + const m = value & self.mask(); + if (self.isNegative(m)) return @bitCast(m | ~self.mask()); + return @intCast(m); + } +}; + +/// How far a shift moves, once the distance has been checked. +const Distance = union(enum) { + /// Shorter than the width, so some bits survive. + within: u7, + /// At or beyond the width: every original bit leaves the value. + past_width, +}; + +/// Interpret the right operand of a shift as a distance. +/// +/// A negative distance is a domain error rather than a very large one. Standard +/// mode used to reduce it modulo 64, so `8 >> -1` quietly became `8 >> 63`. +fn distance(domain: Domain, right: u128) CalcError!Distance { + if (domain.isNegative(right)) return CalcError.DomainError; + const value = right & domain.mask(); + if (value >= domain.bits()) return .past_width; + return .{ .within = @intCast(value) }; +} + +/// Interpret the right operand of a rotation as a distance. +/// +/// Rotation is cyclic, so a distance beyond the width is reduced rather than +/// saturated: rotating a 64-bit value by 65 is rotating it by 1. +fn rotation(domain: Domain, right: u128) CalcError!u7 { + if (domain.isNegative(right)) return CalcError.DomainError; + return @intCast((right & domain.mask()) % domain.bits()); +} + +/// Apply a fixed-width operation. Operands and result are masked bit patterns. +pub fn apply(domain: Domain, op: Op, left_in: u128, right_in: u128) CalcError!u128 { + const mask = domain.mask(); + const left = left_in & mask; + const right = right_in & mask; + + return switch (op) { + .bit_and => left & right, + .bit_or => left | right, + .bit_xor => left ^ right, + // Bits shifted past the end of the width are discarded, not wrapped: + // `0b1000 << 1` is 16. Wrapping is what `rol` and `ror` are for. + .shift_left => switch (try distance(domain, right)) { + .past_width => 0, + .within => |amt| (left << amt) & mask, + }, + .shift_right_logical => switch (try distance(domain, right)) { + .past_width => 0, + .within => |amt| (left >> amt) & mask, + }, + .shift_right => blk: { + const negative = domain.isNegative(left); + switch (try distance(domain, right)) { + // Shifting a negative value all the way out leaves the sign fill, + // which is every bit set; a non-negative one leaves zero. + .past_width => break :blk if (negative) mask else 0, + .within => |amt| { + if (!negative) break :blk (left >> amt) & mask; + // Shift in the signed domain so the sign bit is the fill. + const extended = domain.signExtend(left); + break :blk @as(u128, @bitCast(extended >> amt)) & mask; + }, + } + }, + .rotate_left => blk: { + const amt = try rotation(domain, right); + if (amt == 0) break :blk left; + const anti: u7 = @intCast(domain.bits() - amt); + break :blk ((left << amt) | (left >> anti)) & mask; + }, + .rotate_right => blk: { + const amt = try rotation(domain, right); + if (amt == 0) break :blk left; + const anti: u7 = @intCast(domain.bits() - amt); + break :blk ((left >> amt) | (left << anti)) & mask; + }, + }; +} + +/// Bitwise complement within the width. +pub fn not(domain: Domain, value: u128) u128 { + return ~value & domain.mask(); +} + +/// Two's complement negation within the width. +pub fn negate(domain: Domain, value: u128) u128 { + return (~value +% 1) & domain.mask(); +} + +// -- Tests -- + +const testing = std.testing; + +const w8: Domain = .{ .bit_width = .bits8, .signedness = .signed }; +const u8_domain: Domain = .{ .bit_width = .bits8, .signedness = .unsigned }; + +test "fromBinaryOp: every fixed-width operator maps, no arithmetic one does" { + // The compiler enforces the total mapping; this pins which side each lands on. + try testing.expectEqual(Op.shift_right, fromBinaryOp(.shift_right).?); + try testing.expectEqual(Op.rotate_right, fromBinaryOp(.rotate_right).?); + for ([_]BinaryOp{ .add, .sub, .mul, .div, .mod, .pow }) |op| { + try testing.expect(fromBinaryOp(op) == null); + } + var fixed_width: usize = 0; + inline for (@typeInfo(BinaryOp).@"enum".fields) |field| { + if (fromBinaryOp(@field(BinaryOp, field.name)) != null) fixed_width += 1; + } + try testing.expectEqual(@as(usize, @typeInfo(Op).@"enum".fields.len), fixed_width); +} + +test "arithmetic right shift fills with the sign bit" { + // -8 in 8 bits is 0b1111_1000; one place right is 0b1111_1100, which is -4. + const result = try apply(w8, .shift_right, 0b1111_1000, 1); + try testing.expectEqual(@as(u128, 0b1111_1100), result); + try testing.expectEqual(@as(i128, -4), w8.signExtend(result)); +} + +test "logical right shift fills with zeros" { + // The same bits, shifted the other way: 0b0111_1100 is 124. + const result = try apply(w8, .shift_right_logical, 0b1111_1000, 1); + try testing.expectEqual(@as(u128, 124), result); +} + +test "the two right shifts agree on non-negative values" { + var value: u128 = 0; + while (value < 0x80) : (value += 1) { + var amt: u128 = 0; + while (amt < 8) : (amt += 1) { + try testing.expectEqual( + try apply(w8, .shift_right, value, amt), + try apply(w8, .shift_right_logical, value, amt), + ); + } + } +} + +test "an unsigned domain has no sign to extend" { + // 0xFF is 255 here, not -1, so the arithmetic shift is a zero fill too. The + // old programmer-mode implementation looked at the top bit regardless of the + // configured signedness and gave 0xFF. + try testing.expectEqual(@as(u128, 0x7F), try apply(u8_domain, .shift_right, 0xFF, 1)); + try testing.expectEqual(@as(u128, 0x7F), try apply(u8_domain, .shift_right_logical, 0xFF, 1)); +} + +test "shifts run to completion instead of wrapping or clamping the distance" { + // Standard mode reduced the distance modulo the width, so `1 << 64` was 1; + // programmer mode clamped it to width - 1, so 8-bit `0xFF >>> 20` was 1. + const standard_domain = Domain.standard; + try testing.expectEqual(@as(u128, 0), try apply(standard_domain, .shift_left, 1, 64)); + try testing.expectEqual(@as(u128, 0), try apply(standard_domain, .shift_left, 1, 1000)); + try testing.expectEqual(@as(u128, 0), try apply(w8, .shift_right_logical, 0xFF, 20)); + try testing.expectEqual(@as(u128, 0), try apply(w8, .shift_left, 0xFF, 8)); + + // A negative value shifted all the way out is all sign bits, not zero. + try testing.expectEqual(@as(u128, 0xFF), try apply(w8, .shift_right, 0b1111_1000, 8)); + try testing.expectEqual(@as(u128, 0xFF), try apply(w8, .shift_right, 0b1111_1000, 100)); + // A non-negative one is zero. + try testing.expectEqual(@as(u128, 0), try apply(w8, .shift_right, 0b0100_0000, 8)); +} + +test "shifting by one less than the width still keeps a bit" { + // The boundary the clamping rule used to hide. + try testing.expectEqual(@as(u128, 0b1000_0000), try apply(w8, .shift_left, 1, 7)); + try testing.expectEqual(@as(u128, 1), try apply(w8, .shift_right_logical, 0b1000_0000, 7)); + try testing.expectEqual(@as(u128, 0xFF), try apply(w8, .shift_right, 0b1000_0000, 7)); +} + +test "a negative shift distance is a domain error, not a huge one" { + const neg_one: u128 = 0xFF; // -1 in 8-bit signed + try testing.expectError(CalcError.DomainError, apply(w8, .shift_left, 1, neg_one)); + try testing.expectError(CalcError.DomainError, apply(w8, .shift_right, 1, neg_one)); + try testing.expectError(CalcError.DomainError, apply(w8, .shift_right_logical, 1, neg_one)); + try testing.expectError(CalcError.DomainError, apply(w8, .rotate_left, 1, neg_one)); + try testing.expectError(CalcError.DomainError, apply(w8, .rotate_right, 1, neg_one)); + // The same pattern in an unsigned domain is 255, a distance past the width. + try testing.expectEqual(@as(u128, 0), try apply(u8_domain, .shift_left, 1, neg_one)); +} + +test "rotation is cyclic and reduces the distance" { + try testing.expectEqual(@as(u128, 0b0000_0011), try apply(w8, .rotate_left, 0b1000_0001, 1)); + try testing.expectEqual(@as(u128, 0b1100_0000), try apply(w8, .rotate_right, 0b1000_0001, 1)); + // Rotating by the width is the identity, and by width + 1 is by 1. + try testing.expectEqual(@as(u128, 0b1000_0001), try apply(w8, .rotate_left, 0b1000_0001, 8)); + try testing.expectEqual(@as(u128, 0b0000_0011), try apply(w8, .rotate_left, 0b1000_0001, 9)); +} + +test "rotate left and rotate right are inverses at every distance and width" { + for ([_]BitWidth{ .bits8, .bits16, .bits32, .bits64, .bits128 }) |bw| { + const domain: Domain = .{ .bit_width = bw, .signedness = .unsigned }; + const value: u128 = 0x1234_5678_9ABC_DEF0 & domain.mask(); + var amt: u128 = 0; + while (amt < domain.bits()) : (amt += 1) { + const there = try apply(domain, .rotate_left, value, amt); + const back = try apply(domain, .rotate_right, there, amt); + try testing.expectEqual(value, back); + } + } +} + +test "results stay inside the width" { + for ([_]BitWidth{ .bits8, .bits16, .bits32, .bits64, .bits128 }) |bw| { + const domain: Domain = .{ .bit_width = bw, .signedness = .signed }; + const all_ones = domain.mask(); + inline for (@typeInfo(Op).@"enum".fields) |field| { + const op = @field(Op, field.name); + // 1 is a safe distance for the shifts and a legal operand for the rest. + const result = try apply(domain, op, all_ones, 1); + try testing.expectEqual(result, result & domain.mask()); + } + } +} + +test "not and negate stay inside the width" { + try testing.expectEqual(@as(u128, 0xFF), not(w8, 0)); + try testing.expectEqual(@as(u128, 0), not(w8, 0xFF)); + try testing.expectEqual(@as(u128, 0xFF), negate(w8, 1)); + try testing.expectEqual(@as(u128, 1), negate(w8, 0xFF)); + // Negating the most negative value gives itself back, as two's complement does. + try testing.expectEqual(@as(u128, 0x80), negate(w8, 0x80)); +} + +test "signExtend reads the top bit only when the domain is signed" { + try testing.expectEqual(@as(i128, -1), w8.signExtend(0xFF)); + try testing.expectEqual(@as(i128, 255), u8_domain.signExtend(0xFF)); + try testing.expectEqual(@as(i128, -128), w8.signExtend(0x80)); + try testing.expectEqual(@as(i128, 127), w8.signExtend(0x7F)); + // A 128-bit domain has no bits above the width to fill. + const w128: Domain = .{ .bit_width = .bits128, .signedness = .signed }; + try testing.expectEqual(@as(i128, -1), w128.signExtend(w128.mask())); +} + +test "standard mode is 64-bit signed" { + try testing.expectEqual(BitWidth.bits64, Domain.standard.bit_width); + try testing.expectEqual(Signedness.signed, Domain.standard.signedness); + // -8 >> 1 is -4 there, which is the case that used to differ between modes. + const minus_eight: u128 = @bitCast(@as(i128, -8) & @as(i128, @bitCast(Domain.standard.mask()))); + const shifted = try apply(Domain.standard, .shift_right, minus_eight, 1); + try testing.expectEqual(@as(i128, -4), Domain.standard.signExtend(shifted)); +} diff --git a/engine/src/engine.zig b/engine/src/engine.zig index b7bdf98..97c7fc8 100644 --- a/engine/src/engine.zig +++ b/engine/src/engine.zig @@ -10,6 +10,7 @@ pub const ast = @import("ast.zig"); pub const parser = @import("parser.zig"); pub const evaluator = @import("evaluator.zig"); pub const programmer = @import("programmer.zig"); +pub const bitwise = @import("bitwise.zig"); pub const formatter = @import("formatter.zig"); pub const float_interp = @import("float_interp.zig"); pub const units = @import("units.zig"); diff --git a/engine/src/evaluator.zig b/engine/src/evaluator.zig index e967023..0462b59 100644 --- a/engine/src/evaluator.zig +++ b/engine/src/evaluator.zig @@ -19,6 +19,7 @@ const parser_mod = @import("parser.zig"); const Parser = parser_mod.Parser; const number_mod = @import("number.zig"); const Number = number_mod.Number; +const bitwise = @import("bitwise.zig"); const financial = @import("financial.zig"); /// Evaluation environment holding variables, history, and config. @@ -165,12 +166,14 @@ fn evalExact(env: *Environment, scratch: Allocator, expr: *const Expr) CalcError return switch (u.op) { .negate => Number.negate(scratch, operand) catch |err| mapError(err), // Bitwise NOT is a fixed-width integer operation, not rational - // arithmetic, so it drops to the float/integer path. + // arithmetic, so it drops to the float/integer path. The width is + // the fixed standard-mode one: this used to read + // `env.programmer_config.bit_width`, so `tally --bits 8 '~0'` gave + // 255 in standard mode while the shifts alongside it ignored the + // setting entirely. .bitwise_not => blk: { const bits = try toFixedWidthBits(operand.toFloat(scratch)); - const mask_val: u64 = @truncate(env.programmer_config.bit_width.mask()); - const result = ~bits & mask_val; - break :blk Number.fromFloat(@floatFromInt(@as(i64, @bitCast(result)))); + break :blk fromFixedWidthBits(bitwise.not(standard_domain, bits)); }, }; }, @@ -221,76 +224,52 @@ fn evalBinaryOp(scratch: Allocator, op: BinaryOp, left: Number, right: Number) C .mod => Number.mod(scratch, left, right) catch |err| mapError(err), .pow => Number.pow(scratch, left, right) catch |err| mapError(err), // The remaining operators are fixed-width integer operations rather than - // rational arithmetic, so they work on the float/integer projection. - .bit_and => Number.fromFloat(try floatBitwise(left.toFloat(scratch), right.toFloat(scratch), bitwiseAnd)), - .bit_or => Number.fromFloat(try floatBitwise(left.toFloat(scratch), right.toFloat(scratch), bitwiseOr)), - .bit_xor => Number.fromFloat(try floatBitwise(left.toFloat(scratch), right.toFloat(scratch), bitwiseXor)), - .shift_left => Number.fromFloat(try floatShift(left.toFloat(scratch), right.toFloat(scratch), true)), - .shift_right, .shift_right_logical => Number.fromFloat(try floatShift(left.toFloat(scratch), right.toFloat(scratch), false)), - .rotate_left, .rotate_right => blk: { - // Rotations need bit width context; in standard mode, use 64-bit + // rational arithmetic, so they work on the 64-bit projection, in the shared + // implementation programmer mode also uses (FR-2.12). `inline else` + // resolves the operator at comptime, so an operator added to `BinaryOp` + // that `bitwise.fromBinaryOp` does not know is a compile error here. + inline else => |fixed_op| blk: { const l = try toFixedWidthBits(left.toFloat(scratch)); - const r = try shiftAmount(right.toFloat(scratch)); - const result = if (op == .rotate_left) - math.rotl(u64, l, r) - else - math.rotr(u64, l, r); - break :blk Number.fromFloat(@floatFromInt(@as(i64, @bitCast(result)))); + const r = try toFixedWidthBits(right.toFloat(scratch)); + const result = try bitwise.apply( + standard_domain, + comptime bitwise.fromBinaryOp(fixed_op).?, + l, + r, + ); + break :blk fromFixedWidthBits(result); }, }; } -/// Project a float onto the 64-bit integer domain the bitwise operators work in. +/// Standard mode's integer domain: 64-bit two's complement, fixed (FR-2.3). +/// +/// Standard mode does not consult the programmer-mode width. A width other than 64 +/// is what programmer mode is for, and pretending otherwise is how `~` came to +/// honour the setting while the shifts beside it did not. +const standard_domain = bitwise.Domain.standard; + +/// Project a float onto the integer domain the bitwise operators work in. /// /// Every one of these operators used to do `@intFromFloat` straight onto the /// unchecked value, which is illegal behaviour out of range and aborted the /// process: `2^64 and 1` and `~1e30` both killed it, and a NaN operand produced a -/// garbage answer instead. An operand that does not fit a machine word is a -/// reportable error, not a crash. -fn toFixedWidthBits(value: f64) CalcError!u64 { +/// garbage answer instead. An operand that does not fit the width is a reportable +/// error, not a crash. +fn toFixedWidthBits(value: f64) CalcError!u128 { if (!math.isFinite(value)) return CalcError.DomainError; // i64 covers [-2^63, 2^63); 2^63 itself is the first excluded value and is // exactly representable, so these bounds are exact. if (value >= 9223372036854775808.0 or value < -9223372036854775808.0) { return CalcError.Overflow; } - return @bitCast(@as(i64, @intFromFloat(value))); + const bits: u64 = @bitCast(@as(i64, @intFromFloat(value))); + return @as(u128, bits); } -/// The shift or rotate distance, reduced into 0..63. -/// -/// NOTE: reducing modulo the width is the behaviour standard mode has always had, -/// and it disagrees with programmer mode, which saturates. That divergence is a -/// separate open issue (the two implementations of these operators need to become -/// one); this only stops the conversion from being undefined. -fn shiftAmount(value: f64) CalcError!u6 { - const bits = try toFixedWidthBits(value); - const signed: i64 = @bitCast(bits); - return @intCast(@mod(signed, 64)); -} - -fn bitwiseAnd(a: u64, b: u64) u64 { - return a & b; -} -fn bitwiseOr(a: u64, b: u64) u64 { - return a | b; -} -fn bitwiseXor(a: u64, b: u64) u64 { - return a ^ b; -} - -fn floatBitwise(left: f64, right: f64, op: *const fn (u64, u64) u64) CalcError!f64 { - const l = try toFixedWidthBits(left); - const r = try toFixedWidthBits(right); - const result = op(l, r); - return @floatFromInt(@as(i64, @bitCast(result))); -} - -fn floatShift(left: f64, right: f64, is_left: bool) CalcError!f64 { - const l = try toFixedWidthBits(left); - const shift_amt = try shiftAmount(right); - const result = if (is_left) l << shift_amt else l >> shift_amt; - return @floatFromInt(@as(i64, @bitCast(result))); +/// Read a result pattern back as a number, signed, since standard mode is signed. +fn fromFixedWidthBits(bits: u128) Number { + return Number.fromFloat(@floatFromInt(standard_domain.signExtend(bits))); } /// Evaluate a built-in function call. @@ -864,6 +843,102 @@ test "eval bitwise not in standard mode" { try testing.expectEqual(@as(f64, -1.0), result); } +test "standard mode: the fixed-width operators use one implementation with programmer mode" { + // FR-2.12: an operator means the same thing in both modes. Standard mode is + // 64-bit signed (FR-2.3), so the same expression evaluated through the + // evaluator and through programmer.zig has to agree. Note that this compares + // the two real paths: `testEvalProgrammer` only sets the mode flag and still + // runs the evaluator, so it would have compared one implementation with itself. + const programmer_mod = @import("programmer.zig"); + const shared = [_][]const u8{ + "0xF0 and 0x0F", + "0xF0 or 0x0F", + "5 xor 3", + "1 << 10", + "1 << 63", + "1 << 64", + "1024 >> 4", + "1024 >>> 4", + "0 - 8 >> 1", + "1 rol 4", + "1 rol 65", + "16 ror 4", + "~0", + "~5", + }; + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + for (shared) |source| { + const standard = try testEval(source); + const prog = try programmer_mod.evalProgrammerString(alloc, source, .{ + .bit_width = .bits64, + .signedness = .signed, + }); + try testing.expectEqual(@as(i128, @intFromFloat(standard)), prog.signedValue()); + } +} + +test "standard mode: >> is arithmetic and >>> is logical" { + // This is the case that used to differ: standard mode had only a logical shift, + // so `-8 >> 1` was 9223372036854775804 rather than -4. + try testing.expectEqual(@as(f64, -4.0), try testEval("0 - 8 >> 1")); + try testing.expectEqual(@as(f64, -1.0), try testEval("0 - 1 >> 1")); + try testing.expectEqual(@as(f64, -64.0), try testEval("0 - 128 >> 1")); + // The zero-filling variant is still available and still huge. + try testing.expect(try testEval("0 - 8 >>> 1") > 9.0e18); + // Non-negative values shift identically either way. + try testing.expectEqual(try testEval("1024 >> 4"), try testEval("1024 >>> 4")); +} + +test "standard mode: a shift runs to completion instead of wrapping the distance" { + // The distance used to be reduced modulo 64, so `1 << 64` was `1 << 0`. + try testing.expectEqual(@as(f64, 0.0), try testEval("1 << 64")); + try testing.expectEqual(@as(f64, 0.0), try testEval("1 << 65")); + try testing.expectEqual(@as(f64, 0.0), try testEval("1 << 1000")); + try testing.expectEqual(@as(f64, 0.0), try testEval("1024 >>> 64")); + // A negative value shifted all the way out is all sign bits, which is -1. + try testing.expectEqual(@as(f64, -1.0), try testEval("0 - 8 >> 64")); + // One place short of the width still keeps a bit: 1 << 63 is the sign bit. + try testing.expect(try testEval("1 << 63") < 0.0); +} + +test "standard mode: a negative shift distance is a domain error" { + // It used to be reduced modulo 64, so `8 >> -1` quietly became `8 >> 63`. + try testing.expectError(CalcError.DomainError, testEval("8 >> 0 - 1")); + try testing.expectError(CalcError.DomainError, testEval("8 << 0 - 1")); + try testing.expectError(CalcError.DomainError, testEval("8 >>> 0 - 1")); + try testing.expectError(CalcError.DomainError, testEval("8 rol 0 - 1")); +} + +test "standard mode: rotation is cyclic, not clamped" { + try testing.expectEqual(@as(f64, 1.0), try testEval("1 rol 64")); + try testing.expectEqual(@as(f64, 2.0), try testEval("1 rol 65")); + try testing.expectEqual(@as(f64, 1.0), try testEval("1 ror 64")); +} + +test "standard mode: the programmer width setting does not reach it" { + // `~` used to read env.programmer_config.bit_width while the shifts beside it + // ignored it, so `--bits 8` changed one and not the other. Standard mode is + // fixed at 64-bit signed. + var env = Environment.init(testing.allocator, .standard); + defer env.deinit(); + env.programmer_config.bit_width = .bits8; + + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var not_zero = try evalString(&env, alloc, "~0"); + defer not_zero.deinit(); + try testing.expectEqual(@as(f64, -1.0), not_zero.toFloat(alloc)); + + var shifted = try evalString(&env, alloc, "1 << 10"); + defer shifted.deinit(); + try testing.expectEqual(@as(f64, 1024.0), shifted.toFloat(alloc)); +} + test "eval rotate left in standard mode" { // 1 rol 4 = 16 (for 64-bit) const result = try testEvalProgrammer("1 rol 4"); diff --git a/engine/src/programmer.zig b/engine/src/programmer.zig index 366ffb9..6963e67 100644 --- a/engine/src/programmer.zig +++ b/engine/src/programmer.zig @@ -15,6 +15,7 @@ const ProgrammerConfig = types.ProgrammerConfig; const CalcError = types.CalcError; const parser_mod = @import("parser.zig"); const Parser = parser_mod.Parser; +const bitwise = @import("bitwise.zig"); /// Evaluate an AST in programmer mode, producing an exact integer result. pub fn evalProgrammer(config: ProgrammerConfig, expr: *const Expr) CalcError!Integer { @@ -66,13 +67,10 @@ fn evalExpr(config: ProgrammerConfig, expr: *const Expr) CalcError!u128 { }, .unary => |u| { const operand = try evalExpr(config, u.operand); + const domain = bitwise.Domain.fromConfig(config); return switch (u.op) { - .negate => blk: { - // Two's complement negation: ~x + 1, masked to width - const negated = (~operand +% 1) & config.bit_width.mask(); - break :blk negated; - }, - .bitwise_not => (~operand) & config.bit_width.mask(), + .negate => bitwise.negate(domain, operand), + .bitwise_not => bitwise.not(domain, operand), }; }, .binary => |b| { @@ -88,9 +86,13 @@ fn evalExpr(config: ProgrammerConfig, expr: *const Expr) CalcError!u128 { } /// Evaluate a binary operation on two u128 values, masked to bit width. +/// +/// The bitwise operators, shifts and rotations are not here: they live in +/// `bitwise.zig`, which standard mode uses too, so the two modes cannot drift +/// apart again. What remains is the arithmetic, which genuinely differs between the +/// modes: it wraps at the width here and is exact rational arithmetic there. fn evalBinaryOp(config: ProgrammerConfig, op: BinaryOp, left: u128, right: u128) CalcError!u128 { const mask = config.bit_width.mask(); - const width = config.bit_width.bits(); const result: u128 = switch (op) { .add => (left +% right) & mask, @@ -115,52 +117,15 @@ fn evalBinaryOp(config: ProgrammerConfig, op: BinaryOp, left: u128, right: u128) } break :blk acc; }, - .bit_and => left & right, - .bit_or => left | right, - .bit_xor => left ^ right, - .shift_left => blk: { - const shift_amt: u7 = if (right >= width) - @intCast(width - 1) - else - @intCast(right); - break :blk (left << shift_amt) & mask; - }, - .shift_right => blk: { - // Arithmetic right shift: preserves sign bit - const shift_amt: u7 = if (right >= width) - @intCast(width - 1) - else - @intCast(right); - // Sign-extend, shift, then mask - const sign_bit: u128 = @as(u128, 1) << @intCast(width - 1); - if (left & sign_bit != 0) { - // Negative: fill with 1s from the top - const extended = left | ~mask; - const shifted: u128 = @bitCast(@as(i128, @bitCast(extended)) >> shift_amt); - break :blk shifted & mask; - } - break :blk (left >> shift_amt) & mask; - }, - .shift_right_logical => blk: { - // Logical right shift: always fills with 0s - const shift_amt: u7 = if (right >= width) - @intCast(width - 1) - else - @intCast(right); - break :blk (left >> shift_amt) & mask; - }, - .rotate_left => blk: { - const amt: u7 = @intCast(@mod(right, width)); - if (amt == 0) break :blk left; - const anti: u7 = @intCast(width - amt); - break :blk ((left << amt) | (left >> anti)) & mask; - }, - .rotate_right => blk: { - const amt: u7 = @intCast(@mod(right, width)); - if (amt == 0) break :blk left; - const anti: u7 = @intCast(width - amt); - break :blk ((left >> amt) | (left << anti)) & mask; - }, + // The fixed-width operators, in the shared implementation. `inline else` + // resolves the operator at comptime, so an operator added to `BinaryOp` + // that `bitwise.fromBinaryOp` does not know is a compile error here. + inline else => |fixed_op| try bitwise.apply( + bitwise.Domain.fromConfig(config), + comptime bitwise.fromBinaryOp(fixed_op).?, + left, + right, + ), }; return result; @@ -298,10 +263,14 @@ test "prog: shift left" { try testing.expectEqual(@as(u128, 256), result.unsignedValue()); } -test "prog: shift left overflow 8-bit" { +test "prog: shift left past the width shifts everything out" { + // The distance used to be clamped to width - 1, so this gave 128. const result = try testProgWith("1 << 8", .{ .bit_width = .bits8 }); - // Shifting by width or more in 8-bit: shift_amt clamped to 7 - try testing.expectEqual(@as(u128, 128), result.unsignedValue()); + try testing.expectEqual(@as(u128, 0), result.unsignedValue()); + + // One less than the width still keeps the bit. + const edge = try testProgWith("1 << 7", .{ .bit_width = .bits8 }); + try testing.expectEqual(@as(u128, 128), edge.unsignedValue()); } test "prog: logical shift right" { @@ -440,21 +409,27 @@ test "prog: float literal truncates to integer" { try testing.expectEqual(@as(u128, 3), result.unsignedValue()); } -test "prog: arithmetic shift right amount >= width clamps" { - // 0xFF in 8-bit is negative; >> 20 clamps shift to width-1 (7), - // arithmetic shift fills with sign bit -> stays 0xFF +test "prog: arithmetic shift right past the width leaves the sign fill" { + // 0xFF in 8-bit is -1; shifting a negative value all the way out leaves every + // bit set, which is still -1. The distance used to be clamped to width - 1, + // which reached the same answer here for the wrong reason. const result = try testProgWith("0xFF >> 20", .{ .bit_width = .bits8 }); try testing.expectEqual(@as(u128, 0xFF), result.unsignedValue()); + try testing.expectEqual(@as(i128, -1), result.signedValue()); } -test "prog: logical shift right amount >= width clamps" { - // 0xFF in 8-bit; >>> 20 clamps shift to width-1 (7) -> 0x01 +test "prog: logical shift right past the width shifts everything out" { + // Used to clamp the distance to 7 and give 1. const result = try testProgWith("0xFF >>> 20", .{ .bit_width = .bits8 }); - try testing.expectEqual(@as(u128, 0x01), result.unsignedValue()); + try testing.expectEqual(@as(u128, 0), result.unsignedValue()); + + // One less than the width still keeps the bottom bit. + const edge = try testProgWith("0xFF >>> 7", .{ .bit_width = .bits8 }); + try testing.expectEqual(@as(u128, 1), edge.unsignedValue()); } -test "prog: arithmetic shift right amount >= width positive value" { - // 0x40 in 8-bit is positive; >> 20 clamps to 7 -> 0 +test "prog: arithmetic shift right past the width, positive value" { + // 0x40 in 8-bit is positive, so the fill is zeros and everything shifts out. const result = try testProgWith("0x40 >> 20", .{ .bit_width = .bits8 }); try testing.expectEqual(@as(u128, 0), result.unsignedValue()); }