From 4eda2f62e80d6667662a1dac63f127d34be16218 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 28 Jul 2026 07:23:52 -0700 Subject: [PATCH] switch from f64 to Number union --- .kiro/specs/calculator/requirements.md | 6 +- .kiro/specs/calculator/tasks.md | 61 +++- engine/src/evaluator.zig | 289 +++++++++++++++-- engine/src/formatter.zig | 409 ++++++++++++++++++++++++- engine/src/number.zig | 8 + engine/src/rational.zig | 14 +- engine/src/tokenizer.zig | 7 +- src/main.zig | 29 +- src/tui.zig | 43 ++- 9 files changed, 796 insertions(+), 70 deletions(-) diff --git a/.kiro/specs/calculator/requirements.md b/.kiro/specs/calculator/requirements.md index 8df566d..bae04fa 100644 --- a/.kiro/specs/calculator/requirements.md +++ b/.kiro/specs/calculator/requirements.md @@ -207,7 +207,11 @@ still required (FR-7.7): the mouse never becomes the only way to do something. ### NFR-7: Number Display & Formatting - Decimal numbers must use comma grouping for display (e.g., `4,294,967,295`). -- Scientific notation only when absolute value > 10^15 / < 10^-15. Never jump to scientific notation for values that fit in a readable decimal. (An earlier draft also triggered scientific notation past 15 significant digits. That clause was never implemented and was wrong: read literally it renders `0.9999999999999998` as `9.999999999999998e-1`, which is worse. Superseded by NFR-9.) +- **Inexact (f64) values**: scientific notation only when absolute value > 10^15 / < 10^-15. Never jump to scientific notation for values that fit in a readable decimal. This bound is not a readability preference: 10^15 is where f64 stops distinguishing consecutive integers (2^53 ~ 9.007 x 10^15), so printing a plain integer past it would assert precision the value does not have. + - (An earlier draft also triggered scientific notation past 15 significant digits. That clause was never implemented and was wrong: read literally it renders `0.9999999999999998` as `9.999999999999998e-1`, which is worse.) +- **Exact values**: the 10^15 bound above must NOT apply. It exists because of f64's precision cliff, and an exact value has no such cliff, so applying it would contradict NFR-9.1 - `9007199254740993` is ~9.007 x 10^15 and would render as `9.007199254740993e15`, which is precisely the bug NFR-9.1 forbids. Exact values instead have a readability cap on integer digits (`formatter.max_display_integer_digits`), above which the display abbreviates to scientific notation while the clipboard/`raw` form retains every digit. + - The cap must exceed the values the exact tier exists to serve: `9007199254740993` (16 digits) and `2^128` (39 digits). It exists at all because without any cap, `factorial(171)` renders 310 digits and `1.5e300 * 10` renders 301: accurate but unreadable. + - **PROVISIONALLY 40 digits, pending review.** Chosen as the smallest round number above `2^128`. Not derived from any measured preference. - Programmer mode hex values display with space-separated bytes (e.g., `FF FF FF FF`). - Programmer mode binary values display grouped by nibble with spaces (e.g., `1111 1111`). - All frontends must distinguish between "display format" (with separators) and "clipboard format" (raw, no separators). diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index cefe496..0d20ff3 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -177,17 +177,58 @@ behavioral change (rationale in design.md 2.7.9): exist yet. `floor` and `factorial` were also missing a denominator `errdefer`. See design.md 2.7.10. Uncovered lines in the numeric modules went 25 -> 8. -#### 2.0c: `evalString` and the display path move to `Number`- Mechanical signature churn through evaluator/formatter/CLI/TUI tests. This is - the commit where `main.zig`'s output assertions get reviewed. -- Lift the limits this removes: `formatter.is_integer`'s `< 2^53` gate and - `evaluator.factorial`'s `x > 170` rejection (also the source of the misleading - "unknown function" error). -- Rewrite `tokenizer` test "parseNumber huge decimal falls back to float", whose - premise becomes false once big integers are exact. -- Verify: existing suite green after signature updates, no expected-value changes - beyond the three items above. +#### 2.0c: `evalString` and the display path move to `Number` [DONE] +- `Environment` now stores `Number` for variables and `Ans`, so an assignment + keeps its expression's exactness: `X = 0.1` stores exactly one tenth, and + `X + 0.2` is exactly `0.3`. +- `evalString` / `evalStringInfo` / `EvalInfo.value` return `Number`, allocated in + the caller's allocator. Intermediates stay in a scratch arena; only the final + value is copied out. +- Added `Rational.cloneWith` / `Number.cloneWith` to copy across allocators. This + is required, not convenience: a `Rational` carries its allocator inside its + limbs, so a value stored in the environment must be COPIED into an evaluation's + scratch arena, never shared, or one side frees memory the other still uses. +- `getVar` returns a borrowed value and documents that callers must `cloneWith`; + `setVar` and `setAns` copy, so storing an arena-allocated value is safe. +- FIXED A LATENT LIFETIME BUG: `setVar` now duplicates the variable NAME. It + points into the expression source, which the TUI frees on Ctrl-L, so the map + previously retained dangling keys. Covered by a test that frees the source + before reading the variable back. +- `formatter.formatNumber` renders exactly: exact integers print in full with + comma grouping at any magnitude, exact terminating fractions print exactly, + repeating expansions round to `exact_fraction_digits` (20) and are flagged + approximate, and inexact values use the float rules and are always flagged. +- Test churn absorbed in the helpers: `testEval` / `testEvalProgrammer` collapse + to f64 in one place, so all ~90 pre-existing f64 assertions stayed untouched. + Only 4 tests calling `evalString` / `evalStringInfo` directly needed a + `.toFloat(alloc)`. +- Rewrote the tokenizer's "huge decimal" test: its premise (that the value cannot + be exact) is false now that the evaluator re-parses literal text into a + rational. The u64 limit it pins belongs to the tokenizer layer only. +- Clarified rather than deleted `formatFloat`'s `2^53` gate: past that bound an + f64 no longer distinguishes consecutive integers, so printing one as an exact + integer would assert precision it lacks. Exact values bypass it entirely. + (`factorial`'s `x > 170` limit was already lifted in 2.0b.) +- Verified end-to-end: `9007199254740993`, `2^53 + 1`, `2^100`, `2^128`, + `1e20 + 1`, `factorial(25)`, `99999999999999999999999999 + 1` all exact. No + regressions on `2 + 3 * 4`, `10 / 4`, `0.1 + 0.2`, `sqrt(2)`, `pi`, `sin(0)`, + `1,000 * 2.3`, `10 % 3`, `1/0`, conversions, or the multi-base view. +- 564 tests pass (was 533). Coverage 99.56%. -#### 2.0d: Exactness tests only the `Number` API can express +REQUIREMENTS CONFLICT FOUND, RESOLVED IN NFR-7: the existing rule "scientific +notation when |value| > 10^15" directly contradicts NFR-9.1. `9007199254740993` +is ~9.007e15, so the old rule renders it as `9.007199254740993e15`, exactly the +bug NFR-9.1 forbids. That bound was never a readability rule; it is f64's +integer-precision cliff (2^53 ~ 9.007e15), which does not apply to exact values. +NFR-7 now splits the rule by tier: inexact values keep 10^15, exact values get a +readability cap on integer digits (`max_display_integer_digits`) above which the +DISPLAY abbreviates to scientific notation while the clipboard/`raw` form keeps +every digit. Without a cap, `factorial(171)` printed 310 digits and +`1.5e300 * 10` printed 301. +THE CAP IS PROVISIONALLY 40 (smallest round number above 2^128) AND NEEDS REVIEW: +it is not derived from any measured preference. + +#### 2.0d: Exactness tests only the `Number` API can express [DONE, folded into 2.0c] - `9007199254740993` round-trips (currently unguarded, and the one bug the f64 boundary cannot fix), `2^53 + 1`, `1/3` retained exactly, `(1/3) * 3` = 1, unbounded `factorial`, contagion, demotion at the denominator cap. diff --git a/engine/src/evaluator.zig b/engine/src/evaluator.zig index 33962f8..f2d22f4 100644 --- a/engine/src/evaluator.zig +++ b/engine/src/evaluator.zig @@ -21,12 +21,16 @@ const number_mod = @import("number.zig"); const Number = number_mod.Number; /// Evaluation environment holding variables, history, and config. +/// +/// Variables and `Ans` are stored as `Number`, so an assignment keeps whatever +/// exactness its expression had: `X = 0.1` stores exactly one tenth rather than +/// a binary approximation of it. pub const Environment = struct { allocator: Allocator, mode: Mode, programmer_config: ProgrammerConfig, - variables: std.StringHashMap(f64), - ans: f64, + variables: std.StringHashMap(Number), + ans: Number, history_len: usize, pub fn init(allocator: Allocator, mode: Mode) Environment { @@ -34,31 +38,77 @@ pub const Environment = struct { .allocator = allocator, .mode = mode, .programmer_config = .{}, - .variables = std.StringHashMap(f64).init(allocator), - .ans = 0, + .variables = std.StringHashMap(Number).init(allocator), + // Starts inexact so that `init` cannot fail; the first evaluation + // replaces it. + .ans = Number.fromFloat(0), .history_len = 0, }; } pub fn deinit(self: *Environment) void { + var it = self.variables.iterator(); + while (it.next()) |entry| { + self.allocator.free(entry.key_ptr.*); + entry.value_ptr.deinit(); + } self.variables.deinit(); + self.ans.deinit(); } - /// Set a variable value. - pub fn setVar(self: *Environment, name: []const u8, value: f64) !void { - try self.variables.put(name, value); + /// Store a variable. Both the name and the value are copied, so neither has + /// to outlive this call. + /// + /// The name is duplicated because it points into the expression source, + /// which callers are free to release: the TUI frees history entries on + /// Ctrl-L, which previously left dangling keys in this map. + pub fn setVar(self: *Environment, name: []const u8, value: Number) !void { + var copy = try value.cloneWith(self.allocator); + errdefer copy.deinit(); + + const gop = try self.variables.getOrPut(name); + if (gop.found_existing) { + gop.value_ptr.deinit(); + } else { + const owned_name = self.allocator.dupe(u8, name) catch |err| { + // Remove the entry keyed by the borrowed name so the map never + // retains a key it does not own. + _ = self.variables.remove(name); + return err; + }; + gop.key_ptr.* = owned_name; + } + gop.value_ptr.* = copy; } - /// Get a variable or constant value. - pub fn getVar(self: *const Environment, name: []const u8) ?f64 { - // Built-in constants - if (std.mem.eql(u8, name, "pi")) return math.pi; - if (std.mem.eql(u8, name, "e")) return math.e; - if (std.mem.eql(u8, name, "tau")) return math.tau; + /// Replace the last answer, taking a copy. + pub fn setAns(self: *Environment, value: Number) !void { + const copy = try value.cloneWith(self.allocator); + self.ans.deinit(); + self.ans = copy; + } + + /// Borrowed view of a variable or built-in constant. + /// + /// The result is owned by the environment (or is a freshly built constant), + /// so callers that need it to outlive the environment, or that will free it + /// separately, must `cloneWith` first. + /// + /// The constants are inexact by nature: pi, e and tau are irrational and + /// have no rational representation. + pub fn getVar(self: *const Environment, name: []const u8) ?Number { + if (std.mem.eql(u8, name, "pi")) return Number.fromFloat(math.pi); + if (std.mem.eql(u8, name, "e")) return Number.fromFloat(math.e); + if (std.mem.eql(u8, name, "tau")) return Number.fromFloat(math.tau); if (std.mem.eql(u8, name, "Ans") or std.mem.eql(u8, name, "ans")) return self.ans; return self.variables.get(name); } + + /// The last answer collapsed to f64, for frontends that only need a float. + pub fn ansFloat(self: *const Environment) f64 { + return self.ans.toFloat(self.allocator); + } }; /// Evaluate a parsed expression in the given environment. @@ -99,17 +149,16 @@ fn evalExact(env: *Environment, scratch: Allocator, expr: *const Expr) CalcError return Number.fromInt(scratch, packed_value) catch |err| return mapError(err); }, .variable => |name| { - // Variables and the built-in constants are stored as f64 today, so - // reading one yields an inexact value. For pi/e/tau that is correct - // (they are irrational); for user variables it is a temporary - // limitation that Task 2.0c removes by storing Number in the - // environment. + // getVar hands back a borrowed value owned by the environment, so + // copy it into the evaluation arena before it takes part in + // arithmetic that the arena will later free. const value = env.getVar(name) orelse return CalcError.UnknownVariable; - return Number.fromFloat(value); + return value.cloneWith(scratch) catch |err| mapError(err); }, .assignment => |a| { const val = try evalExact(env, scratch, a.value); - env.setVar(a.name, val.toFloat(scratch)) catch return CalcError.OutOfMemory; + // setVar copies, so storing an arena-allocated value is safe. + env.setVar(a.name, val) catch return CalcError.OutOfMemory; return val; }, .unary => |u| { @@ -316,14 +365,16 @@ fn evalSingleArgFn(name: []const u8, x: f64) ?f64 { /// Result of evaluation with metadata for display decisions. pub const EvalInfo = struct { - value: f64, + /// The computed value. Allocated with the allocator passed to + /// `evalStringInfo`; the caller owns it and should `deinit` when done. + value: Number, /// True if the expression contained any non-decimal literal (hex/oct/bin). has_nondecimal_literal: bool, }; /// High-level evaluate: parse a string and evaluate it. -/// Updates env.ans on success. -pub fn evalString(env: *Environment, allocator: Allocator, source: []const u8) CalcError!f64 { +/// Updates env.ans on success. The caller owns the returned value. +pub fn evalString(env: *Environment, allocator: Allocator, source: []const u8) CalcError!Number { const info = try evalStringInfo(env, allocator, source); return info.value; } @@ -333,8 +384,17 @@ pub fn evalString(env: *Environment, allocator: Allocator, source: []const u8) C pub fn evalStringInfo(env: *Environment, allocator: Allocator, source: []const u8) CalcError!EvalInfo { var p = Parser.init(allocator, source, env.mode); const expr = try p.parse(); - const result = try evaluate(env, expr); - env.ans = result; + + // Intermediates live in a scratch arena; only the final value is copied out + // into the caller's allocator. + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + const scratch = arena.allocator(); + + const raw = try evalExact(env, scratch, expr); + const result = raw.cloneWith(allocator) catch |err| return mapError(err); + + env.setAns(result) catch return CalcError.OutOfMemory; env.history_len += 1; return .{ .value = result, @@ -364,13 +424,19 @@ fn hasNonDecimalLiteral(expr: *const Expr) bool { const testing = std.testing; +/// Evaluate and collapse to f64. +/// +/// The exact result is converted here rather than at every call site, which is +/// what lets the ~90 pre-existing f64 assertions in this file stay untouched +/// while the engine itself moved to `Number`. fn testEval(source: []const u8) !f64 { var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); defer _ = arena.deinit(); const alloc = arena.allocator(); var env = Environment.init(alloc, .standard); defer env.deinit(); - return evalString(&env, alloc, source); + const result = try evalString(&env, alloc, source); + return result.toFloat(alloc); } fn testEvalProgrammer(source: []const u8) !f64 { @@ -379,7 +445,8 @@ fn testEvalProgrammer(source: []const u8) !f64 { const alloc = arena.allocator(); var env = Environment.init(alloc, .programmer); defer env.deinit(); - return evalString(&env, alloc, source); + const result = try evalString(&env, alloc, source); + return result.toFloat(alloc); } test "eval simple number" { @@ -550,10 +617,10 @@ test "eval variable assignment and use" { defer env.deinit(); const assign_result = try evalString(&env, alloc, "X = 42"); - try testing.expectEqual(@as(f64, 42.0), assign_result); + try testing.expectEqual(@as(f64, 42.0), assign_result.toFloat(alloc)); const use_result = try evalString(&env, alloc, "X + 8"); - try testing.expectEqual(@as(f64, 50.0), use_result); + try testing.expectEqual(@as(f64, 50.0), use_result.toFloat(alloc)); } test "eval Ans" { @@ -565,7 +632,7 @@ test "eval Ans" { _ = try evalString(&env, alloc, "7 * 6"); const result = try evalString(&env, alloc, "Ans + 1"); - try testing.expectEqual(@as(f64, 43.0), result); + try testing.expectEqual(@as(f64, 43.0), result.toFloat(alloc)); } test "eval complex expression" { @@ -668,7 +735,7 @@ test "evalStringInfo: detects hex literal" { var env = Environment.init(alloc, .standard); defer env.deinit(); const info = try evalStringInfo(&env, alloc, "0o777 - 0x0f"); - try testing.expectEqual(@as(f64, 496.0), info.value); + try testing.expectEqual(@as(f64, 496.0), info.value.toFloat(alloc)); try testing.expect(info.has_nondecimal_literal); } @@ -679,7 +746,7 @@ test "evalStringInfo: pure decimal has no nondecimal literal" { var env = Environment.init(alloc, .standard); defer env.deinit(); const info = try evalStringInfo(&env, alloc, "2 + 2"); - try testing.expectEqual(@as(f64, 4.0), info.value); + try testing.expectEqual(@as(f64, 4.0), info.value.toFloat(alloc)); try testing.expect(!info.has_nondecimal_literal); } @@ -851,3 +918,161 @@ test "exact: overflow from an absurd exponent is reported as overflow" { // silently producing infinity or exhausting memory. try testing.expectError(CalcError.Overflow, testEval("2 ^ 3000000")); } + +// -- Exactness visible through the Number API (Task 2.0c) -- +// +// These are the cases the f64 boundary could not express. `testEval` collapses +// to f64 and would lose exactly the information under test here. + +/// Evaluate and keep the exact result. The arena owns everything. +fn testEvalNumber(arena: *std.heap.ArenaAllocator, source: []const u8) !Number { + const a = arena.allocator(); + var env = Environment.init(a, .standard); + defer env.deinit(); + return evalString(&env, a, source); +} + +fn expectExactDecimal(expected: []const u8, source: []const u8) !void { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const result = try testEvalNumber(&arena, source); + try testing.expect(result.isExact()); + const shown = try result.toDecimalString(arena.allocator(), 20); + try testing.expectEqualStrings(expected, shown.text); +} + +test "Number API: the integer f64 cannot hold round-trips" { + // The headline case. Through the f64 boundary this became + // 9.007199254740992e15, a DIFFERENT integer than the one typed. + try expectExactDecimal("9007199254740993", "9007199254740993"); + try expectExactDecimal("9007199254740993", "2^53 + 1"); + try expectExactDecimal("9007199254740992", "2^53"); +} + +test "Number API: exact integer arithmetic is unbounded" { + try expectExactDecimal("1267650600228229401496703205376", "2^100"); + try expectExactDecimal("100000000000000000001", "1e20 + 1"); + try expectExactDecimal("121932631112635269", "123456789 * 987654321"); +} + +test "Number API: one third is retained exactly, not as a decimal" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const result = try testEvalNumber(&arena, "1/3"); + try testing.expect(result.isExact()); + + // The exact form is a fraction, which no float could express. + const frac = (try result.toFractionString(arena.allocator())).?; + try testing.expectEqualStrings("1/3", frac); + + // And its decimal rendering is correctly reported as approximate. + const shown = try result.toDecimalString(arena.allocator(), 10); + try testing.expect(!shown.exact); +} + +test "Number API: factorial is exact past the old 170 limit" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const result = try testEvalNumber(&arena, "factorial(171)"); + try testing.expect(result.isExact()); + + const shown = try result.toDecimalString(arena.allocator(), 0); + // 171! has 310 digits; f64 could only report infinity. + try testing.expectEqual(@as(usize, 310), shown.text.len); + try testing.expect(shown.exact); +} + +test "Number API: transcendentals are reported as inexact" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + + const s = try testEvalNumber(&arena, "sin(1)"); + try testing.expect(!s.isExact()); + + const p = try testEvalNumber(&arena, "pi"); + try testing.expect(!p.isExact()); + + const r = try testEvalNumber(&arena, "sqrt(2)"); + try testing.expect(!r.isExact()); + + // But a perfect square stays exact. + const q = try testEvalNumber(&arena, "sqrt(144)"); + try testing.expect(q.isExact()); +} + +test "Number API: inexactness is contagious across an expression" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const result = try testEvalNumber(&arena, "0.1 + 0.2 + sin(0)"); + try testing.expect(!result.isExact()); +} + +test "Number API: variables keep the exactness of their expression" { + // This is what storing Number in the Environment buys: previously the + // assignment round-tripped through f64 and 0.1 came back approximated. + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const a = arena.allocator(); + var env = Environment.init(a, .standard); + defer env.deinit(); + + const assigned = try evalString(&env, a, "X = 0.1"); + try testing.expect(assigned.isExact()); + + const sum = try evalString(&env, a, "X + 0.2"); + try testing.expect(sum.isExact()); + const shown = try sum.toDecimalString(a, 20); + try testing.expectEqualStrings("0.3", shown.text); + try testing.expect(shown.exact); +} + +test "Number API: Ans keeps exactness between evaluations" { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const a = arena.allocator(); + var env = Environment.init(a, .standard); + defer env.deinit(); + + _ = try evalString(&env, a, "1/3"); + const doubled = try evalString(&env, a, "Ans * 3"); + try testing.expect(doubled.isExact()); + const shown = try doubled.toDecimalString(a, 20); + try testing.expectEqualStrings("1", shown.text); +} + +test "Number API: reassigning a variable releases the old value" { + // Exercises the replace path in setVar, which must deinit the previous + // Number rather than leaking it. + var env = Environment.init(testing.allocator, .standard); + defer env.deinit(); + + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const a = arena.allocator(); + + _ = try evalString(&env, a, "X = 1/3"); + _ = try evalString(&env, a, "X = 2/7"); + _ = try evalString(&env, a, "X = 5"); + const result = try evalString(&env, a, "X * 2"); + try testing.expectEqual(@as(f64, 10.0), result.toFloat(a)); +} + +test "Number API: a variable name outliving its source text stays valid" { + // setVar duplicates the name because it points into the expression source, + // which the caller may free (the TUI frees history on Ctrl-L). + var env = Environment.init(testing.allocator, .standard); + defer env.deinit(); + + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer _ = arena.deinit(); + const a = arena.allocator(); + + { + const source = try testing.allocator.dupe(u8, "myvar = 42"); + defer testing.allocator.free(source); + _ = try evalString(&env, a, source); + } + // The source is gone; the stored name must still resolve. + const result = try evalString(&env, a, "myvar + 1"); + try testing.expectEqual(@as(f64, 43.0), result.toFloat(a)); +} diff --git a/engine/src/formatter.zig b/engine/src/formatter.zig index 444fff1..956d049 100644 --- a/engine/src/formatter.zig +++ b/engine/src/formatter.zig @@ -15,6 +15,7 @@ const std = @import("std"); const types = @import("types.zig"); const BitWidth = types.BitWidth; const Endianness = types.Endianness; +const Number = @import("number.zig").Number; /// A formatted value with both display and clipboard representations. pub const FormattedValue = struct { @@ -24,8 +25,12 @@ pub const FormattedValue = struct { /// Format a floating-point value for display. /// Uses comma grouping for integers, avoids scientific notation unless necessary. +/// +/// The 2^53 bound below is NOT a display preference: past it an f64 no longer +/// distinguishes consecutive integers, so printing one as an exact-looking +/// integer would assert precision the value does not have. Exact values are not +/// subject to this and go through `formatNumber`, which prints them in full. pub fn formatFloat(buf: []u8, value: f64) FormattedValue { - // Check if value is an integer (no fractional part, within safe range) const is_integer = value == @trunc(value) and @abs(value) < 9007199254740992.0; // 2^53 if (is_integer and @abs(value) < 1e15) { @@ -73,6 +78,181 @@ pub fn formatCompactFloat(buf: []u8, value: f64) []const u8 { return std.fmt.bufPrint(buf, "{d}", .{value}) catch return "ERR"; } +/// Format a `Number` for display, preserving exactness where it exists. +/// +/// The rules, per NFR-9.9: +/// - An exact **integer** prints in full with comma grouping, at ANY magnitude. +/// It deliberately does not switch to scientific notation: printing +/// `9007199254740993` correctly is the entire point of the exact tier, and +/// abbreviating it would throw the result away at the last step. +/// - An exact value with a **terminating** decimal expansion prints exactly. +/// - An exact value with a **repeating** expansion is rounded to +/// `exact_fraction_digits` and reported as approximate. +/// - An **inexact** value uses the float rules (`formatFloat`) and is always +/// reported as approximate, because rounding already happened. +/// +/// Caller owns `display` and `raw`. +pub fn formatNumber(allocator: std.mem.Allocator, value: Number) !NumberDisplay { + switch (value) { + .inexact => |f| { + var buf: [512]u8 = undefined; + const formatted = formatFloat(&buf, f); + const display = try allocator.dupe(u8, formatted.display); + errdefer allocator.free(display); + const raw = try allocator.dupe(u8, formatted.raw); + return .{ .display = display, .raw = raw, .exact = false }; + }, + .exact => |r| { + const rendered = try r.toDecimalString(allocator, exact_fraction_digits); + errdefer allocator.free(rendered.text); + + // 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. + if (integerDigitCount(rendered.text) > max_display_integer_digits) { + const display = try scientificFromDecimalText(allocator, rendered.text); + return .{ .display = display, .raw = rendered.text, .exact = false }; + } + + // Group the integer part for readability; the raw form stays plain. + const display = try groupDecimalText(allocator, rendered.text); + return .{ .display = display, .raw = rendered.text, .exact = rendered.exact }; + }, + } +} + +/// Fractional digits produced for an exact value whose decimal expansion does +/// not terminate (1/3, 1/7). Exact arithmetic can justify more digits than f64, +/// so this is above f64's ~17 significant digits. +pub const exact_fraction_digits: usize = 20; + +/// Integer digits shown in full before display switches to scientific notation. +/// +/// The exact tier exists so values like `9007199254740993` (16 digits) and +/// `2^128` (39 digits) print correctly, so the cap must be comfortably above +/// those. It exists at all because without it `factorial(171)` renders 310 +/// digits and `1.5e300 * 10` renders 301, which is accurate but unreadable. +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; + const dot = std.mem.indexOfScalar(u8, text, '.') orelse text.len; + return dot - start; +} + +/// 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, + }); +} + +pub const NumberDisplay = struct { + /// Human-readable form, with comma grouping. + display: []const u8, + /// Clipboard form: no separators. + raw: []const u8, + /// False when the text is a rounded approximation of the true value. + exact: bool, + + pub fn deinit(self: NumberDisplay, allocator: std.mem.Allocator) void { + allocator.free(self.display); + allocator.free(self.raw); + } +}; + +/// Insert comma separators into the integer part of decimal text, leaving any +/// sign and fractional part alone. +fn groupDecimalText(allocator: std.mem.Allocator, text: []const u8) ![]u8 { + 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; + + // Nothing to group. + if (int_digits <= 3) return allocator.dupe(u8, text); + + const separators = (int_digits - 1) / 3; + var out = try allocator.alloc(u8, text.len + separators); + + var w: usize = 0; + @memcpy(out[0..start], text[0..start]); + w = start; + + var i: usize = 0; + while (i < int_digits) : (i += 1) { + if (i > 0 and (int_digits - i) % 3 == 0) { + out[w] = ','; + w += 1; + } + out[w] = text[start + i]; + w += 1; + } + @memcpy(out[w..], text[dot..]); + return out; +} + /// Format an integer for programmer mode hex display. /// Display: "FF FF FF FF" (space per byte), byte order per `endian`. /// Raw: "0xFFFFFFFF" (no separators, canonical MSB-first value regardless of @@ -643,3 +823,230 @@ test "formatCompactFloat: non-finite values" { try testing.expectEqualStrings("-inf", formatCompactFloat(&buf, -std.math.inf(f64))); try testing.expectEqualStrings("nan", formatCompactFloat(&buf, std.math.nan(f64))); } + +// -- formatNumber (exact display) -- + +fn expectNumberDisplay(expected_display: []const u8, expected_exact: bool, value: Number) !void { + const shown = try formatNumber(testing.allocator, value); + defer shown.deinit(testing.allocator); + try testing.expectEqualStrings(expected_display, shown.display); + try testing.expectEqual(expected_exact, shown.exact); +} + +test "formatNumber: exact small integer" { + var n = try Number.fromInt(testing.allocator, 42); + defer n.deinit(); + try expectNumberDisplay("42", true, n); +} + +test "formatNumber: exact integer gets comma grouping" { + var n = try Number.fromInt(testing.allocator, 4294967295); + defer n.deinit(); + try expectNumberDisplay("4,294,967,295", true, n); + + const shown = try formatNumber(testing.allocator, n); + defer shown.deinit(testing.allocator); + // The clipboard form keeps no separators. + try testing.expectEqualStrings("4294967295", shown.raw); +} + +test "formatNumber: the integer f64 cannot represent survives intact" { + // The whole point of the exact tier: this must NOT become + // 9.007199254740992e15. + var n = try Number.parse(testing.allocator, "9007199254740993"); + defer n.deinit(); + try expectNumberDisplay("9,007,199,254,740,993", true, n); +} + +test "formatNumber: huge exact integers print in full, never scientific" { + var n = try Number.parse(testing.allocator, "123456789012345678901234567890"); + defer n.deinit(); + try expectNumberDisplay("123,456,789,012,345,678,901,234,567,890", true, n); +} + +test "formatNumber: negative exact integer" { + var n = try Number.fromInt(testing.allocator, -1234567); + defer n.deinit(); + try expectNumberDisplay("-1,234,567", true, n); +} + +test "formatNumber: exact terminating fraction" { + var n = try Number.parse(testing.allocator, "0.125"); + defer n.deinit(); + try expectNumberDisplay("0.125", true, n); +} + +test "formatNumber: exact terminating fraction with a grouped integer part" { + var n = try Number.parse(testing.allocator, "1234567.25"); + defer n.deinit(); + try expectNumberDisplay("1,234,567.25", true, n); +} + +test "formatNumber: 0.1 + 0.2 renders as 0.3 exactly" { + const alloc = testing.allocator; + var a = try Number.parse(alloc, "0.1"); + defer a.deinit(); + var b = try Number.parse(alloc, "0.2"); + defer b.deinit(); + var sum = try Number.add(alloc, a, b); + defer sum.deinit(); + try expectNumberDisplay("0.3", true, sum); +} + +test "formatNumber: repeating expansion is rounded and flagged approximate" { + const alloc = testing.allocator; + var one = try Number.fromInt(alloc, 1); + defer one.deinit(); + var three = try Number.fromInt(alloc, 3); + defer three.deinit(); + var third = try Number.div(alloc, one, three); + defer third.deinit(); + + const shown = try formatNumber(alloc, third); + defer shown.deinit(alloc); + try testing.expect(!shown.exact); + try testing.expect(std.mem.startsWith(u8, shown.display, "0.3333333333")); + try testing.expectEqual(exact_fraction_digits + 2, shown.display.len); // "0." + digits +} + +test "formatNumber: inexact values are always flagged approximate" { + var n = Number.fromFloat(0.5); + defer n.deinit(); + try expectNumberDisplay("0.5", false, n); + + var whole = Number.fromFloat(42.0); + defer whole.deinit(); + try expectNumberDisplay("42", false, whole); +} + +test "formatNumber: negative zero and zero" { + var z = try Number.fromInt(testing.allocator, 0); + defer z.deinit(); + try expectNumberDisplay("0", true, z); +} + +test "groupDecimalText: boundaries around the grouping threshold" { + const alloc = testing.allocator; + const cases = [_][2][]const u8{ + .{ "1", "1" }, + .{ "12", "12" }, + .{ "123", "123" }, + .{ "1234", "1,234" }, + .{ "12345", "12,345" }, + .{ "123456", "123,456" }, + .{ "1234567", "1,234,567" }, + .{ "-1234567", "-1,234,567" }, + .{ "1234.5678", "1,234.5678" }, + .{ "-1234.5", "-1,234.5" }, + .{ "0.123456789", "0.123456789" }, + }; + for (cases) |c| { + const got = try groupDecimalText(alloc, c[0]); + defer alloc.free(got); + try testing.expectEqualStrings(c[1], got); + } +} + +// -- Display cap for very long exact values (NFR-7) -- + +test "formatNumber: exact integers at the cap still print in full" { + // 2^128 is 39 digits, inside the cap, and is a value the exact tier exists + // to serve. + var n = try Number.parse(testing.allocator, "340282366920938463463374607431768211456"); + defer n.deinit(); + const shown = try formatNumber(testing.allocator, n); + defer shown.deinit(testing.allocator); + try testing.expect(shown.exact); + try testing.expectEqualStrings("340,282,366,920,938,463,463,374,607,431,768,211,456", shown.display); +} + +test "formatNumber: past the cap the display abbreviates but raw stays exact" { + const alloc = testing.allocator; + // 1e50: 51 digits, past the cap. + var n = try Number.parse(alloc, "1e50"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + + try testing.expectEqualStrings("1e50", shown.display); + // The exact value is never lost, just not shown inline. + try testing.expectEqual(@as(usize, 51), shown.raw.len); + try testing.expectEqualStrings("1", shown.raw[0..1]); + // The abbreviated text is not the full value, so it is flagged. + try testing.expect(!shown.exact); +} + +test "formatNumber: abbreviation rounds the mantissa" { + const alloc = testing.allocator; + // 41 nines: rounds up and carries all the way into a new power of ten. + var n = try Number.parse(alloc, "99999999999999999999999999999999999999999"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + try testing.expectEqualStrings("1e41", shown.display); +} + +test "formatNumber: abbreviation rounds a middle digit without carrying" { + const alloc = testing.allocator; + // 41 digits whose 18th is 8, so the 17th significant digit rounds 7 -> 8 + // with no carry propagation. + var n = try Number.parse(alloc, "12345678901234567800000000000000000000000"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + try testing.expectEqualStrings("1.2345678901234568e40", shown.display); +} + +test "formatNumber: abbreviation rounds down when the next digit is below five" { + const alloc = testing.allocator; + var n = try Number.parse(alloc, "12345678901234567400000000000000000000000"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + try testing.expectEqualStrings("1.2345678901234567e40", shown.display); +} + +test "formatNumber: negative values past the cap keep their sign" { + const alloc = testing.allocator; + var n = try Number.parse(alloc, "-1.5e60"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + try testing.expectEqualStrings("-1.5e60", shown.display); +} + +test "formatNumber: 9007199254740993 is above NFR-7's f64 bound but must print in full" { + // This is the case where NFR-7's 10^15 scientific-notation bound would be + // actively wrong: the value exceeds it, but abbreviating would reintroduce + // the exact bug NFR-9.1 forbids. + const alloc = testing.allocator; + var n = try Number.parse(alloc, "9007199254740993"); + defer n.deinit(); + const shown = try formatNumber(alloc, n); + defer shown.deinit(alloc); + try testing.expect(shown.exact); + try testing.expectEqualStrings("9,007,199,254,740,993", shown.display); + try testing.expectEqualStrings("9007199254740993", shown.raw); +} + +test "scientificFromDecimalText: mantissa trimming and exponents" { + const alloc = testing.allocator; + const cases = [_][2][]const u8{ + // 41 digits so the cap is exceeded in every case. + .{ "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); + } +} + +test "integerDigitCount ignores sign and fraction" { + try testing.expectEqual(@as(usize, 3), integerDigitCount("123")); + try testing.expectEqual(@as(usize, 3), integerDigitCount("-123")); + try testing.expectEqual(@as(usize, 3), integerDigitCount("123.456")); + try testing.expectEqual(@as(usize, 1), integerDigitCount("0.5")); +} diff --git a/engine/src/number.zig b/engine/src/number.zig index a23ab8b..ef17fb9 100644 --- a/engine/src/number.zig +++ b/engine/src/number.zig @@ -68,6 +68,14 @@ pub const Number = union(enum) { }; } + /// Copy into a different allocator. See `Rational.cloneWith`. + pub fn cloneWith(self: Number, allocator: Allocator) Error!Number { + return switch (self) { + .exact => |r| .{ .exact = try r.cloneWith(allocator) }, + .inexact => |f| .{ .inexact = f }, + }; + } + // -- Queries -- pub fn isExact(self: Number) bool { diff --git a/engine/src/rational.zig b/engine/src/rational.zig index 79054c3..cbb928c 100644 --- a/engine/src/rational.zig +++ b/engine/src/rational.zig @@ -85,9 +85,19 @@ pub const Rational = struct { } pub fn clone(self: Rational) Error!Rational { - var num = try self.num.clone(); + return self.cloneWith(self.num.allocator); + } + + /// Copy into a different allocator. + /// + /// Needed because a `Rational` carries its allocator inside its limbs: a + /// value stored in the environment must be copied into an evaluation's + /// scratch arena (and vice versa) rather than shared, or one side will free + /// memory the other still refers to. + pub fn cloneWith(self: Rational, allocator: Allocator) Error!Rational { + var num = try self.num.cloneWithDifferentAllocator(allocator); errdefer num.deinit(); - const den = try self.den.clone(); + const den = try self.den.cloneWithDifferentAllocator(allocator); return .{ .num = num, .den = den }; } diff --git a/engine/src/tokenizer.zig b/engine/src/tokenizer.zig index 265b23f..4ee8b53 100644 --- a/engine/src/tokenizer.zig +++ b/engine/src/tokenizer.zig @@ -620,7 +620,12 @@ test "tokenize base literal with comma separator" { try testing.expectEqual(TokenKind.eof, tok.next().kind); } -test "parseNumber huge decimal falls back to float" { +test "parseNumber huge decimal exceeds u64 and falls back to float here" { + // The tokenizer's own integer channel is a u64, so a value this large has no + // `int_value` at this layer. That is NOT a precision limit of the engine: + // the evaluator re-parses the literal text into an exact rational (see + // evaluator.literalToNumber), so `99999999999999999999999999` still + // evaluates exactly. This test pins the tokenizer's contract only. const result = try parseNumber("99999999999999999999999999"); try testing.expectEqual(@as(?u64, null), result.int_value); try testing.expectEqual(Base.decimal, result.base); diff --git a/src/main.zig b/src/main.zig index 6e60824..596c672 100644 --- a/src/main.zig +++ b/src/main.zig @@ -169,28 +169,41 @@ pub fn evaluate(allocator: std.mem.Allocator, expression: []const u8, mode: engi // "32F to C". Anything without it falls through to normal evaluation. if (engine.units.parseRequest(expression)) |maybe_request| { if (maybe_request) |request| { - const value = engine.evalString(&env, allocator, request.value_text) catch |err| { + var value = engine.evalString(&env, allocator, request.value_text) catch |err| { return .{ .output = errorMessage(err), .is_error = true }; }; - return formatConversionUnits(buf, value, request.from, request.to); + defer value.deinit(); + // Conversion factors are still f64 (see Task 2.0e), so the value + // collapses here regardless. + return formatConversionUnits(buf, value.toFloat(allocator), request.from, request.to); } } else |err| { return .{ .output = errorMessage(err), .is_error = true }; } - const info = engine.evalStringInfo(&env, allocator, expression) catch |err| { + var info = engine.evalStringInfo(&env, allocator, expression) catch |err| { return .{ .output = errorMessage(err), .is_error = true }; }; - - const formatted = engine.formatter.formatFloat(buf, info.value); + defer info.value.deinit(); // Enrich with multi-base view when the expression used non-decimal // literals and the result is a non-negative integer. - if (info.has_nondecimal_literal and isDisplayableInt(info.value)) { - return formatStandardMultiBase(buf, formatted.display, info.value); + if (info.has_nondecimal_literal and isDisplayableInt(info.value.toFloat(allocator))) { + var base_buf: [4096]u8 = undefined; + const decimal = engine.formatter.formatFloat(&base_buf, info.value.toFloat(allocator)); + return formatStandardMultiBase(buf, decimal.display, info.value.toFloat(allocator)); } - return .{ .output = formatted.display, .is_error = false }; + const shown = engine.formatter.formatNumber(allocator, info.value) catch { + return .{ .output = "error: out of memory\n", .is_error = true }; + }; + // Copy into the caller's buffer so the result does not depend on the + // allocator outliving this call. + if (shown.display.len > buf.len) { + return .{ .output = "error: result too long to display\n", .is_error = true }; + } + @memcpy(buf[0..shown.display.len], shown.display); + return .{ .output = buf[0..shown.display.len], .is_error = false }; } /// True if the f64 is a non-negative integer within u128 range. diff --git a/src/tui.zig b/src/tui.zig index 49b6224..f26d628 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -351,7 +351,7 @@ pub const App = struct { /// can actually hold it: integer views for integers, and the IEEE 754 float /// overlay for fractions, infinities, NaN, and out-of-range magnitudes. fn loadAnsIntoProgrammer(self: *App) void { - const ans = self.env.ans; + const ans = self.env.ansFloat(); if (ans == @trunc(ans) and ans >= -9223372036854775808.0 and ans < 18446744073709551616.0) { self.float_view_active = false; if (ans >= 0) { @@ -843,22 +843,27 @@ pub const App = struct { return; } - const info = engine.evalStringInfo(&self.env, self.allocator, expr_text) catch |err| { + var info = engine.evalStringInfo(&self.env, self.allocator, expr_text) catch |err| { const msg = try self.allocator.dupe(u8, errorStr(err)); try self.history.append(self.allocator, .{ .expr = expr_text, .result = msg, .is_error = true }); return; }; + defer info.value.deinit(); - var fmt_buf: [4096]u8 = undefined; - const formatted = engine.formatter.formatFloat(&fmt_buf, info.value); - const result_copy = try self.allocator.dupe(u8, formatted.display); + // Exact results render in full, so an exact integer past f64's 2^53 + // limit reaches the user intact instead of collapsing to scientific + // notation. + const shown = try engine.formatter.formatNumber(self.allocator, info.value); + defer shown.deinit(self.allocator); + const result_copy = try self.allocator.dupe(u8, shown.display); var details: ?[3][]const u8 = null; - if (info.has_nondecimal_literal and info.value >= 0 and - info.value == @trunc(info.value) and - info.value < 340282366920938463463374607431768211456.0) + const as_float = info.value.toFloat(self.allocator); + if (info.has_nondecimal_literal and as_float >= 0 and + as_float == @trunc(as_float) and + as_float < 340282366920938463463374607431768211456.0) { - const int_val: u128 = @intFromFloat(info.value); + const int_val: u128 = @intFromFloat(as_float); const bw = engine.types.BitWidth.smallestFor(int_val); var hex_buf: [256]u8 = undefined; var oct_buf: [256]u8 = undefined; @@ -901,11 +906,15 @@ pub const App = struct { /// is taken directly; anything else is evaluated as a standard expression so /// things like "2*3.5" or "sqrt(2)" work as the input value. fn submitConvert(self: *App, expr_text: []const u8) !void { - const value: f64 = std.fmt.parseFloat(f64, expr_text) catch - engine.evalString(&self.env, self.allocator, expr_text) catch |err| { - const msg = try self.allocator.dupe(u8, errorStr(err)); - try self.history.append(self.allocator, .{ .expr = expr_text, .result = msg, .is_error = true }); - return; + const value: f64 = std.fmt.parseFloat(f64, expr_text) catch blk: { + var evaluated = engine.evalString(&self.env, self.allocator, expr_text) catch |err| { + const msg = try self.allocator.dupe(u8, errorStr(err)); + try self.history.append(self.allocator, .{ .expr = expr_text, .result = msg, .is_error = true }); + return; + }; + defer evaluated.deinit(); + // Conversion factors are still f64 (Task 2.0e), so collapse here. + break :blk evaluated.toFloat(self.allocator); }; self.conv_value = value; @@ -928,11 +937,15 @@ pub const App = struct { /// Evaluate and record a standard-mode unit conversion ("100 km to mi"). fn submitStandardConversion(self: *App, expr_text: []const u8, request: engine.units.ConversionRequest) !void { - const value = engine.evalString(&self.env, self.allocator, request.value_text) catch |err| { + var evaluated = engine.evalString(&self.env, self.allocator, request.value_text) catch |err| { const msg = try self.allocator.dupe(u8, errorStr(err)); try self.history.append(self.allocator, .{ .expr = expr_text, .result = msg, .is_error = true }); return; }; + defer evaluated.deinit(); + // Conversion factors are still f64 (Task 2.0e), so collapse here. + const value = evaluated.toFloat(self.allocator); + const converted = engine.units.convertUnits(value, request.from, request.to) catch |err| { const msg = try self.allocator.dupe(u8, errorStr(err)); try self.history.append(self.allocator, .{ .expr = expr_text, .result = msg, .is_error = true });