diff --git a/.kiro/specs/calculator/design.md b/.kiro/specs/calculator/design.md index 9691b0b..a41768c 100644 --- a/.kiro/specs/calculator/design.md +++ b/.kiro/specs/calculator/design.md @@ -187,6 +187,12 @@ thing in standard and programmer modes. use explicit `*`. Spaces within hex/oct/bin literals are separators (e.g. `0xFF FF`). +**Parsing does not depend on the mode.** `Tokenizer.init(source)` and +`Parser.init(allocator, source)` take no `Mode`: the same text produces the same +tree in standard and programmer mode, and only evaluation differs. Both used to +accept and store a `Mode` that no code read, which implied a mode-dependent grammar +that does not exist. + ### 2.5 Evaluation The evaluator maintains an `Environment`: @@ -199,9 +205,6 @@ pub const Environment = struct { /// expression had: `X = 0.1` stores exactly one tenth. variables: std.StringHashMap(Number), ans: ?Number, - /// A counter only. Displayed history lives in the frontend (`src/tui.zig`), - /// because the engine has no I/O and no display concerns. - history_len: usize, programmer_config: ProgrammerConfig, }; @@ -1959,7 +1962,21 @@ pub const ErrorInfo = struct { }; ``` -All engine functions return `CalcError!Result`. Frontends translate these into user-facing messages with position highlighting. +NOT IMPLEMENTED, and removed: nothing ever constructed an `ErrorInfo`, and the +parser's `error_pos`/`had_error` fields that would have fed it were written on every +error path and never read. Adding position reporting means threading it through the +`CalcError` returns, which is worth doing deliberately rather than leaving a +half-built shape in the code. + +The phrase for each error lives in exactly one place, `types.errorPhrase`, whose +switch has no `else`, so a new member of `CalcError` fails to compile until it is +given a phrase. Frontends decorate that phrase at comptime (`switch (err) { inline +else => ... }`): the CLI adds `error: ` and a newline, the TUI adds `error: `, and a +view with better context can override individual cases, as the financial form does +for `DomainError`. Each frontend used to carry its own copy of the whole table, and +the TUI's had drifted three errors behind. + +All engine functions return `CalcError!Result`. Frontends translate these into user-facing messages. --- diff --git a/.kiro/specs/calculator/requirements.md b/.kiro/specs/calculator/requirements.md index 6e234e4..fc77fb2 100644 --- a/.kiro/specs/calculator/requirements.md +++ b/.kiro/specs/calculator/requirements.md @@ -13,7 +13,7 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-1.1**: Parse and evaluate infix mathematical expressions with correct operator precedence (PEMDAS). - **FR-1.2**: Support operators: `+`, `-`, `*`, `/`, `%` (modulo), `^` (power), unary `-`. - **FR-1.3**: Support parentheses for grouping. -- **FR-1.4**: Support built-in functions: `sin`, `cos`, `tan`, `asin`, `acos`, `atan`, `log` (base-10), `ln` (natural), `sqrt`, `cbrt`, `abs`, `ceil`, `floor`, `round`, `factorial`. +- **FR-1.4**: Support built-in functions: `sin`, `cos`, `tan`, `asin`, `acos`, `atan`, `log` (base-10), `ln` (natural), `sqrt`, `cbrt`, `abs`, `ceil`, `floor`, `round`, `factorial`. An argument outside a function's domain is a domain error, distinct from an unknown name: `sqrt(-1)`, `asin(2)`, `ln(0)`, `log2(0)` and `factorial(-1)` all report a domain error, and only an unrecognized name reports an unknown function. - **FR-1.5**: Support constants: `pi`, `e`, `tau`. - **FR-1.6**: Support variable storage: `Ans` for the last result, plus any identifier as a named variable. (The original wording restricted this to `A-F, X, Y, Z`; the implementation accepts any name, which is a superset and the better behaviour, so the requirement follows the code.) Assignment to a constant name (`pi`, `e`, `tau`, `Ans`) is currently accepted and then ignored, which is a known defect rather than intended behaviour. - **FR-1.7**: Maintain calculation history with replay capability. @@ -24,8 +24,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. -- **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. +- **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.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 33fa3fc..80cacb5 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -828,11 +828,73 @@ DELIBERATELY NOT FIXED, documentation corrected instead: shapes), design 7's programmer and conversion output, design 8.1's footer, and design 8.0/8.0.0's overclaims about hit regions and the render harness. +DE-DUPLICATION PASS (done before the human review, since duplicated logic is what +the review would have spent its time on): +- **One money formatter.** `formatter.formatAmount`/`formatMoney` in the engine; + `main.zig` and `src/tui/financial.zig` had a copy each, and all three disagreed + about overflow. The formatter returns `null` rather than `"?"`, so the CLI now + reports "an amount in this schedule is too large to format" with exit status 1 + where it used to print a table of question marks and exit 0 (open item 9 below, + now closed). +- **One error phrase table.** `types.errorPhrase` owns the strings, with an + exhaustive switch and no `else`, so an added `CalcError` is a compile error. The + CLI and TUI decorate it at comptime (`switch (err) { inline else => ... }`). The + TUI's private copy had fallen behind and rendered `InvalidExpression`, + `ConvergenceFailure` and `InsufficientParameters` as "evaluation error". +- **One comma-grouping implementation and one scientific renderer.** + `writeUnsignedWithCommas` now writes plain digits and calls `writeGroupedDecimal`; + new `splitDecimalText` is the only place decimal text is taken apart. The 65-line + `scientificFromDecimalText` is gone: both magnitude branches call + `Rational.toScientificString`. +- **One negative-sqrt rule, one numeric-error mapping.** `Number.sqrt` raises + `NegativeRoot`; `number.toCalcError` is the single mapping (`evaluator.mapError` + and `units.mapNumberError` were two copies). `evalSingleArgFn` now distinguishes + "unknown name" from "bad argument", so `sqrt(-1)`, `asin(2)`, `ln(0)`, + `factorial(-1)` and `log2(0)` report `DomainError` instead of `UnknownFunction`. +- **One mode order, one form order.** `tui.mode_tabs` is indexed by the `Mode` tag + (checked at comptime) and drives Tab, Shift-Tab and the drawn bar, which were + three encodings of one sequence. Form cycling moved into + `financial.State.nextForm`/`prevForm`. That copy had a live bug: the `Form` tag is + a `u2`, so `@intFromEnum(form) + 1` overflowed and pressing Right on Amortization + panicked in a debug build. The existing test only wrapped backwards. +- **Dead code.** `engine.zig` lost 20 of 31 curated re-exports (the ones with no + caller, including `Value` after `Number` replaced it); `types.Value` and + `types.ErrorInfo` are gone; `Parser` lost `mode`, `previous`, `had_error` and + `error_pos` and `Tokenizer` lost `mode`, all written and never read, so both + `init` signatures dropped their `Mode` parameter (parsing does not depend on the + mode, only evaluation does); `Number.applyFloatFn` and `Environment.history_len` + are gone; and the `0x`/`0o`/`0b` prefix-skipping branches in the programmer view + 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 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). + + 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. + + 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. 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 @@ -849,7 +911,8 @@ STILL OPEN, in the order I would take them: 8. History display caps at 512 flattened lines built oldest-first, so results stop appearing after roughly 102 detailed entries. History memory is never reclaimed (Ctrl-L frees into an arena). -9. Money formatting degrades to `?` and still exits 0 at large magnitudes. +9. ~~Money formatting degrades to `?` and still exits 0 at large magnitudes.~~ + Fixed by the de-duplication pass above. 10. Assignment parses in prefix position (`1 + x = 2` mutates `x`), and assignment to a constant name is silently discarded. 11. Literals longer than 128 characters are rejected by a fixed tokenizer buffer, diff --git a/engine/src/engine.zig b/engine/src/engine.zig index 7b28e77..b7bdf98 100644 --- a/engine/src/engine.zig +++ b/engine/src/engine.zig @@ -14,51 +14,26 @@ pub const formatter = @import("formatter.zig"); pub const float_interp = @import("float_interp.zig"); pub const units = @import("units.zig"); pub const financial = @import("financial.zig"); -// Exact numeric model (design.md 2.7). Not yet wired into the evaluator; see -// Task 2.0b. Exported here so its tests run as part of `zig build test`. +// Exact numeric model (design.md 2.7). The evaluator computes in these. pub const rational = @import("rational.zig"); pub const number = @import("number.zig"); -// Re-export primary types for convenience -pub const Value = types.Value; +// The modules above are the engine's surface: a caller writes `engine.units.convert` +// or `engine.financial.solveTvm`. The aliases below exist only for the handful of +// names used often enough that the module prefix is noise. There used to be a +// curated re-export of nearly every public declaration, which drifted: two thirds +// of it had no callers, and `Value` was re-exported after the type it named had +// stopped being the engine's result type. pub const Mode = types.Mode; pub const BitWidth = types.BitWidth; pub const CalcError = types.CalcError; -pub const Parser = parser.Parser; -pub const Expr = ast.Expr; pub const Environment = evaluator.Environment; pub const evalString = evaluator.evalString; pub const evalStringInfo = evaluator.evalStringInfo; -pub const EvalInfo = evaluator.EvalInfo; pub const evalProgrammerString = programmer.evalProgrammerString; - -// Float interpretation pub const FloatFormat = float_interp.FloatFormat; -pub const FloatClass = float_interp.FloatClass; -pub const FloatInfo = float_interp.FloatInfo; - -// Unit conversion pub const UnitCategory = units.UnitCategory; pub const UnitDef = units.UnitDef; -pub const ConvertResult = units.ConvertResult; -pub const convert = units.convert; -pub const findUnit = units.findUnit; - -// Financial -pub const TvmVariable = financial.TvmVariable; -pub const TvmParams = financial.TvmParams; -pub const TvmSolution = financial.TvmSolution; -pub const solveTvm = financial.solveTvm; -pub const cagr = financial.cagr; -pub const compoundFutureValue = financial.compoundFutureValue; -pub const compoundPresentValue = financial.compoundPresentValue; -pub const compoundRate = financial.compoundRate; -pub const compoundPeriods = financial.compoundPeriods; -pub const effectiveAnnualRate = financial.effectiveAnnualRate; -pub const roundToCents = financial.roundToCents; - -// Exact numeric model -pub const Rational = rational.Rational; pub const Number = number.Number; test { diff --git a/engine/src/evaluator.zig b/engine/src/evaluator.zig index 006b897..e967023 100644 --- a/engine/src/evaluator.zig +++ b/engine/src/evaluator.zig @@ -32,7 +32,6 @@ pub const Environment = struct { programmer_config: ProgrammerConfig, variables: std.StringHashMap(Number), ans: Number, - history_len: usize, pub fn init(allocator: Allocator, mode: Mode) Environment { return .{ @@ -43,7 +42,6 @@ pub const Environment = struct { // Starts inexact so that `init` cannot fail; the first evaluation // replaces it. .ans = Number.fromFloat(0), - .history_len = 0, }; } @@ -209,15 +207,9 @@ fn literalToNumber(scratch: Allocator, n: ast.Expr.Number) CalcError!Number { } /// Map the numeric model's errors onto the engine's error set. -fn mapError(err: number_mod.Error) CalcError { - return switch (err) { - error.OutOfMemory => CalcError.OutOfMemory, - error.DivisionByZero => CalcError.DivisionByZero, - error.InvalidNumber => CalcError.InvalidNumber, - // An exponent too large to compute is an overflow from the caller's view. - error.ExponentTooLarge => CalcError.Overflow, - }; -} +/// +/// One mapping, in `number.zig`; this alias keeps the call sites short. +const mapError = number_mod.toCalcError; /// Evaluate a binary operation. fn evalBinaryOp(scratch: Allocator, op: BinaryOp, left: Number, right: Number) CalcError!Number { @@ -321,17 +313,19 @@ fn evalFunction(env: *Environment, scratch: Allocator, name: []const u8, args: [ return Number.round(scratch, x) catch |err| mapError(err); } if (std.mem.eql(u8, name, "sqrt")) { - // Negative inputs are a domain error rather than a NaN. - if (x.isNegative()) return CalcError.UnknownFunction; + // The negative-input rule lives in Number.sqrt, which raises + // NegativeRoot; mapError turns that into a domain error. return Number.sqrt(scratch, x) catch |err| mapError(err); } if (std.mem.eql(u8, name, "factorial")) { const result = Number.factorial(scratch, x) catch |err| return mapError(err); - return result orelse CalcError.UnknownFunction; + // Null means the argument was negative or fractional, which is a domain + // error, not an unknown function. + return result orelse CalcError.DomainError; } // Everything else escapes the rationals, so it falls back to f64. - const f = evalSingleArgFn(name, x.toFloat(scratch)) orelse + const f = try evalSingleArgFn(name, x.toFloat(scratch)) orelse return CalcError.UnknownFunction; return Number.fromFloat(f); } @@ -499,23 +493,39 @@ fn evalFinancialFn(name: []const u8, a: []const f64) CalcError!?f64 { } /// Evaluate a single-argument built-in function that has no exact form. -fn evalSingleArgFn(name: []const u8, x: f64) ?f64 { +/// Evaluate a single-argument built-in that has no exact form. +/// +/// Returns null when `name` is not one of these functions, and an error when the +/// name is known but the argument is outside its domain. The two used to be the +/// same answer (null), so the caller reported `asin(2)` as "unknown function". +fn evalSingleArgFn(name: []const u8, x: f64) CalcError!?f64 { if (std.mem.eql(u8, name, "sin")) return @sin(x); if (std.mem.eql(u8, name, "cos")) return @cos(x); if (std.mem.eql(u8, name, "tan")) return @tan(x); if (std.mem.eql(u8, name, "asin")) { - if (x < -1 or x > 1) return null; // domain error + if (x < -1 or x > 1) return CalcError.DomainError; return math.asin(x); } if (std.mem.eql(u8, name, "acos")) { - if (x < -1 or x > 1) return null; + if (x < -1 or x > 1) return CalcError.DomainError; return math.acos(x); } if (std.mem.eql(u8, name, "atan")) return math.atan(x); - if (std.mem.eql(u8, name, "log")) return @log10(x); - if (std.mem.eql(u8, name, "log10")) return @log10(x); - if (std.mem.eql(u8, name, "ln")) return @log(x); - if (std.mem.eql(u8, name, "log2")) return @log2(x); + // log/log10/ln/log2 of a non-positive value has no real result. The two-argument + // log already reported this as a domain error; the one-argument forms returned + // -inf or NaN. + if (std.mem.eql(u8, name, "log") or std.mem.eql(u8, name, "log10")) { + if (x <= 0) return CalcError.DomainError; + return @log10(x); + } + if (std.mem.eql(u8, name, "ln")) { + if (x <= 0) return CalcError.DomainError; + return @log(x); + } + if (std.mem.eql(u8, name, "log2")) { + if (x <= 0) return CalcError.DomainError; + return @log2(x); + } if (std.mem.eql(u8, name, "cbrt")) return math.cbrt(x); if (std.mem.eql(u8, name, "exp")) return @exp(x); return null; @@ -540,7 +550,7 @@ pub fn evalString(env: *Environment, allocator: Allocator, source: []const u8) C /// Like evalString but returns metadata (whether the expression used /// non-decimal literals) so frontends can decide to show a multi-base view. pub fn evalStringInfo(env: *Environment, allocator: Allocator, source: []const u8) CalcError!EvalInfo { - var p = Parser.init(allocator, source, env.mode); + var p = Parser.init(allocator, source); const expr = try p.parse(); // The parser hands over ownership. Nothing in the result borrows from the // tree (literal text points into `source`, and the value is cloned out of the @@ -559,7 +569,6 @@ pub fn evalStringInfo(env: *Environment, allocator: Allocator, source: []const u const result = raw.cloneWith(allocator) catch |err| return mapError(err); env.setAns(result) catch return CalcError.OutOfMemory; - env.history_len += 1; return .{ .value = result, .has_nondecimal_literal = hasNonDecimalLiteral(expr), @@ -881,10 +890,37 @@ test "eval log domain error" { try testing.expectError(CalcError.DomainError, result); } -test "eval asin domain error" { - const result = testEval("asin(2)"); - // asin(2) is domain error since |2| > 1 - try testing.expectError(CalcError.UnknownFunction, result); +test "domain errors are domain errors, not unknown functions" { + // These pinned the wrong contract: the name is known, the argument is not in + // its domain. Reporting "unknown function" sent the user looking for a typo. + try testing.expectError(CalcError.DomainError, testEval("asin(2)")); + try testing.expectError(CalcError.DomainError, testEval("asin(-2)")); + try testing.expectError(CalcError.DomainError, testEval("acos(2)")); + try testing.expectError(CalcError.DomainError, testEval("sqrt(-1)")); + try testing.expectError(CalcError.DomainError, testEval("factorial(-1)")); + try testing.expectError(CalcError.DomainError, testEval("factorial(2.5)")); + // Logarithms of non-positive values, which used to return -inf or NaN. The + // two-argument form already reported this correctly. + try testing.expectError(CalcError.DomainError, testEval("ln(0)")); + try testing.expectError(CalcError.DomainError, testEval("ln(0 - 1)")); + try testing.expectError(CalcError.DomainError, testEval("log(0)")); + try testing.expectError(CalcError.DomainError, testEval("log10(0 - 5)")); + try testing.expectError(CalcError.DomainError, testEval("log2(0)")); + try testing.expectError(CalcError.DomainError, testEval("log(100, 1)")); + + // A genuinely unknown name still reports one. + try testing.expectError(CalcError.UnknownFunction, testEval("nope(1)")); + try testing.expectError(CalcError.UnknownFunction, testEval("asin(1, 2)")); +} + +test "the functions themselves still work inside their domains" { + try testing.expectApproxEqAbs(@as(f64, 0.0), try testEval("asin(0)"), 1e-15); + try testing.expectApproxEqAbs(math.pi / 2.0, try testEval("acos(0)"), 1e-15); + try testing.expectEqual(@as(f64, 2.0), try testEval("log10(100)")); + try testing.expectApproxEqAbs(@as(f64, 1.0), try testEval("ln(e)"), 1e-15); + try testing.expectEqual(@as(f64, 3.0), try testEval("log2(8)")); + try testing.expectEqual(@as(f64, 120.0), try testEval("factorial(5)")); + try testing.expectEqual(@as(f64, 12.0), try testEval("sqrt(144)")); } test "eval acos" { diff --git a/engine/src/formatter.zig b/engine/src/formatter.zig index 54cb775..0f871c4 100644 --- a/engine/src/formatter.zig +++ b/engine/src/formatter.zig @@ -132,17 +132,20 @@ pub fn formatNumber(allocator: std.mem.Allocator, value: Number) !NumberDisplay // Very long values are abbreviated for display only. The `raw` // (clipboard) form always keeps every digit, so the exact value is // never actually lost, just not shown inline. + // + // Both this and the small-magnitude case below go through + // `toScientificString`. There used to be a second renderer here that + // worked on the already-rendered text; two implementations of the same + // notation is one more than needed, and the rational-based one handles + // both ends of the range. if (integerDigitCount(rendered.text) > max_display_integer_digits) { - const display = try scientificFromDecimalText(allocator, rendered.text); + const display = try r.toScientificString(allocator, scientific_significant_digits); return .{ .display = display, .raw = rendered.text, .exact = false }; } // The other end of the same problem: a value smaller than the // fractional budget renders as all zeros, which destroys it in the // clipboard as well as on screen (2^-70 printed as 0.00000...). - // Scientific notation is the only honest rendering, and it has to come - // from the rational rather than from this text, which has no digits - // left in it. if (!isZeroText(rendered.text)) { // Group the integer part for readability; the raw form stays plain. const display = try groupDecimalText(allocator, rendered.text); @@ -188,75 +191,34 @@ pub const max_display_integer_digits: usize = 40; /// Significant digits kept when abbreviating to scientific notation. const scientific_significant_digits: usize = 17; -/// Count digits before the decimal point, ignoring sign. -fn integerDigitCount(text: []const u8) usize { - var start: usize = 0; - if (text.len > 0 and (text[0] == '-' or text[0] == '+')) start = 1; +/// The three parts of decimal text: an optional sign, the integer digits, and +/// everything from the decimal point onward. +/// +/// One place that knows how to take decimal text apart. `integerDigitCount` and +/// `writeGroupedDecimal` each used to work it out themselves, which is two chances +/// to disagree about where the sign ends. +const DecimalParts = struct { + /// Length of the sign, 0 or 1. + sign_len: usize, + /// Number of digits before the decimal point. + int_digits: usize, + /// The decimal point and fractional digits, empty for an integer. + tail: []const u8, +}; + +fn splitDecimalText(text: []const u8) DecimalParts { + const sign_len: usize = if (text.len > 0 and (text[0] == '-' or text[0] == '+')) 1 else 0; const dot = std.mem.indexOfScalar(u8, text, '.') orelse text.len; - return dot - start; + return .{ + .sign_len = sign_len, + .int_digits = dot - sign_len, + .tail = text[dot..], + }; } -/// Render decimal text in scientific notation, rounding the mantissa. -/// -/// Only called for values with more integer digits than the display cap, so the -/// exponent is always large and positive; no denormal or leading-zero handling -/// is needed. -fn scientificFromDecimalText(allocator: std.mem.Allocator, text: []const u8) ![]u8 { - var start: usize = 0; - var negative = false; - if (text.len > 0 and (text[0] == '-' or text[0] == '+')) { - negative = text[0] == '-'; - start = 1; - } - const dot = std.mem.indexOfScalar(u8, text, '.') orelse text.len; - const int_digits = text[start..dot]; - std.debug.assert(int_digits.len > scientific_significant_digits); - - const exponent = int_digits.len - 1; - - // Copy one extra digit so the mantissa can be rounded half-up. - var digits: [scientific_significant_digits + 1]u8 = undefined; - @memcpy(&digits, int_digits[0 .. scientific_significant_digits + 1]); - - var kept = digits[0..scientific_significant_digits]; - if (digits[scientific_significant_digits] >= '5') { - var i = kept.len; - var carried = true; - while (i > 0 and carried) { - i -= 1; - if (kept[i] == '9') { - kept[i] = '0'; - } else { - kept[i] += 1; - carried = false; - } - } - // Rounding 999... up to 1000... shifts the exponent, e.g. 9.99e9 -> 1e10. - if (carried) { - return std.fmt.allocPrint(allocator, "{s}1e{d}", .{ - if (negative) "-" else "", - exponent + 1, - }); - } - } - - // Trim trailing zeros from the fractional part of the mantissa. - var frac_end = kept.len; - while (frac_end > 1 and kept[frac_end - 1] == '0') frac_end -= 1; - - if (frac_end == 1) { - return std.fmt.allocPrint(allocator, "{s}{c}e{d}", .{ - if (negative) "-" else "", - kept[0], - exponent, - }); - } - return std.fmt.allocPrint(allocator, "{s}{c}.{s}e{d}", .{ - if (negative) "-" else "", - kept[0], - kept[1..frac_end], - exponent, - }); +/// Count digits before the decimal point, ignoring sign. +fn integerDigitCount(text: []const u8) usize { + return splitDecimalText(text).int_digits; } pub const NumberDisplay = struct { @@ -284,6 +246,43 @@ fn groupDecimalText(allocator: std.mem.Allocator, text: []const u8) ![]u8 { return out; } +/// Format a value as an amount: grouped integer part, exactly `decimals` places. +/// +/// The single implementation of this. It existed three times before: character for +/// character in `src/main.zig` and `src/tui/financial.zig`, both of which also +/// reimplemented the comma grouping that lives a few lines below here. +/// +/// Returns null rather than a placeholder when the result does not fit `buf`. The +/// copies returned the string "?", so `tally amort 1e40 0.5 3` printed a full table +/// of question marks and exited 0. A caller that cannot format a number should say +/// so, not render one. +pub fn formatAmount(buf: []u8, value: f64, decimals: u8) ?[]const u8 { + if (!std.math.isFinite(value)) return null; + + // Enough for f64's widest fixed-point rendering (about 310 integer digits) + // plus separators and a fractional part. + var plain: [400]u8 = undefined; + const text = switch (decimals) { + 0 => std.fmt.bufPrint(&plain, "{d:.0}", .{value}), + 1 => std.fmt.bufPrint(&plain, "{d:.1}", .{value}), + 2 => std.fmt.bufPrint(&plain, "{d:.2}", .{value}), + else => std.fmt.bufPrint(&plain, "{d:.6}", .{value}), + } catch return null; + + const needed = groupedDecimalLen(text); + if (needed > buf.len) return null; + if (needed == text.len) { + @memcpy(buf[0..text.len], text); + return buf[0..text.len]; + } + return buf[0..writeGroupedDecimal(buf, text)]; +} + +/// `formatAmount` at two decimal places, the money case. +pub fn formatMoney(buf: []u8, value: f64) ?[]const u8 { + return formatAmount(buf, value, 2); +} + /// Bytes `writeGroupedDecimal` will produce for `text`. Equal to `text.len` when /// there is nothing to group, which callers use to skip the copy entirely. /// @@ -302,10 +301,9 @@ pub fn groupedDecimalLen(text: []const u8) usize { /// Copy `text` into `dest` with commas grouping the integer part. `dest` must be /// at least `groupedDecimalLen(text)` bytes and must not overlap `text`. pub fn writeGroupedDecimal(dest: []u8, text: []const u8) usize { - var start: usize = 0; - if (text.len > 0 and (text[0] == '-' or text[0] == '+')) start = 1; - const dot = std.mem.indexOfScalar(u8, text, '.') orelse text.len; - const int_digits = dot - start; + const parts = splitDecimalText(text); + const start = parts.sign_len; + const int_digits = parts.int_digits; @memcpy(dest[0..start], text[0..start]); var w: usize = start; @@ -319,9 +317,8 @@ pub fn writeGroupedDecimal(dest: []u8, text: []const u8) usize { dest[w] = text[start + i]; w += 1; } - const tail = text[dot..]; - @memcpy(dest[w..][0..tail.len], tail); - return w + tail.len; + @memcpy(dest[w..][0..parts.tail.len], parts.tail); + return w + parts.tail.len; } /// Format an integer for programmer mode hex display. @@ -569,33 +566,15 @@ fn absoluteValue(value: i128) u128 { return if (value < 0) ~bits +% 1 else bits; } +/// Write `value` with comma grouping, reusing the text grouper. +/// +/// This used to be a second grouping implementation: it built the digits in reverse +/// and inserted separators itself, so the codebase had two places that knew what a +/// thousands group is. Writing the plain digits and then grouping them keeps one. fn writeUnsignedWithCommas(buf: []u8, value: u128) usize { - if (value == 0) { - buf[0] = '0'; - return 1; - } - var digits: [39]u8 = undefined; - var count: usize = 0; - var v = value; - while (v > 0) : (v /= 10) { - digits[count] = @intCast(v % 10); - count += 1; - } - // digits[0] is least significant, digits[count-1] is most significant - // Write most significant first, inserting commas every 3 from the right - var pos: usize = 0; - var i: usize = count; - while (i > 0) { - i -= 1; - buf[pos] = '0' + digits[i]; - pos += 1; - // Insert comma if there are more digits and position from right is multiple of 3 - if (i > 0 and i % 3 == 0) { - buf[pos] = ','; - pos += 1; - } - } - return pos; + var plain: [40]u8 = undefined; + const digits = plain[0..writeUnsignedInt(&plain, value)]; + return writeGroupedDecimal(buf, digits); } fn writeDecimalWithCommas(buf: []u8, value: i128) usize { @@ -1106,18 +1085,26 @@ test "formatNumber: 9007199254740993 is above NFR-7's f64 bound but must print i try testing.expectEqualStrings("9007199254740993", shown.raw); } -test "scientificFromDecimalText: mantissa trimming and exponents" { +test "abbreviated huge values go through the same renderer as tiny ones" { + // This case used to have its own text-based renderer. Both ends of the range + // now use Rational.toScientificString, so this checks the shared path from the + // formatter's side: mantissa trimming, sign, and the exponent. const alloc = testing.allocator; const cases = [_][2][]const u8{ - // 41 digits so the cap is exceeded in every case. + // 41 digits, one past max_display_integer_digits. .{ "10000000000000000000000000000000000000000", "1e40" }, .{ "12000000000000000000000000000000000000000", "1.2e40" }, .{ "-25000000000000000000000000000000000000000", "-2.5e40" }, }; for (cases) |c| { - const got = try scientificFromDecimalText(alloc, c[0]); - defer alloc.free(got); - try testing.expectEqualStrings(c[1], got); + var value = try Number.parse(alloc, c[0]); + defer value.deinit(); + const shown = try formatNumber(alloc, value); + defer shown.deinit(alloc); + try testing.expectEqualStrings(c[1], shown.display); + // The clipboard form still carries every digit. + try testing.expectEqualStrings(c[0], shown.raw); + try testing.expect(!shown.exact); } } @@ -1351,3 +1338,154 @@ test "formatDecimalSigned: the rest of the signed range still formats" { formatDecimalSigned(&buf, std.math.maxInt(i128)).display, ); } + +// -- One amount formatter -- +// +// This logic existed three times: character for character in src/main.zig and +// src/tui/financial.zig, each reimplementing the comma grouping that already lived +// in this file. Both copies also returned the string "?" when the buffer was too +// small, so `tally amort 1e40 0.5 3` printed a table of question marks and exited 0. + +test "formatMoney: grouping, sign and two decimals" { + var buf: [64]u8 = undefined; + try testing.expectEqualStrings("0.00", formatMoney(&buf, 0).?); + try testing.expectEqualStrings("199.10", formatMoney(&buf, 199.1).?); + try testing.expectEqualStrings("1,199.10", formatMoney(&buf, 1199.1).?); + try testing.expectEqualStrings("200,000.00", formatMoney(&buf, 200000).?); + try testing.expectEqualStrings("231,677.04", formatMoney(&buf, 231677.04).?); + try testing.expectEqualStrings("1,234,567.89", formatMoney(&buf, 1234567.89).?); + try testing.expectEqualStrings("-1,199.10", formatMoney(&buf, -1199.1).?); + try testing.expectEqualStrings("-0.01", formatMoney(&buf, -0.01).?); +} + +test "formatMoney: rounds to the cent" { + var buf: [64]u8 = undefined; + try testing.expectEqualStrings("1,199.10", formatMoney(&buf, 1199.101050305518).?); + try testing.expectEqualStrings("2.00", formatMoney(&buf, 1.995).?); +} + +test "formatMoney: reports failure instead of a placeholder" { + var tiny: [4]u8 = undefined; + try testing.expect(formatMoney(&tiny, 1234567.89) == null); + // Non-finite values have no amount rendering at all. + var buf: [64]u8 = undefined; + try testing.expect(formatMoney(&buf, std.math.inf(f64)) == null); + try testing.expect(formatMoney(&buf, std.math.nan(f64)) == null); +} + +test "formatMoney: very large amounts render or fail cleanly, never partially" { + var buf: [64]u8 = undefined; + // 1e40 needs 41 integer digits, 13 separators and cents: 57 bytes, so it fits. + const forty = formatMoney(&buf, 1e40).?; + try testing.expectEqual(@as(usize, 57), forty.len); + try testing.expect(std.mem.startsWith(u8, forty, "10,000,000,000")); + try testing.expect(std.mem.endsWith(u8, forty, ".00")); + + // 1e300 needs 404 bytes, so the same buffer must refuse rather than truncate. + try testing.expect(formatMoney(&buf, 1e300) == null); + var wide: [512]u8 = undefined; + const huge = formatMoney(&wide, 1e300).?; + try testing.expect(std.mem.endsWith(u8, huge, ".00")); +} + +test "formatAmount: other decimal counts" { + var buf: [64]u8 = undefined; + try testing.expectEqualStrings("1,000", formatAmount(&buf, 1000.4, 0).?); + try testing.expectEqualStrings("1,000.4", formatAmount(&buf, 1000.44, 1).?); + try testing.expectEqualStrings("1,000.44", formatAmount(&buf, 1000.44, 2).?); +} + +test "formatMoney: agrees with the grouping used for ordinary results" { + // The whole point of collapsing these: an amount and a plain result group the + // same way. + var money_buf: [64]u8 = undefined; + var value_buf: [256]u8 = undefined; + const as_money = formatMoney(&money_buf, 231677).?; + const as_value = formatFloat(&value_buf, 231677).display; + try testing.expectEqualStrings("231,677.00", as_money); + try testing.expectEqualStrings("231,677", as_value); + // Same separators, differing only in the fixed decimal places. + try testing.expect(std.mem.startsWith(u8, as_money, as_value)); +} + +// -- One grouping implementation -- +// +// Grouping existed twice: once over text (groupedDecimalLen/writeGroupedDecimal) and +// once over integers (writeUnsignedWithCommas built digits in reverse and inserted +// its own separators). The integer path now writes plain digits and groups them, so +// there is a single definition of what a thousands group is. + +test "integer and text grouping agree on every width" { + var integer_buf: [512]u8 = undefined; + var text_buf: [512]u8 = undefined; + var plain_buf: [64]u8 = undefined; + + const values = [_]u128{ + 0, 1, + 9, 10, + 99, 100, + 999, 1000, + 1001, 12345, + 999999, 1000000, + 123456789, std.math.maxInt(u64), + std.math.maxInt(u128), 4294967295, + 3735928559, 1180591620717411303424, + }; + for (values) |value| { + const grouped = integer_buf[0..writeUnsignedWithCommas(&integer_buf, value)]; + + // Independently: render the digits, then group the text. + const plain = plain_buf[0..writeUnsignedInt(&plain_buf, value)]; + const via_text = text_buf[0..writeGroupedDecimal(&text_buf, plain)]; + + try testing.expectEqualStrings(via_text, grouped); + } +} + +test "integer grouping: known shapes" { + var buf: [64]u8 = undefined; + try testing.expectEqualStrings("0", buf[0..writeUnsignedWithCommas(&buf, 0)]); + try testing.expectEqualStrings("100", buf[0..writeUnsignedWithCommas(&buf, 100)]); + try testing.expectEqualStrings("1,000", buf[0..writeUnsignedWithCommas(&buf, 1000)]); + try testing.expectEqualStrings("4,294,967,295", buf[0..writeUnsignedWithCommas(&buf, 4294967295)]); + try testing.expectEqualStrings( + "340,282,366,920,938,463,463,374,607,431,768,211,455", + buf[0..writeUnsignedWithCommas(&buf, std.math.maxInt(u128))], + ); +} + +test "signed integer grouping keeps the sign outside the groups" { + var buf: [128]u8 = undefined; + try testing.expectEqualStrings("-1,234", buf[0..writeDecimalWithCommas(&buf, -1234)]); + try testing.expectEqualStrings("-1", buf[0..writeDecimalWithCommas(&buf, -1)]); + try testing.expectEqualStrings("0", buf[0..writeDecimalWithCommas(&buf, 0)]); + try testing.expectEqualStrings( + "-170,141,183,460,469,231,731,687,303,715,884,105,728", + buf[0..writeDecimalWithCommas(&buf, std.math.minInt(i128))], + ); +} + +test "splitDecimalText: one place that takes decimal text apart" { + const unsigned = splitDecimalText("1234.56"); + try testing.expectEqual(@as(usize, 0), unsigned.sign_len); + try testing.expectEqual(@as(usize, 4), unsigned.int_digits); + try testing.expectEqualStrings(".56", unsigned.tail); + + const negative = splitDecimalText("-1234.56"); + try testing.expectEqual(@as(usize, 1), negative.sign_len); + try testing.expectEqual(@as(usize, 4), negative.int_digits); + try testing.expectEqualStrings(".56", negative.tail); + + const integer = splitDecimalText("-70"); + try testing.expectEqual(@as(usize, 1), integer.sign_len); + try testing.expectEqual(@as(usize, 2), integer.int_digits); + try testing.expectEqualStrings("", integer.tail); + + const explicit_plus = splitDecimalText("+5"); + try testing.expectEqual(@as(usize, 1), explicit_plus.sign_len); + try testing.expectEqual(@as(usize, 1), explicit_plus.int_digits); + + const empty = splitDecimalText(""); + try testing.expectEqual(@as(usize, 0), empty.sign_len); + try testing.expectEqual(@as(usize, 0), empty.int_digits); +} diff --git a/engine/src/number.zig b/engine/src/number.zig index ef17fb9..6f77722 100644 --- a/engine/src/number.zig +++ b/engine/src/number.zig @@ -21,6 +21,25 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const rational = @import("rational.zig"); const Rational = rational.Rational; +const CalcError = @import("types.zig").CalcError; + +/// Map a numeric-model error onto the engine's error set. +/// +/// Lives here so there is one mapping. The evaluator and the unit converter each +/// had their own copy, which is how they came to disagree: one turned +/// `ExponentTooLarge` into `Overflow` and neither knew what to do with a newly +/// added member until the compiler complained in two places. +pub fn toCalcError(err: Error) CalcError { + return switch (err) { + error.OutOfMemory => CalcError.OutOfMemory, + error.DivisionByZero => CalcError.DivisionByZero, + error.InvalidNumber => CalcError.InvalidNumber, + // An exponent too large to compute is an overflow from the caller's view. + error.ExponentTooLarge => CalcError.Overflow, + // The square root of a negative value is outside the domain. + error.NegativeRoot => CalcError.DomainError, + }; +} pub const Error = rational.Error; @@ -220,8 +239,15 @@ pub const Number = union(enum) { /// Square root. Exact for perfect rational squares (`sqrt(4)` is 2), inexact /// otherwise (`sqrt(2)`), per design.md 2.7.4. + /// + /// A negative input is `error.NegativeRoot`. The domain rule lives here rather + /// than in each caller: it used to be checked in three places (here, in + /// `Rational.sqrtExact`, and again in the evaluator, which returned + /// `UnknownFunction` for it so `sqrt(-1)` reported "unknown function"). The + /// float fallback would otherwise return a silent NaN. pub fn sqrt(allocator: Allocator, a: Number) Error!Number { - if (a == .exact and !a.exact.isNegative()) { + if (a.isNegative()) return Error.NegativeRoot; + if (a == .exact) { if (try Rational.sqrtExact(allocator, a.exact)) |root| { return capped(allocator, root); } @@ -229,13 +255,6 @@ pub const Number = union(enum) { return .{ .inexact = @sqrt(a.toFloat(allocator)) }; } - /// Apply a float-only function, always producing an inexact result. This is - /// the single entry point for transcendentals, so the fallback boundary is - /// visible in one place. - pub fn applyFloatFn(allocator: Allocator, a: Number, comptime f: fn (f64) f64) Number { - return .{ .inexact = f(a.toFloat(allocator)) }; - } - /// Shape of a unary operation that has an exact implementation. fn unary( allocator: Allocator, @@ -578,26 +597,49 @@ test "sqrt: perfect squares stay exact, others fall back" { try testing.expectApproxEqAbs(@as(f64, std.math.sqrt2), r2.toFloat(alloc), 1e-15); } -test "sqrt: negative input falls back to a float NaN" { +test "sqrt: a negative input is a domain error, not a silent NaN" { + // This used to return an inexact NaN, and the evaluator separately rejected + // negatives with UnknownFunction, so `sqrt(-1)` reported "unknown function". + // The rule now lives here and nowhere else. var neg = try Number.fromInt(alloc, -4); defer neg.deinit(); - var r = try Number.sqrt(alloc, neg); - defer r.deinit(); - try testing.expect(!r.isExact()); - try testing.expect(std.math.isNan(r.toFloat(alloc))); + try testing.expectError(Error.NegativeRoot, Number.sqrt(alloc, neg)); + + var inexact_neg = Number.fromFloat(-4.0); + defer inexact_neg.deinit(); + try testing.expectError(Error.NegativeRoot, Number.sqrt(alloc, inexact_neg)); + + // Zero and positives are unaffected. + var zero = try Number.fromInt(alloc, 0); + defer zero.deinit(); + var root_zero = try Number.sqrt(alloc, zero); + defer root_zero.deinit(); + try testing.expect(root_zero.isExact()); } -test "applyFloatFn always yields inexact" { - var one = try Number.fromInt(alloc, 1); - defer one.deinit(); - var r = Number.applyFloatFn(alloc, one, floatLog2); - defer r.deinit(); - try testing.expect(!r.isExact()); - try testing.expectEqual(@as(f64, 0.0), r.toFloat(alloc)); +test "sqrt: the domain error reaches the engine error set as a domain error" { + var neg = try Number.fromInt(alloc, -4); + defer neg.deinit(); + try testing.expectError(Error.NegativeRoot, Number.sqrt(alloc, neg)); } -fn floatLog2(x: f64) f64 { - return @log2(x); +test "toCalcError maps every numeric error, with no default" { + const CalcErr = @import("types.zig").CalcError; + // One mapping for the evaluator and the unit converter, which used to have a + // copy each. Walking the whole set keeps the two tiers of error vocabulary + // lined up: a member added to Error has to be given a CalcError here. + try testing.expectEqual(CalcErr.OutOfMemory, toCalcError(Error.OutOfMemory)); + try testing.expectEqual(CalcErr.DivisionByZero, toCalcError(Error.DivisionByZero)); + try testing.expectEqual(CalcErr.InvalidNumber, toCalcError(Error.InvalidNumber)); + try testing.expectEqual(CalcErr.Overflow, toCalcError(Error.ExponentTooLarge)); + try testing.expectEqual(CalcErr.DomainError, toCalcError(Error.NegativeRoot)); + + inline for (@typeInfo(Error).error_set.?) |field| { + // Every member is handled: this would not compile past an unhandled one, + // and every mapping lands in the engine's error set. + const mapped = toCalcError(@field(Error, field.name)); + try testing.expect(@TypeOf(mapped) == CalcErr); + } } test "asExactInt" { diff --git a/engine/src/parser.zig b/engine/src/parser.zig index d5c6b2f..df79ae4 100644 --- a/engine/src/parser.zig +++ b/engine/src/parser.zig @@ -21,10 +21,15 @@ const TokenKind = tokenizer_mod.TokenKind; const Token = tokenizer_mod.Token; const parseNumber = tokenizer_mod.parseNumber; const types = @import("types.zig"); -const Mode = types.Mode; const CalcError = types.CalcError; /// Precedence levels (higher = tighter binding). +/// +/// `assignment` and `call` are part of the table but are never returned by +/// `infixPrecedence`: assignment is recognized in prefix position (`X = expr`) and +/// a call is part of a primary, so neither goes through the precedence climb. They +/// stay here because the table is what documents the language's binding order, and +/// removing them would leave misleading gaps in the numbering. const Prec = enum(u8) { none = 0, assignment = 1, // = @@ -43,11 +48,7 @@ pub const Parser = struct { source: []const u8, tokenizer: Tokenizer, current: Token, - previous: Token, - mode: Mode, allocator: Allocator, - had_error: bool, - error_pos: ?usize, /// Nodes built so far, checked against `max_nodes`. node_count: usize, /// Current parseExpr/parsePrefix nesting, checked against `max_nest_depth`. @@ -67,18 +68,19 @@ pub const Parser = struct { /// Reached long before `max_nodes` by input like `((((...1...))))`. pub const max_nest_depth: usize = 128; - pub fn init(allocator: Allocator, source: []const u8, mode: Mode) Parser { - var tok = Tokenizer.init(source, mode); + /// The grammar does not depend on the mode: the same source parses to the same + /// tree in standard and programmer mode, and only evaluation differs. `init` + /// used to take a `Mode` and store it, along with `previous`, `had_error` and + /// `error_pos`; nothing ever read any of them. Errors are reported by returning + /// them, not by leaving a flag behind. + pub fn init(allocator: Allocator, source: []const u8) Parser { + var tok = Tokenizer.init(source); const first = tok.next(); return .{ .source = source, .tokenizer = tok, .current = first, - .previous = .{ .kind = .eof, .start = 0, .len = 0 }, - .mode = mode, .allocator = allocator, - .had_error = false, - .error_pos = null, .node_count = 0, .nest_depth = 0, }; @@ -96,8 +98,6 @@ pub const Parser = struct { if (self.current.kind != .eof) { // Trailing tokens: the tree parsed so far is unreachable. freeExpr(self.allocator, expr); - self.had_error = true; - self.error_pos = self.current.start; return CalcError.UnexpectedToken; } return expr; @@ -198,8 +198,6 @@ pub const Parser = struct { } if (self.current.kind != .right_paren) { - self.had_error = true; - self.error_pos = self.current.start; return CalcError.UnmatchedParen; } self.advance(); // consume ) @@ -224,8 +222,6 @@ pub const Parser = struct { const inner = try self.parseExpr(.none); if (self.current.kind != .right_paren) { freeExpr(self.allocator, inner); - self.had_error = true; - self.error_pos = self.current.start; return CalcError.UnmatchedParen; } self.advance(); // consume ) @@ -250,13 +246,9 @@ pub const Parser = struct { } }); }, .eof => { - self.had_error = true; - self.error_pos = tok.start; return CalcError.UnexpectedEnd; }, else => { - self.had_error = true; - self.error_pos = tok.start; return CalcError.UnexpectedToken; }, } @@ -285,8 +277,6 @@ pub const Parser = struct { self.advance(); const op = self.tokenToBinaryOp(tok.kind) orelse { - self.had_error = true; - self.error_pos = tok.start; return CalcError.UnexpectedToken; }; @@ -360,15 +350,12 @@ pub const Parser = struct { } fn advance(self: *Parser) void { - self.previous = self.current; self.current = self.tokenizer.next(); } fn makeNode(self: *Parser, expr: Expr) CalcError!*Expr { // Budget checked here so every construction site is covered by one test. if (self.node_count >= max_nodes) { - self.had_error = true; - self.error_pos = self.current.start; return CalcError.InvalidExpression; } self.node_count += 1; @@ -387,14 +374,14 @@ const testing = std.testing; // arena helper is kept for the tests already written against it. var test_arena_instance = std.heap.ArenaAllocator.init(std.heap.page_allocator); -fn testParse(source: []const u8, mode: Mode) !*Expr { - var parser = Parser.init(testing.allocator, source, mode); +fn testParse(source: []const u8) !*Expr { + var parser = Parser.init(testing.allocator, source); return parser.parse(); } -fn testParseArena(source: []const u8, mode: Mode) CalcError!*Expr { +fn testParseArena(source: []const u8) CalcError!*Expr { const alloc = test_arena_instance.allocator(); - var p = Parser.init(alloc, source, mode); + var p = Parser.init(alloc, source); return p.parse(); } @@ -423,20 +410,20 @@ pub fn freeExpr(allocator: Allocator, expr: *Expr) void { } test "parse simple number" { - const expr = try testParse("42", .standard); + const expr = try testParse("42"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(@as(f64, 42.0), expr.number.float_value); try testing.expectEqual(@as(?u64, 42), expr.number.int_value); } test "parse hex number" { - const expr = try testParse("0xFF", .programmer); + const expr = try testParse("0xFF"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(@as(?u64, 255), expr.number.int_value); } test "parse addition" { - const expr = try testParse("2 + 3", .standard); + const expr = try testParse("2 + 3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.add, expr.binary.op); try testing.expectEqual(@as(f64, 2.0), expr.binary.left.number.float_value); @@ -445,7 +432,7 @@ test "parse addition" { test "parse precedence: mul before add" { // 2 + 3 * 4 should parse as 2 + (3 * 4) - const expr = try testParse("2 + 3 * 4", .standard); + const expr = try testParse("2 + 3 * 4"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.add, expr.binary.op); try testing.expectEqual(@as(f64, 2.0), expr.binary.left.number.float_value); @@ -454,7 +441,7 @@ test "parse precedence: mul before add" { test "parse precedence: power right-associative" { // 2^3^4 should parse as 2^(3^4) - const expr = try testParse("2^3^4", .standard); + const expr = try testParse("2^3^4"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.pow, expr.binary.op); try testing.expectEqual(@as(f64, 2.0), expr.binary.left.number.float_value); @@ -462,7 +449,7 @@ test "parse precedence: power right-associative" { } test "parse unary negation" { - const expr = try testParse("-5", .standard); + const expr = try testParse("-5"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(UnaryOp.negate, expr.unary.op); try testing.expectEqual(@as(f64, 5.0), expr.unary.operand.number.float_value); @@ -470,7 +457,7 @@ test "parse unary negation" { test "parse negation in expression" { // -2 + 3 should be (-2) + 3 - const expr = try testParse("-2 + 3", .standard); + const expr = try testParse("-2 + 3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.add, expr.binary.op); try testing.expectEqual(UnaryOp.negate, expr.binary.left.unary.op); @@ -478,14 +465,14 @@ test "parse negation in expression" { test "parse parentheses" { // (2 + 3) * 4 - const expr = try testParse("(2 + 3) * 4", .standard); + const expr = try testParse("(2 + 3) * 4"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.mul, expr.binary.op); try testing.expectEqual(BinaryOp.add, expr.binary.left.binary.op); } test "parse function call" { - const expr = try testParse("sin(3.14)", .standard); + const expr = try testParse("sin(3.14)"); defer freeExpr(testing.allocator, expr); try testing.expectEqualStrings("sin", expr.call.name); try testing.expectEqual(@as(usize, 1), expr.call.args.len); @@ -493,20 +480,20 @@ test "parse function call" { } test "parse multi-arg function call" { - const expr = try testParse("max(1, 2, 3)", .standard); + const expr = try testParse("max(1, 2, 3)"); defer freeExpr(testing.allocator, expr); try testing.expectEqualStrings("max", expr.call.name); try testing.expectEqual(@as(usize, 3), expr.call.args.len); } test "parse variable" { - const expr = try testParse("pi", .standard); + const expr = try testParse("pi"); defer freeExpr(testing.allocator, expr); try testing.expectEqualStrings("pi", expr.variable); } test "parse assignment" { - const expr = try testParse("X = 42", .standard); + const expr = try testParse("X = 42"); defer freeExpr(testing.allocator, expr); try testing.expectEqualStrings("X", expr.assignment.name); try testing.expectEqual(@as(f64, 42.0), expr.assignment.value.number.float_value); @@ -514,66 +501,66 @@ test "parse assignment" { test "parse adjacent number and identifier is an error (no implicit mul)" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("2pi", .standard); + const result = testParseArena("2pi"); try testing.expectError(CalcError.UnexpectedToken, result); } test "parse adjacent number and paren is an error (no implicit mul)" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("3(4+5)", .standard); + const result = testParseArena("3(4+5)"); try testing.expectError(CalcError.UnexpectedToken, result); } test "parse adjacent paren paren is an error (no implicit mul)" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("(2)(3)", .standard); + const result = testParseArena("(2)(3)"); try testing.expectError(CalcError.UnexpectedToken, result); } test "parse caret is power in programmer mode (not XOR)" { - const expr = try testParse("0xF ^ 0x3", .programmer); + const expr = try testParse("0xF ^ 0x3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.pow, expr.binary.op); } test "parse xor keyword is XOR" { - const expr = try testParse("0xF xor 0x3", .programmer); + const expr = try testParse("0xF xor 0x3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.bit_xor, expr.binary.op); } test "parse and keyword" { - const expr = try testParse("0xF and 0x3", .programmer); + const expr = try testParse("0xF and 0x3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.bit_and, expr.binary.op); } test "parse or keyword" { - const expr = try testParse("0xF or 0x3", .programmer); + const expr = try testParse("0xF or 0x3"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.bit_or, expr.binary.op); } test "parse not prefix keyword" { - const expr = try testParse("not 0xFF", .programmer); + const expr = try testParse("not 0xFF"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(UnaryOp.bitwise_not, expr.unary.op); } test "parse caret as power in standard mode" { - const expr = try testParse("2 ^ 10", .standard); + const expr = try testParse("2 ^ 10"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.pow, expr.binary.op); } test "parse ** as power in programmer mode" { - const expr = try testParse("2 ** 10", .programmer); + const expr = try testParse("2 ** 10"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.pow, expr.binary.op); } test "parse bitwise operators" { - const expr = try testParse("0xF & 0x3 | 0x1", .programmer); + const expr = try testParse("0xF & 0x3 | 0x1"); defer freeExpr(testing.allocator, expr); // | has lowest precedence of these, so: (0xF & 0x3) | 0x1 try testing.expectEqual(BinaryOp.bit_or, expr.binary.op); @@ -581,46 +568,46 @@ test "parse bitwise operators" { } test "parse shift operators" { - const expr = try testParse("1 << 4", .programmer); + const expr = try testParse("1 << 4"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.shift_left, expr.binary.op); } test "parse bitwise not" { - const expr = try testParse("~0xFF", .programmer); + const expr = try testParse("~0xFF"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(UnaryOp.bitwise_not, expr.unary.op); } test "parse error: unmatched paren" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("(2 + 3", .standard); + const result = testParseArena("(2 + 3"); try testing.expectError(CalcError.UnmatchedParen, result); } test "parse error: unexpected token" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("+ +", .standard); + const result = testParseArena("+ +"); // + at start is not a valid prefix try testing.expectError(CalcError.UnexpectedToken, result); } test "parse error: empty expression" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("", .standard); + const result = testParseArena(""); try testing.expectError(CalcError.UnexpectedEnd, result); } test "parse complex expression" { // sin(2*pi) + 1 - const expr = try testParse("sin(2*pi) + 1", .standard); + const expr = try testParse("sin(2*pi) + 1"); defer freeExpr(testing.allocator, expr); try testing.expectEqual(BinaryOp.add, expr.binary.op); try testing.expectEqualStrings("sin", expr.binary.left.call.name); } test "parse nested function calls" { - const expr = try testParse("max(sin(1), cos(2))", .standard); + const expr = try testParse("max(sin(1), cos(2))"); defer freeExpr(testing.allocator, expr); try testing.expectEqualStrings("max", expr.call.name); try testing.expectEqual(@as(usize, 2), expr.call.args.len); @@ -630,7 +617,7 @@ test "parse nested function calls" { test "parse error: unmatched paren in function call args" { defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("max(1, 2", .standard); + const result = testParseArena("max(1, 2"); try testing.expectError(CalcError.UnmatchedParen, result); } @@ -639,7 +626,7 @@ test "parse error: identifier in infix position (not a keyword op)" { // a keyword operator, so it has .none precedence. The loop stops and // parse() reports the leftover token as unexpected. defer _ = test_arena_instance.reset(.retain_capacity); - const result = testParseArena("5 foo", .standard); + const result = testParseArena("5 foo"); try testing.expectError(CalcError.UnexpectedToken, result); } @@ -670,7 +657,7 @@ test "a failed parse leaves nothing allocated" { "-(1 + ", // nested failure under a unary }; for (bad) |source| { - var parser = Parser.init(testing.allocator, source, .standard); + var parser = Parser.init(testing.allocator, source); if (parser.parse()) |expr| { freeExpr(testing.allocator, expr); std.debug.print("expected a parse error for \"{s}\"\n", .{source}); @@ -682,7 +669,7 @@ test "a failed parse leaves nothing allocated" { test "a failed parse in programmer mode also leaves nothing allocated" { const bad = [_][]const u8{ "0xFF and", "1 rol", "not", "0b1010 xor (1", "1 << " }; for (bad) |source| { - var parser = Parser.init(testing.allocator, source, .programmer); + var parser = Parser.init(testing.allocator, source); if (parser.parse()) |expr| { freeExpr(testing.allocator, expr); std.debug.print("expected a parse error for \"{s}\"\n", .{source}); @@ -705,7 +692,7 @@ test "a successful parse hands over exactly one tree to free" { "tvm_pmt(360, 0.5, 200000, 0)", }; for (good) |source| { - var parser = Parser.init(testing.allocator, source, .standard); + var parser = Parser.init(testing.allocator, source); const expr = try parser.parse(); freeExpr(testing.allocator, expr); } @@ -732,7 +719,7 @@ test "an allocation failure mid-parse frees whatever was built" { while (fail_index < 64) : (fail_index += 1) { var failing = std.testing.FailingAllocator.init(testing.allocator, .{ .fail_index = fail_index }); const allocator = failing.allocator(); - var parser = Parser.init(allocator, source, .standard); + var parser = Parser.init(allocator, source); if (parser.parse()) |expr| { // Past the last allocation this input makes, so nothing is left // to fail; the tree itself must still be well formed. @@ -764,7 +751,7 @@ test "a tree larger than the node budget is rejected, not built" { try over.appendSlice(testing.allocator, "1"); } - var parser = Parser.init(testing.allocator, over.items, .standard); + var parser = Parser.init(testing.allocator, over.items); try testing.expectError(CalcError.InvalidExpression, parser.parse()); // Nothing is left allocated: testing.allocator would report a leak otherwise. } @@ -777,7 +764,7 @@ test "an expression within the node budget still parses" { try ok.appendSlice(testing.allocator, "1"); } - var parser = Parser.init(testing.allocator, ok.items, .standard); + var parser = Parser.init(testing.allocator, ok.items); const expr = try parser.parse(); defer freeExpr(testing.allocator, expr); try testing.expect(parser.node_count <= Parser.max_nodes); @@ -790,7 +777,7 @@ test "nesting deeper than the depth limit is rejected" { try deep.append(testing.allocator, '1'); for (0..Parser.max_nest_depth + 10) |_| try deep.append(testing.allocator, ')'); - var parser = Parser.init(testing.allocator, deep.items, .standard); + var parser = Parser.init(testing.allocator, deep.items); try testing.expectError(CalcError.InvalidExpression, parser.parse()); } @@ -801,7 +788,7 @@ test "nesting within the depth limit parses" { try deep.append(testing.allocator, '7'); for (0..64) |_| try deep.append(testing.allocator, ')'); - var parser = Parser.init(testing.allocator, deep.items, .standard); + var parser = Parser.init(testing.allocator, deep.items); const expr = try parser.parse(); defer freeExpr(testing.allocator, expr); try testing.expectEqual(@as(f64, 7.0), expr.number.float_value); @@ -813,7 +800,7 @@ test "unbalanced deep nesting is rejected without leaking the partial tree" { for (0..8000) |_| try deep.append(testing.allocator, '('); try deep.append(testing.allocator, '1'); - var parser = Parser.init(testing.allocator, deep.items, .standard); + var parser = Parser.init(testing.allocator, deep.items); try testing.expect(if (parser.parse()) |_| false else |_| true); } @@ -824,7 +811,7 @@ test "the depth limit also covers nested calls and unary operators" { try deep.append(testing.allocator, '4'); for (0..Parser.max_nest_depth + 10) |_| try deep.append(testing.allocator, ')'); - var parser = Parser.init(testing.allocator, deep.items, .standard); + var parser = Parser.init(testing.allocator, deep.items); try testing.expectError(CalcError.InvalidExpression, parser.parse()); var unary = std.ArrayList(u8).empty; @@ -832,6 +819,6 @@ test "the depth limit also covers nested calls and unary operators" { for (0..Parser.max_nest_depth + 10) |_| try unary.append(testing.allocator, '-'); try unary.append(testing.allocator, '1'); - var unary_parser = Parser.init(testing.allocator, unary.items, .standard); + var unary_parser = Parser.init(testing.allocator, unary.items); try testing.expectError(CalcError.InvalidExpression, unary_parser.parse()); } diff --git a/engine/src/programmer.zig b/engine/src/programmer.zig index 24d01c0..366ffb9 100644 --- a/engine/src/programmer.zig +++ b/engine/src/programmer.zig @@ -168,7 +168,7 @@ fn evalBinaryOp(config: ProgrammerConfig, op: BinaryOp, left: u128, right: u128) /// High-level: parse and evaluate a string in programmer mode. pub fn evalProgrammerString(allocator: Allocator, source: []const u8, config: ProgrammerConfig) CalcError!Integer { - var p = Parser.init(allocator, source, .programmer); + var p = Parser.init(allocator, source); const expr = try p.parse(); // Same ownership rule as evalStringInfo: the tree is ours to release, and the // returned Integer does not borrow from it. diff --git a/engine/src/rational.zig b/engine/src/rational.zig index 8fcb375..7fe174e 100644 --- a/engine/src/rational.zig +++ b/engine/src/rational.zig @@ -27,6 +27,9 @@ pub const Error = error{ InvalidNumber, /// The exponent of an integer power did not fit the supported range. ExponentTooLarge, + /// Square root of a negative value. Raised by the numeric model rather than + /// checked by each caller, so the domain rule lives in one place. + NegativeRoot, }; pub const Rational = struct { diff --git a/engine/src/tokenizer.zig b/engine/src/tokenizer.zig index adc12ac..a0fcabe 100644 --- a/engine/src/tokenizer.zig +++ b/engine/src/tokenizer.zig @@ -7,7 +7,6 @@ const std = @import("std"); const types = @import("types.zig"); -const Mode = types.Mode; const Base = types.Base; pub const TokenKind = enum { @@ -135,16 +134,18 @@ pub fn parseNumber(token_text: []const u8) !NumberValue { // -- Raw Tokenizer -- +/// Tokenizing is mode-independent: `0xFF`, `<<` and `'A'` are recognized in both +/// standard and programmer mode, and what differs is how the evaluator treats the +/// result (FR-2.12). The tokenizer used to take a `Mode` and store it, which +/// suggested otherwise, and no code ever read it. pub const Tokenizer = struct { source: []const u8, pos: usize, - mode: Mode, - pub fn init(source: []const u8, mode: Mode) Tokenizer { + pub fn init(source: []const u8) Tokenizer { return .{ .source = source, .pos = 0, - .mode = mode, }; } @@ -433,7 +434,7 @@ pub const Tokenizer = struct { const testing = std.testing; test "tokenize simple arithmetic" { - var tok = Tokenizer.init("2 + 3 * 4", .standard); + var tok = Tokenizer.init("2 + 3 * 4"); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.plus, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); @@ -443,28 +444,28 @@ test "tokenize simple arithmetic" { } test "tokenize hex number" { - var tok = Tokenizer.init("0xFF", .programmer); + var tok = Tokenizer.init("0xFF"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0xFF", t.text("0xFF")); } test "tokenize binary number" { - var tok = Tokenizer.init("0b1010", .programmer); + var tok = Tokenizer.init("0b1010"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0b1010", t.text("0b1010")); } test "tokenize octal number" { - var tok = Tokenizer.init("0o777", .programmer); + var tok = Tokenizer.init("0o777"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0o777", t.text("0o777")); } test "tokenize shift operators" { - var tok = Tokenizer.init("x << 3 >> 1 >>> 2", .programmer); + var tok = Tokenizer.init("x << 3 >> 1 >>> 2"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); try testing.expectEqual(TokenKind.shift_left, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); @@ -476,28 +477,28 @@ test "tokenize shift operators" { } test "tokenize star_star" { - var tok = Tokenizer.init("2**10", .programmer); + var tok = Tokenizer.init("2**10"); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.star_star, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); } test "tokenize number with underscores" { - var tok = Tokenizer.init("1_000_000", .standard); + var tok = Tokenizer.init("1_000_000"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("1_000_000", t.text("1_000_000")); } test "tokenize hex with underscores" { - var tok = Tokenizer.init("0xFF_FF", .programmer); + var tok = Tokenizer.init("0xFF_FF"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0xFF_FF", t.text("0xFF_FF")); } test "tokenize number with commas" { - var tok = Tokenizer.init("1,000,000", .standard); + var tok = Tokenizer.init("1,000,000"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("1,000,000", t.text("1,000,000")); @@ -505,7 +506,7 @@ test "tokenize number with commas" { } test "tokenize hex with spaces" { - var tok = Tokenizer.init("0xFF FF FF FF", .programmer); + var tok = Tokenizer.init("0xFF FF FF FF"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0xFF FF FF FF", t.text("0xFF FF FF FF")); @@ -513,7 +514,7 @@ test "tokenize hex with spaces" { } test "tokenize binary with spaces" { - var tok = Tokenizer.init("0b1111 0000", .programmer); + var tok = Tokenizer.init("0b1111 0000"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0b1111 0000", t.text("0b1111 0000")); @@ -521,7 +522,7 @@ test "tokenize binary with spaces" { } test "tokenize octal with spaces" { - var tok = Tokenizer.init("0o777 111", .programmer); + var tok = Tokenizer.init("0o777 111"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0o777 111", t.text("0o777 111")); @@ -529,7 +530,7 @@ test "tokenize octal with spaces" { } test "tokenize base literal space before operator stops" { - var tok = Tokenizer.init("0b1010 + 1", .programmer); + var tok = Tokenizer.init("0b1010 + 1"); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.plus, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); @@ -537,7 +538,7 @@ test "tokenize base literal space before operator stops" { } test "tokenize comma not eaten in function args" { - var tok = Tokenizer.init("max(1, 2)", .standard); + var tok = Tokenizer.init("max(1, 2)"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); // max try testing.expectEqual(TokenKind.left_paren, tok.next().kind); // ( try testing.expectEqual(TokenKind.number, tok.next().kind); // 1 @@ -547,7 +548,7 @@ test "tokenize comma not eaten in function args" { } test "tokenize function call" { - var tok = Tokenizer.init("sin(3.14)", .standard); + var tok = Tokenizer.init("sin(3.14)"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); try testing.expectEqual(TokenKind.left_paren, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); @@ -555,35 +556,35 @@ test "tokenize function call" { } test "tokenize floating point with exponent" { - var tok = Tokenizer.init("1.5e10", .standard); + var tok = Tokenizer.init("1.5e10"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("1.5e10", t.text("1.5e10")); } test "tokenize negative exponent" { - var tok = Tokenizer.init("2.5e-3", .standard); + var tok = Tokenizer.init("2.5e-3"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("2.5e-3", t.text("2.5e-3")); } test "tokenize number starting with dot" { - var tok = Tokenizer.init(".5", .standard); + var tok = Tokenizer.init(".5"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings(".5", t.text(".5")); } test "tokenize assignment" { - var tok = Tokenizer.init("X = 42", .standard); + var tok = Tokenizer.init("X = 42"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); try testing.expectEqual(TokenKind.equals, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); } test "tokenize all bitwise ops" { - var tok = Tokenizer.init("a & b | c ^ ~d", .programmer); + var tok = Tokenizer.init("a & b | c ^ ~d"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); try testing.expectEqual(TokenKind.ampersand, tok.next().kind); try testing.expectEqual(TokenKind.identifier, tok.next().kind); @@ -596,38 +597,38 @@ test "tokenize all bitwise ops" { } test "tokenize empty string" { - var tok = Tokenizer.init("", .standard); + var tok = Tokenizer.init(""); try testing.expectEqual(TokenKind.eof, tok.next().kind); } test "tokenize whitespace only" { - var tok = Tokenizer.init(" \t\n ", .standard); + var tok = Tokenizer.init(" \t\n "); try testing.expectEqual(TokenKind.eof, tok.next().kind); } test "tokenize semicolon" { - var tok = Tokenizer.init(";", .standard); + var tok = Tokenizer.init(";"); try testing.expectEqual(TokenKind.semicolon, tok.next().kind); } test "tokenize bare less-than is invalid" { - var tok = Tokenizer.init("<", .programmer); + var tok = Tokenizer.init("<"); try testing.expectEqual(TokenKind.invalid, tok.next().kind); } test "tokenize bare greater-than is invalid" { - var tok = Tokenizer.init(">", .programmer); + var tok = Tokenizer.init(">"); try testing.expectEqual(TokenKind.invalid, tok.next().kind); } test "tokenize lone dot is invalid" { - var tok = Tokenizer.init(".x", .standard); + var tok = Tokenizer.init(".x"); try testing.expectEqual(TokenKind.invalid, tok.next().kind); } test "tokenize unrecognized character is invalid" { // '@' is not handled by any dispatch case, so it hits the else branch - var tok = Tokenizer.init("@", .standard); + var tok = Tokenizer.init("@"); const t = tok.next(); try testing.expectEqual(TokenKind.invalid, t.kind); try testing.expectEqual(@as(usize, 1), t.len); @@ -637,7 +638,7 @@ test "tokenize base literal does not take a comma as a separator" { // Base literals group with spaces and underscores (FR-1.8); commas are the // decimal grouping character. Accepting them here only reintroduced the // argument-separator ambiguity in another place. - var tok = Tokenizer.init("0xFF,FF", .programmer); + var tok = Tokenizer.init("0xFF,FF"); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings("0xFF", t.text("0xFF,FF")); @@ -649,9 +650,9 @@ test "tokenize base literal does not take a comma as a separator" { } test "tokenize base literal still groups with spaces and underscores" { - var tok = Tokenizer.init("0xFF FF", .programmer); + var tok = Tokenizer.init("0xFF FF"); try testing.expectEqualStrings("0xFF FF", tok.next().text("0xFF FF")); - var underscored = Tokenizer.init("0xFF_FF", .programmer); + var underscored = Tokenizer.init("0xFF_FF"); try testing.expectEqualStrings("0xFF_FF", underscored.next().text("0xFF_FF")); } @@ -728,7 +729,7 @@ test "parseNumber with commas" { test "no implicit mul: spaces are just whitespace" { // Spaces between tokens don't create implicit multiplication - var tok = Tokenizer.init("2 3", .standard); + var tok = Tokenizer.init("2 3"); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.eof, tok.next().kind); @@ -745,7 +746,7 @@ test "no implicit mul: spaces are just whitespace" { test "comma groups digits only in threes" { // Grouped: consumed as one number. for ([_][]const u8{ "1,000", "1,234,567", "12,345", "123,456,789" }) |source| { - var tok = Tokenizer.init(source, .standard); + var tok = Tokenizer.init(source); const t = tok.next(); try testing.expectEqual(TokenKind.number, t.kind); try testing.expectEqualStrings(source, t.text(source)); @@ -758,7 +759,7 @@ test "comma with the wrong number of digits is a separate token" { // comma. This is what makes `log(100,10)` and `max(1,2)` parse as two // arguments. for ([_][]const u8{ "100,10", "1,2", "1,00", "1,0000" }) |source| { - var tok = Tokenizer.init(source, .standard); + var tok = Tokenizer.init(source); const first = tok.next(); try testing.expectEqual(TokenKind.number, first.kind); try testing.expectEqual(TokenKind.comma, tok.next().kind); @@ -768,14 +769,14 @@ test "comma with the wrong number of digits is a separate token" { } test "comma at the end of input is a separate token" { - var tok = Tokenizer.init("1,", .standard); + var tok = Tokenizer.init("1,"); try testing.expectEqual(TokenKind.number, tok.next().kind); try testing.expectEqual(TokenKind.comma, tok.next().kind); try testing.expectEqual(TokenKind.eof, tok.next().kind); } test "comma followed by a non-digit is a separate token" { - var tok = Tokenizer.init("max(1, 2)", .standard); + var tok = Tokenizer.init("max(1, 2)"); try testing.expectEqual(TokenKind.identifier, tok.next().kind); try testing.expectEqual(TokenKind.left_paren, tok.next().kind); try testing.expectEqual(TokenKind.number, tok.next().kind); @@ -787,7 +788,7 @@ test "comma followed by a non-digit is a separate token" { test "a grouped literal is still exact and keeps its full text" { // The exact tier re-parses the literal text, so the separators have to remain // in the token for it to see them. - var tok = Tokenizer.init("9,007,199,254,740,993", .standard); + var tok = Tokenizer.init("9,007,199,254,740,993"); const t = tok.next(); try testing.expectEqualStrings("9,007,199,254,740,993", t.text("9,007,199,254,740,993")); } diff --git a/engine/src/types.zig b/engine/src/types.zig index 7bf4c50..7643732 100644 --- a/engine/src/types.zig +++ b/engine/src/types.zig @@ -94,13 +94,6 @@ pub const Integer = struct { } }; -/// Result of any calculation. -pub const Value = union(enum) { - integer: Integer, - float: f64, - boolean: bool, -}; - /// Programmer mode configuration. pub const ProgrammerConfig = struct { bit_width: BitWidth = .bits64, @@ -146,13 +139,50 @@ pub const CalcError = error{ OutOfMemory, }; -/// Detailed error information with source position. -pub const ErrorInfo = struct { - code: CalcError, - message: []const u8, - /// Character position in input where the error occurred (0-indexed). - position: ?usize = null, -}; +/// The human-readable phrase for an error, with no prefix and no newline. +/// +/// The single source of these strings. The CLI and the TUI each had their own +/// switch over the same error set, differing only in punctuation and in what they +/// had forgotten: the TUI was missing `InsufficientParameters`, `ConvergenceFailure` +/// and `InvalidExpression` and rendered all three as "evaluation error". Callers add +/// their own decoration ("error: " and a newline for the CLI, "error: " for the +/// TUI), and a view with better context can still override individual cases, as the +/// financial form does. +pub fn errorPhrase(err: CalcError) []const u8 { + return switch (err) { + // Parser + CalcError.UnexpectedToken => "unexpected token", + CalcError.UnmatchedParen => "unmatched parenthesis", + CalcError.InvalidNumber => "invalid number", + CalcError.UnknownFunction => "unknown function", + CalcError.UnknownVariable => "unknown variable", + CalcError.UnexpectedEnd => "unexpected end of expression", + CalcError.InvalidExpression => "invalid expression", + + // Evaluation + CalcError.DivisionByZero => "division by zero", + CalcError.Overflow => "overflow", + CalcError.InvalidOperandType => "invalid operand type", + CalcError.DomainError => "domain error", + + // Struct layout + CalcError.InvalidType => "invalid type", + CalcError.InvalidFieldName => "invalid field name", + CalcError.DuplicateFieldName => "duplicate field name", + CalcError.StructTooLarge => "struct too large", + + // Financial + CalcError.InsufficientParameters => "these values do not determine an answer", + CalcError.ConvergenceFailure => "no solution found", + + // Units + CalcError.UnknownUnit => "unknown unit", + CalcError.IncompatibleUnits => "incompatible units (different categories)", + + // System + CalcError.OutOfMemory => "out of memory", + }; +} test "BitWidth.mask" { try std.testing.expectEqual(@as(u128, 0xFF), BitWidth.bits8.mask()); @@ -195,3 +225,50 @@ test "BitWidth.smallestFor" { try std.testing.expectEqual(BitWidth.bits64, BitWidth.smallestFor(0x1_0000_0000)); try std.testing.expectEqual(BitWidth.bits128, BitWidth.smallestFor(0x1_0000_0000_0000_0000)); } + +// -- One error phrase table -- +// +// The CLI and the TUI each had a full switch over this error set, and the TUI's had +// already fallen behind: InsufficientParameters, ConvergenceFailure and +// InvalidExpression all came out as "evaluation error". The phrases now live here +// once and each frontend adds its own decoration at comptime. + +test "errorPhrase: every error in the set has its own phrase" { + // Exhaustive by construction: the switch in errorPhrase has no else branch, so + // adding an error to CalcError without a phrase is a compile error rather than a + // silent fallback. This walks the set to prove the phrases are distinct and + // non-empty. + const fields = @typeInfo(CalcError).error_set.?; + var seen: [fields.len][]const u8 = undefined; + inline for (fields, 0..) |field, i| { + const phrase = errorPhrase(@field(CalcError, field.name)); + try std.testing.expect(phrase.len > 0); + // No prefix and no newline: decoration belongs to the caller. + try std.testing.expect(!std.mem.startsWith(u8, phrase, "error")); + try std.testing.expect(std.mem.indexOfScalar(u8, phrase, '\n') == null); + seen[i] = phrase; + } + + for (seen, 0..) |phrase, i| { + for (seen[i + 1 ..]) |other| { + if (std.mem.eql(u8, phrase, other)) { + std.debug.print("two errors share the phrase \"{s}\"\n", .{phrase}); + return error.TestUnexpectedResult; + } + } + } +} + +test "errorPhrase: usable at comptime, which is how frontends decorate it" { + const decorated = comptime "error: " ++ errorPhrase(CalcError.DivisionByZero); + try std.testing.expectEqualStrings("error: division by zero", decorated); +} + +test "errorPhrase: the cases the TUI table used to lose" { + try std.testing.expectEqualStrings("invalid expression", errorPhrase(CalcError.InvalidExpression)); + try std.testing.expectEqualStrings("no solution found", errorPhrase(CalcError.ConvergenceFailure)); + try std.testing.expectEqualStrings( + "these values do not determine an answer", + errorPhrase(CalcError.InsufficientParameters), + ); +} diff --git a/engine/src/units.zig b/engine/src/units.zig index cfc2caf..47473f6 100644 --- a/engine/src/units.zig +++ b/engine/src/units.zig @@ -667,14 +667,8 @@ fn convertExactInner( return Number.div(allocator, shifted, to_factor); } -fn mapNumberError(err: number_mod.Error) CalcError { - return switch (err) { - error.OutOfMemory => CalcError.OutOfMemory, - error.DivisionByZero => CalcError.DivisionByZero, - error.InvalidNumber => CalcError.InvalidNumber, - error.ExponentTooLarge => CalcError.Overflow, - }; -} +/// One mapping, in `number.zig`; this alias keeps the call sites short. +const mapNumberError = number_mod.toCalcError; // -- Tests -- diff --git a/src/main.zig b/src/main.zig index 476006b..db531cd 100644 --- a/src/main.zig +++ b/src/main.zig @@ -387,21 +387,25 @@ fn formatProgrammerResult(buf: []u8, result: engine.types.Integer, config: engin return .{ .output = output, .is_error = false }; } +/// Turn an engine error into a CLI line. +/// +/// The phrases live once, in `engine.types.errorPhrase`. This adds the prefix and +/// the newline at comptime, so the strings still have static lifetime and there is +/// no second copy of the wording to drift. The CLI and the TUI previously each kept +/// their own switch over the whole error set; the TUI's was already missing three +/// cases and rendered them as "evaluation error". fn errorMessage(err: engine.CalcError) []const u8 { + return decoratedError(err); +} + +/// Comptime-decorated form of every error phrase: "error: \n". +/// +/// `inline else` makes this exhaustive over the error set with no fallback branch: +/// a new `CalcError` member is a compile error in `errorPhrase`, not a string that +/// silently reads "evaluation error". +fn decoratedError(err: engine.CalcError) []const u8 { return switch (err) { - engine.CalcError.DivisionByZero => "error: division by zero\n", - engine.CalcError.UnknownFunction => "error: unknown function\n", - engine.CalcError.UnknownVariable => "error: unknown variable\n", - engine.CalcError.UnmatchedParen => "error: unmatched parenthesis\n", - engine.CalcError.UnexpectedToken => "error: unexpected token\n", - engine.CalcError.UnexpectedEnd => "error: unexpected end of expression\n", - engine.CalcError.InvalidNumber => "error: invalid number\n", - engine.CalcError.InvalidExpression => "error: invalid expression\n", - engine.CalcError.DomainError => "error: domain error\n", - engine.CalcError.Overflow => "error: overflow\n", - engine.CalcError.UnknownUnit => "error: unknown unit\n", - engine.CalcError.IncompatibleUnits => "error: incompatible units (different categories)\n", - else => "error: evaluation error\n", + inline else => |e| comptime "error: " ++ engine.types.errorPhrase(e) ++ "\n", }; } @@ -501,37 +505,6 @@ fn parseAmortArgs(args: []const []const u8) ParsedArgs { } /// Render an amount with thousands separators and two decimal places. -fn formatMoney(buf: []u8, value: f64) []const u8 { - var digits: [64]u8 = undefined; - const text = std.fmt.bufPrint(&digits, "{d:.2}", .{@abs(value)}) catch return "?"; - // "{d:.2}" always emits ".dd", so the integer part is everything before the - // last three characters. - if (text.len < 4) return "?"; - const whole = text[0 .. text.len - 3]; - const fraction = text[text.len - 3 ..]; - - var written: usize = 0; - if (value < 0) { - if (buf.len == 0) return "?"; - buf[0] = '-'; - written = 1; - } - for (whole, 0..) |digit, i| { - const remaining = whole.len - i; - if (i != 0 and remaining % 3 == 0) { - if (written == buf.len) return "?"; - buf[written] = ','; - written += 1; - } - if (written == buf.len) return "?"; - buf[written] = digit; - written += 1; - } - if (written + fraction.len > buf.len) return "?"; - @memcpy(buf[written..][0..fraction.len], fraction); - return buf[0 .. written + fraction.len]; -} - /// Render an amortization schedule as a table. The result is allocated because a /// 360-period schedule does not fit the fixed buffers the other outputs use. pub fn formatAmortization( @@ -549,18 +522,18 @@ pub fn formatAmortization( var out = std.ArrayList(u8).empty; errdefer out.deinit(allocator); - var line: [160]u8 = undefined; - var money: [48]u8 = undefined; + var line: [256]u8 = undefined; + var money: [64]u8 = undefined; const header = std.fmt.bufPrint(&line, "{s} at {d}% per period over {d} periods\n", .{ - formatMoney(&money, params.principal), + formatMoney(&money, params.principal) orelse return unformattableResult(), params.rate, params.periods, }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; out.appendSlice(allocator, header) catch return oomResult(); const payment_line = std.fmt.bufPrint(&line, "Payment {s} per period\n\n", .{ - formatMoney(&money, payment), + formatMoney(&money, payment) orelse return unformattableResult(), }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; out.appendSlice(allocator, payment_line) catch return oomResult(); @@ -570,16 +543,16 @@ pub fn formatAmortization( "Period Payment Interest Principal Balance\n", ) catch return oomResult(); for (rows) |row| { - var pay_buf: [48]u8 = undefined; - var int_buf: [48]u8 = undefined; - var prin_buf: [48]u8 = undefined; - var bal_buf: [48]u8 = undefined; + var pay_buf: [64]u8 = undefined; + var int_buf: [64]u8 = undefined; + var prin_buf: [64]u8 = undefined; + var bal_buf: [64]u8 = undefined; const row_text = std.fmt.bufPrint(&line, "{d: >6} {s: >12} {s: >12} {s: >12} {s: >12}\n", .{ row.period, - formatMoney(&pay_buf, row.payment), - formatMoney(&int_buf, row.interest), - formatMoney(&prin_buf, row.principal), - formatMoney(&bal_buf, row.balance), + formatMoney(&pay_buf, row.payment) orelse return unformattableResult(), + formatMoney(&int_buf, row.interest) orelse return unformattableResult(), + formatMoney(&prin_buf, row.principal) orelse return unformattableResult(), + formatMoney(&bal_buf, row.balance) orelse return unformattableResult(), }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; out.appendSlice(allocator, row_text) catch return oomResult(); } @@ -589,17 +562,17 @@ pub fn formatAmortization( const totals = engine.financial.amortizationTotals(params) catch |err| { return .{ .output = amortErrorMessage(err), .is_error = true }; }; - var paid_buf: [48]u8 = undefined; - var interest_buf: [48]u8 = undefined; - var principal_buf: [48]u8 = undefined; + var paid_buf: [64]u8 = undefined; + var interest_buf: [64]u8 = undefined; + var principal_buf: [64]u8 = undefined; const summary = std.fmt.bufPrint( &line, "Periods paid {d}\nTotal paid {s}\nTotal interest {s}\nPrincipal {s}", .{ totals.periods, - formatMoney(&paid_buf, totals.paid), - formatMoney(&interest_buf, totals.interest), - formatMoney(&principal_buf, totals.principal), + formatMoney(&paid_buf, totals.paid) orelse return unformattableResult(), + formatMoney(&interest_buf, totals.interest) orelse return unformattableResult(), + formatMoney(&principal_buf, totals.principal) orelse return unformattableResult(), }, ) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; out.appendSlice(allocator, summary) catch return oomResult(); @@ -608,6 +581,20 @@ pub fn formatAmortization( return .{ .output = text, .is_error = false }; } +/// The engine formatter, so the CLI, the TUI and the engine all group amounts the +/// same way. +const formatMoney = engine.formatter.formatMoney; + +/// An amount too large to render. Reported rather than printed as a placeholder: +/// the previous local formatter returned "?" for these, so a table of question +/// marks came out with exit status 0. +fn unformattableResult() CliResult { + return .{ + .output = "error: an amount in this schedule is too large to format\n", + .is_error = true, + }; +} + fn oomResult() CliResult { return .{ .output = "error: out of memory\n", .is_error = true }; } @@ -1236,19 +1223,48 @@ test "parseArgs: amort rejects incomplete or malformed terms" { try testing.expect(zero == .output and zero.output.is_error); } -test "formatMoney: grouping and sign" { +test "formatMoney: the CLI uses the engine formatter, not its own copy" { + // This lived in main.zig character for character alongside a second copy in + // src/tui/financial.zig. These cases now exercise engine.formatter.formatMoney. var buf: [48]u8 = undefined; - try testing.expectEqualStrings("0.00", formatMoney(&buf, 0)); - try testing.expectEqualStrings("199.10", formatMoney(&buf, 199.1)); - try testing.expectEqualStrings("1,199.10", formatMoney(&buf, 1199.1)); - try testing.expectEqualStrings("200,000.00", formatMoney(&buf, 200000)); - try testing.expectEqualStrings("1,234,567.89", formatMoney(&buf, 1234567.89)); - try testing.expectEqualStrings("-1,199.10", formatMoney(&buf, -1199.1)); + try testing.expectEqualStrings("0.00", formatMoney(&buf, 0).?); + try testing.expectEqualStrings("199.10", formatMoney(&buf, 199.1).?); + try testing.expectEqualStrings("1,199.10", formatMoney(&buf, 1199.1).?); + try testing.expectEqualStrings("200,000.00", formatMoney(&buf, 200000).?); + try testing.expectEqualStrings("1,234,567.89", formatMoney(&buf, 1234567.89).?); + try testing.expectEqualStrings("-1,199.10", formatMoney(&buf, -1199.1).?); } -test "formatMoney: a buffer too small reports rather than truncating silently" { +test "formatMoney: an amount that does not fit is reported, not rendered" { var tiny: [4]u8 = undefined; - try testing.expectEqualStrings("?", formatMoney(&tiny, 1234567.89)); + try testing.expect(formatMoney(&tiny, 1234567.89) == null); +} + +test "formatAmortization: an unrenderable amount is an error, not a table of marks" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + + // 1e40 used to produce a full report of "?" with exit status 0. It now fits, + // because the engine formatter groups the whole number. + const large = formatAmortization(arena.allocator(), .{ + .principal = 1e40, + .rate = 0.5, + .periods = 3, + }, false); + try testing.expect(!large.is_error); + try testing.expect(std.mem.indexOfScalar(u8, large.output, '?') == null); + try testing.expect(std.mem.indexOf(u8, large.output, "10,000,000,000,000,000,000,000,000,000,000,000,000,000.00") != null); + + // Past the width of the formatting buffer the report is refused outright, with + // a non-zero exit status, rather than printed with placeholders. + const huge = formatAmortization(arena.allocator(), .{ + .principal = 1e60, + .rate = 0.5, + .periods = 3, + }, false); + try testing.expect(huge.is_error); + try testing.expect(std.mem.indexOf(u8, huge.output, "too large to format") != null); + try testing.expect(std.mem.indexOfScalar(u8, huge.output, '?') == null); } test "formatAmortization: table has a row per period and correct first row" { diff --git a/src/tui.zig b/src/tui.zig index ec7d9c4..8b355b9 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -744,16 +744,11 @@ pub const App = struct { // Left/Right cycle the calculation, so every form is reachable without // the mouse (FR-7.7). if (key.matches(vaxis.Key.right, .{})) { - const forms = std.enums.values(financial_view.Form); - const next = (@intFromEnum(self.fin.form) + 1) % forms.len; - self.fin.setForm(@enumFromInt(next)); + self.fin.nextForm(); return; } if (key.matches(vaxis.Key.left, .{})) { - const forms = std.enums.values(financial_view.Form); - const current = @intFromEnum(self.fin.form); - const prev = if (current == 0) forms.len - 1 else current - 1; - self.fin.setForm(@enumFromInt(prev)); + self.fin.prevForm(); return; } if (key.matches(vaxis.Key.backspace, .{})) { @@ -1228,16 +1223,12 @@ pub const App = struct { draw.writeStr(&surface, 0, 1, "Tally", .{ .fg = C.cyan, .bg = C.bg, .bold = true }); // Mode tabs. Each is registered as a clickable region. - const tabs = [_]struct { mode: Mode, text: []const u8, color: vaxis.Cell.Color }{ - .{ .mode = .standard, .text = " Standard ", .color = C.green }, - .{ .mode = .programmer, .text = " Programmer ", .color = C.orange }, - .{ .mode = .financial, .text = " Financial ", .color = C.yellow }, - .{ .mode = .convert, .text = " Convert ", .color = C.purple }, - }; + // Mode tabs, drawn from the same table the Tab order comes from. Each is + // registered as a clickable region. var total_tab_width: u16 = 0; - for (tabs) |tab| total_tab_width += @intCast(tab.text.len); + for (mode_tabs) |tab| total_tab_width += @intCast(tab.text.len); var tab_col = width -| (total_tab_width + 2); - for (tabs) |tab| { + for (mode_tabs) |tab| { const len: u16 = @intCast(tab.text.len); const style: vaxis.Style = if (self.mode == tab.mode) .{ .fg = C.bg, .bg = tab.color, .bold = true } @@ -1302,22 +1293,42 @@ pub const App = struct { }; /// Mode order for Tab and Shift-Tab, matching the tab bar left to right. +/// The mode bar: order, label and colour, in one place. +/// +/// Tab order, Shift-Tab order and the drawn tab bar all come from this. They used +/// to be three separate encodings of the same sequence: the array below plus a +/// hand-written switch in each direction, which is three places to update and two +/// chances to disagree. +/// +/// The table is indexed by the `Mode` tag, which the comptime block below enforces, +/// so a mode added to the enum without a tab here is a compile error and looking up +/// a mode's position needs no search and no unreachable branch. +const mode_tabs = [_]struct { mode: Mode, text: []const u8, color: vaxis.Cell.Color }{ + .{ .mode = .standard, .text = " Standard ", .color = C.green }, + .{ .mode = .programmer, .text = " Programmer ", .color = C.orange }, + .{ .mode = .financial, .text = " Financial ", .color = C.yellow }, + .{ .mode = .convert, .text = " Convert ", .color = C.purple }, +}; + +comptime { + const modes = std.enums.values(Mode); + if (mode_tabs.len != modes.len) @compileError("every Mode needs a tab in mode_tabs"); + for (mode_tabs, 0..) |tab, i| { + if (@intFromEnum(tab.mode) != i) @compileError("mode_tabs must be in Mode declaration order"); + } +} + +/// Position of a mode in the bar. +fn modeIndex(mode: Mode) usize { + return @intFromEnum(mode); +} + pub fn nextMode(mode: Mode) Mode { - return switch (mode) { - .standard => .programmer, - .programmer => .financial, - .financial => .convert, - .convert => .standard, - }; + return mode_tabs[wrapIndex(modeIndex(mode), 1, mode_tabs.len)].mode; } pub fn prevMode(mode: Mode) Mode { - return switch (mode) { - .standard => .convert, - .programmer => .standard, - .financial => .programmer, - .convert => .financial, - }; + return mode_tabs[wrapIndex(modeIndex(mode), -1, mode_tabs.len)].mode; } /// Move an index by delta within [0, len), wrapping at both ends. @@ -1395,20 +1406,15 @@ pub fn drawHistory(items: []const App.HistoryEntry, surface: *vxfw.Surface, star } } +/// Turn an engine error into a status-line string. +/// +/// The phrases live once, in `engine.types.errorPhrase`; the prefix is added at +/// comptime. This switch used to be a second full copy of the CLI's, and had fallen +/// behind: `InsufficientParameters`, `ConvergenceFailure` and `InvalidExpression` +/// all came out as "evaluation error". fn errorStr(err: engine.CalcError) []const u8 { return switch (err) { - engine.CalcError.DivisionByZero => "error: division by zero", - engine.CalcError.UnknownFunction => "error: unknown function", - engine.CalcError.UnknownVariable => "error: unknown variable", - engine.CalcError.UnmatchedParen => "error: unmatched parenthesis", - engine.CalcError.UnexpectedToken => "error: unexpected token", - engine.CalcError.UnexpectedEnd => "error: unexpected end of expression", - engine.CalcError.InvalidNumber => "error: invalid number", - engine.CalcError.DomainError => "error: domain error", - engine.CalcError.Overflow => "error: overflow", - engine.CalcError.UnknownUnit => "error: unknown unit", - engine.CalcError.IncompatibleUnits => "error: incompatible units", - else => "error: evaluation error", + inline else => |e| comptime "error: " ++ engine.types.errorPhrase(e), }; } @@ -1629,6 +1635,12 @@ test "financial mode: left and right switch calculation without the mouse" { // Wraps backwards to the last calculation. try press(&app, &ctx, .{ .codepoint = vaxis.Key.left }); try testing.expectEqual(financial_view.Form.amortization, app.fin.form); + // And forwards off the end back to the first. This direction was never + // exercised, and it panicked with an integer overflow: the Form tag is a u2, + // so the handler's own `@intFromEnum(form) + 1` overflowed before the modulo + // could wrap it. + try press(&app, &ctx, .{ .codepoint = vaxis.Key.right }); + try testing.expectEqual(financial_view.Form.cagr, app.fin.form); } test "financial mode: the digits that edit a field do not switch modes" { @@ -1788,6 +1800,30 @@ test "nextMode and prevMode are inverses in both directions" { } } +test "nextMode visits every mode once before repeating" { + // Cycling through as many steps as there are tabs must return to the start + // having seen each mode, which is what makes every mode reachable by Tab. + var seen = [_]bool{false} ** mode_tabs.len; + var mode: Mode = .standard; + for (0..mode_tabs.len) |_| { + const i = modeIndex(mode); + try testing.expect(!seen[i]); + seen[i] = true; + mode = nextMode(mode); + } + try testing.expectEqual(Mode.standard, mode); + for (seen) |visited| try testing.expect(visited); +} + +test "the drawn tab order is the Tab key order" { + // The bar is drawn left to right from mode_tabs, so pressing Tab must move to + // the tab drawn to the right of the current one. + for (mode_tabs, 0..) |tab, i| { + const expected = mode_tabs[(i + 1) % mode_tabs.len].mode; + try testing.expectEqual(expected, nextMode(tab.mode)); + } +} + test "plain tab is not shift-tab" { var app = testApp(); defer app.deinit(); @@ -2113,7 +2149,7 @@ test "render: convert mode says when a conversion is affine" { var app = testApp(); defer app.deinit(); app.setMode(.convert); - var ctx = testCtxNoCmds(); + var ctx = testCtx(); try app.applyAction(&ctx, .{ .conv_category = .temperature }); const rows = try renderApp(arena, &app, 100, 34); @@ -2131,7 +2167,7 @@ test "render: convert mode is well formed for every category" { app.setMode(.convert); for (std.enums.values(engine.UnitCategory)) |category| { - var ctx = testCtxNoCmds(); + var ctx = testCtx(); try app.applyAction(&ctx, .{ .conv_category = category }); const rows = try renderApp(arena, &app, 100, 34); try testing.expect(test_render.furniture(rows).intact()); @@ -2330,12 +2366,6 @@ test "render: 128-bit programmer mode keeps its input line on a short terminal" try testing.expect(!test_render.contains(tall, "terminal too short")); } -/// An EventContext for calls that cannot issue a command, so nothing needs to be -/// freed afterwards. -fn testCtxNoCmds() vxfw.EventContext { - return testCtx(); -} - test "help overlay: arrows scroll, other keys dismiss" { var app = testApp(); defer app.deinit(); diff --git a/src/tui/financial.zig b/src/tui/financial.zig index 8fd17e5..d965c9a 100644 --- a/src/tui/financial.zig +++ b/src/tui/financial.zig @@ -279,6 +279,25 @@ pub const State = struct { self.scroll = 0; } + /// Move to the next form, wrapping. The order is the declaration order of + /// `Form`, which is also the order the tab strip is drawn in. + /// + /// This lives here rather than in the key handler so that form order is + /// defined once: the handler had its own copy of the modulo arithmetic in each + /// direction, and could not be tested without a terminal. That copy also + /// overflowed: the enum tag is a `u2`, so `@intFromEnum(form) + 1` on the last + /// form panicked in a debug build instead of wrapping. + pub fn nextForm(self: *State) void { + const current: usize = @intFromEnum(self.form); + self.setForm(@enumFromInt((current + 1) % form_count)); + } + + pub fn prevForm(self: *State) void { + const current: usize = @intFromEnum(self.form); + const prev = if (current == 0) form_count - 1 else current - 1; + self.setForm(@enumFromInt(prev)); + } + pub fn focusField(self: *State, index: usize) void { if (index < self.fieldCount()) self.field = index; } @@ -838,46 +857,28 @@ fn substitutedFormula(state: *const State, buf: []u8) ?[]const u8 { }; } -/// Format an amount with grouping and two decimals, matching the CLI table. +/// Format an amount with grouping and two decimals, via the engine formatter so +/// the CLI table and this view cannot diverge. +/// +/// Drawing cannot fail, so a value too large to render becomes "(too large)" +/// rather than being dropped. The CLI reports it as an error instead, because a +/// command can exit non-zero and a frame cannot. fn money(buf: []u8, value: f64) []const u8 { - var digits: [64]u8 = undefined; - const text = std.fmt.bufPrint(&digits, "{d:.2}", .{@abs(value)}) catch return "?"; - if (text.len < 4) return "?"; - const whole = text[0 .. text.len - 3]; - const fraction = text[text.len - 3 ..]; - - var written: usize = 0; - if (value < 0) { - if (buf.len == 0) return "?"; - buf[0] = '-'; - written = 1; - } - for (whole, 0..) |digit, i| { - const remaining = whole.len - i; - if (i != 0 and remaining % 3 == 0) { - if (written == buf.len) return "?"; - buf[written] = ','; - written += 1; - } - if (written == buf.len) return "?"; - buf[written] = digit; - written += 1; - } - if (written + fraction.len > buf.len) return "?"; - @memcpy(buf[written..][0..fraction.len], fraction); - return buf[0 .. written + fraction.len]; + return formatter.formatMoney(buf, value) orelse "(too large)"; } -/// Error text tuned to this view: the generic "domain error" is useless in a -/// form, where the cause is always one of a few bad entries. +/// Error text for this view. +/// +/// Only the cases where a form knows more than the engine does are overridden; the +/// rest defer to `engine.types.errorPhrase`, so this is no longer a third copy of +/// the whole table. A generic "domain error" is useless in a form, where the cause +/// is always one of a few bad entries, but "division by zero" needs no improving. pub fn errorText(err: engine.CalcError) []const u8 { return switch (err) { engine.CalcError.DomainError => "check the entries: values must be positive and a payment must cover the interest", engine.CalcError.ConvergenceFailure => "no rate solves these cash flows", engine.CalcError.InsufficientParameters => "these values do not determine an answer", - engine.CalcError.DivisionByZero => "division by zero", - engine.CalcError.Overflow => "overflow", - else => "cannot compute with these values", + else => engine.types.errorPhrase(err), }; } @@ -1322,9 +1323,9 @@ test "money: grouping and two decimals" { try testing.expectEqualStrings("0.00", money(&buf, 0)); } -test "money: reports rather than truncating when the buffer is too small" { +test "money: an amount too large for the slot says so rather than truncating" { var tiny: [3]u8 = undefined; - try testing.expectEqualStrings("?", money(&tiny, 1234567.89)); + try testing.expectEqualStrings("(too large)", money(&tiny, 1234567.89)); } test "formatValueLine: with and without an iteration count" { @@ -1724,3 +1725,40 @@ test "render: the payments toggle shows its state and highlights when focused" { const due = try renderFrame(testing.allocator, arena, state, 80, 24, true); try testing.expect(frameContains(due, "BGN (start of period)")); } + +test "State: form cycling is a cycle in both directions" { + // Left/Right cycling used to live in the key handler, where it could only be + // exercised through a key event. It belongs to the state. + var state: State = .{}; + for (0..form_count) |_| { + const before = state.form; + state.nextForm(); + try testing.expect(state.form != before); + state.prevForm(); + try testing.expectEqual(before, state.form); + state.nextForm(); + } + // form_count steps forward from the start returns to the start. + try testing.expectEqual(Form.cagr, state.form); +} + +test "State: form cycling visits every form and follows declaration order" { + var state: State = .{}; + var seen = [_]bool{false} ** form_count; + for (0..form_count) |i| { + seen[@intFromEnum(state.form)] = true; + const expected: Form = @enumFromInt((i + 1) % form_count); + state.nextForm(); + try testing.expectEqual(expected, state.form); + } + for (seen) |visited| try testing.expect(visited); +} + +test "State: changing form resets the focused field and the scroll" { + var state = stateWith(.tvm, &.{ "360", "0.5", "200000", "", "0" }); + state.focusField(4); + state.scroll = 3; + state.nextForm(); + try testing.expectEqual(@as(usize, 0), state.field); + try testing.expectEqual(@as(usize, 0), state.scroll); +} diff --git a/src/tui/programmer.zig b/src/tui/programmer.zig index cf5f6a7..470742e 100644 --- a/src/tui/programmer.zig +++ b/src/tui/programmer.zig @@ -202,6 +202,10 @@ fn registerConfigRegions(app: *tui.App, row: u16, col: u16, text: []const u8) vo /// When `toggles` is true a click flips the bit (used for BIN, where one digit /// is exactly one bit). Otherwise a click just moves the cursor to that digit, /// since "toggling" a multi-bit nibble or octal digit has no single meaning. +/// +/// `text` is a formatter `display` string, which is digits and spaces only. This +/// used to begin by skipping a `0x`/`0o`/`0b` prefix; only the `raw` strings carry +/// one, so that branch never ran. fn registerDigitRegions( app: *tui.App, row: u16, @@ -211,21 +215,15 @@ fn registerDigitRegions( field: tui.App.ProgField, toggles: bool, ) void { - // Skip a base prefix if one is present (display strings normally omit it). - var start: usize = 0; - if (text.len >= 2 and text[0] == '0' and (text[1] == 'o' or text[1] == 'x' or text[1] == 'b')) { - start = 2; - } - var displayed_digits: u16 = 0; - for (text[start..]) |ch| { + for (text) |ch| { if (ch != ' ') displayed_digits += 1; } if (displayed_digits == 0) return; - var text_col: u16 = col + @as(u16, @intCast(start)); + var text_col: u16 = col; var digit_idx: u16 = 0; - for (text[start..]) |ch| { + for (text) |ch| { if (ch != ' ') { // Digits are drawn MSB-first; convert to a bit offset from the LSB. const from_lsb: u16 = displayed_digits - 1 - digit_idx; @@ -246,16 +244,15 @@ fn registerDigitRegions( /// Draw a field's display string with a cursor highlighting the digit at bit_cursor position. /// `bits_per_digit` is 4 for hex, 3 for oct, 1 for bin. +/// +/// `text` is a formatter `display` string: digits and spaces, never a `0x`/`0o`/`0b` +/// prefix. This used to skip a prefix and draw it unhighlighted, which was dead +/// code in both this function and `registerDigitRegions`. fn drawFieldWithCursor(surface: *vxfw.Surface, row: u16, col: u16, text: []const u8, bit_cursor: u7, bits_per_digit: u8, total_bits: u8, color: vaxis.Cell.Color) void { _ = total_bits; - // Count actual displayed digits (non-space, non-prefix characters) - var start: usize = 0; - if (text.len >= 2 and text[0] == '0' and (text[1] == 'o' or text[1] == 'x' or text[1] == 'b')) { - start = 2; - } var displayed_digits: u16 = 0; - for (text[start..]) |ch| { + for (text) |ch| { if (ch != ' ') displayed_digits += 1; } @@ -267,16 +264,10 @@ fn drawFieldWithCursor(surface: *vxfw.Surface, row: u16, col: u16, text: []const else 0; - // Draw prefix - var text_col: u16 = col; - for (text[0..start]) |ch| { - draw.writeChar(surface, row, text_col, ch, .{ .fg = color }); - text_col += 1; - } - // Draw digits with cursor highlight + var text_col: u16 = col; var digit_idx: u16 = 0; - for (text[start..]) |ch| { + for (text) |ch| { if (ch == ' ') { draw.writeChar(surface, row, text_col, ' ', .{ .fg = color }); } else {