From b84e8648bc5d85035f639717cab56eda7d2e31f1 Mon Sep 17 00:00:00 2001 From: Robert Yokota Date: Sun, 14 Jun 2026 19:39:09 -0700 Subject: [PATCH] Fix $formatInteger word/Roman/letter formatting for values beyond the int64 range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `$formatInteger(1e46, 'w')` (and other magnitudes above `INT64_MAX`) produced garbage or crashed instead of returning `"ten billion trillion trillion trillion"`. The builtin coerced its argument with `Utils::toLong`, narrowing the double to `int64_t`. For values beyond the int64 range this is **undefined behavior**: on x86-64 it yields `INT64_MIN` (negative), on ARM64 it saturates to `INT64_MAX`. Either way the value then indexed out of bounds in the word tables (`few[]`, `decades[]`, `magnitudes[]`) — a silent junk read on release libc++/libstdc++, and a hard assertion abort on MSVC's checked iterators. This case was previously hidden behind an `ignoreError: true` test override; it never actually computed the correct result on any platform. ## Fix Carry the value as a `double` all the way through the word/Roman/letter formatting path, matching the JSONata **JS reference** (which is the source of the test suite's expected value): - `formatInteger` builtin now reads the argument via `Utils::toDouble` instead of `Utils::toLong`. - `formatInteger`, `numberToWords`, and `lookup` signatures changed from `int64_t` to `double`. - `lookup` rewritten with double arithmetic (`std::floor` / `std::fmod` / `std::pow`); `formatInteger` floors the value up front (matching JS `Math.floor`) and renders the DECIMAL case via `std::ostringstream` with `std::fixed`/`setprecision(0)` to avoid scientific notation. - Removed the now-obsolete `function-formatInteger/formatInteger.json_43` override so the case is verified for real. ### Note on floating-point contraction After the rewrite, the result was correct except for a spurious trailing remainder. The cause was FP contraction: at `-O2`, clang fuses `num - mant * factor` into a single full-precision FMA, whereas JS rounds `mant * factor` to a double before subtracting. Computing the product in its own statement (`double product = mant * factor;`) forces the intermediate rounding and reproduces the JS result exactly. ## Compatibility note The Java reference (`jsonata-java`) uses `long` throughout and cannot represent these magnitudes, so this is an intentional divergence from the Java port toward the JS reference for out-of-int64 values. ## Testing - All 1655 generated tests pass, including the previously-overridden `formatInteger` case for `1e46`. Co-Authored-By: Claude Opus 4.8 (1M context) --- include/jsonata/Functions.h | 2 +- include/jsonata/utils/DateTimeUtils.h | 8 ++--- src/jsonata/Functions.cpp | 9 +++--- src/jsonata/utils/DateTimeUtils.cpp | 46 +++++++++++++++++++-------- test/test-overrides.json | 5 --- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/include/jsonata/Functions.h b/include/jsonata/Functions.h index f71bcd9..3ed243f 100644 --- a/include/jsonata/Functions.h +++ b/include/jsonata/Functions.h @@ -160,7 +160,7 @@ class Functions { static std::optional dateTimeFromMillis( int64_t millis, const std::string& picture = "", const std::string& timezone = "UTC"); - static std::optional formatInteger(int64_t value, + static std::optional formatInteger(double value, const std::string& picture); static std::optional parseInteger(const std::string& value, const std::string& picture); diff --git a/include/jsonata/utils/DateTimeUtils.h b/include/jsonata/utils/DateTimeUtils.h index 5ecf56b..722253a 100644 --- a/include/jsonata/utils/DateTimeUtils.h +++ b/include/jsonata/utils/DateTimeUtils.h @@ -55,7 +55,7 @@ namespace utils { class DateTimeUtils { public: // Number to words conversion - static std::string numberToWords(int64_t value, bool ordinal = false); + static std::string numberToWords(double value, bool ordinal = false); static int64_t wordsToNumber(const std::string& text); static int64_t wordsToLong(const std::string& text); @@ -68,7 +68,7 @@ class DateTimeUtils { static std::string decimalToLetters(int64_t value, const std::string& aChar); // Integer formatting - static std::string formatInteger(int64_t value, const std::string& picture); + static std::string formatInteger(double value, const std::string& picture); // Date/time formatting and parsing static std::string formatDateTime(int64_t millis, @@ -251,8 +251,8 @@ class DateTimeUtils { defaultPresentationModifiers; // Static helper functions - static std::string lookup(int64_t num, bool prev, bool ord); - static std::string formatInteger(int64_t value, const Format& format); + static std::string lookup(double num, bool prev, bool ord); + static std::string formatInteger(double value, const Format& format); static Format analyseIntegerPicture(const std::string& picture); static int64_t getRegularRepeat( const std::vector& separators); diff --git a/src/jsonata/Functions.cpp b/src/jsonata/Functions.cpp index 96e9342..2a6bd98 100644 --- a/src/jsonata/Functions.cpp +++ b/src/jsonata/Functions.cpp @@ -1297,9 +1297,10 @@ Functions::getFunctionRegistry() { if (args.size() < 2 || !isNumber(args[0]) || !isString(args[1])) return std::any(); - // Java: value.longValue() - handle int, long, double types - // properly - int64_t value = Utils::toLong(args[0]); + // Keep the value as a double so magnitudes beyond the + // int64 range (e.g. 1e46) are formatted correctly instead + // of overflowing on narrowing to a 64-bit integer. + double value = Utils::toDouble(args[0]); auto picture = std::any_cast(args[1]); auto result = formatInteger(value, picture); return result ? std::any(*result) : std::any(); @@ -4803,7 +4804,7 @@ std::optional Functions::dateTimeFromMillis( } std::optional Functions::formatInteger( - int64_t value, const std::string& picture) { + double value, const std::string& picture) { // Match Java implementation exactly: return // DateTimeUtils.formatInteger(value.longValue(), picture); try { diff --git a/src/jsonata/utils/DateTimeUtils.cpp b/src/jsonata/utils/DateTimeUtils.cpp index ac0f60f..7848f3e 100644 --- a/src/jsonata/utils/DateTimeUtils.cpp +++ b/src/jsonata/utils/DateTimeUtils.cpp @@ -199,17 +199,21 @@ DateTimeUtils::createDefaultPresentationModifiers() { } // Public methods implementation -std::string DateTimeUtils::numberToWords(int64_t value, bool ordinal) { +std::string DateTimeUtils::numberToWords(double value, bool ordinal) { return lookup(value, false, ordinal); } -std::string DateTimeUtils::lookup(int64_t num, bool prev, bool ord) { +// Operates on double (matching the JSONata JS reference) so that values beyond +// the int64 range (e.g. $formatInteger(1e46, 'w')) are handled instead of +// overflowing to a negative/garbage index into the word tables. +std::string DateTimeUtils::lookup(double num, bool prev, bool ord) { std::string words = ""; if (num <= 19) { - words = (prev ? " and " : "") + (ord ? ordinals[num] : few[num]); + size_t idx = static_cast(num); + words = (prev ? " and " : "") + (ord ? ordinals[idx] : few[idx]); } else if (num < 100) { - int64_t tens = static_cast(num) / 10; - int64_t remainder = static_cast(num) % 10; + size_t tens = static_cast(std::floor(num / 10)); + int64_t remainder = static_cast(std::fmod(num, 10)); words = (prev ? " and " : "") + decades[tens - 2]; if (remainder > 0) { words += "-" + lookup(remainder, false, ord); @@ -217,8 +221,8 @@ std::string DateTimeUtils::lookup(int64_t num, bool prev, bool ord) { words = words.substr(0, words.length() - 1) + "ieth"; } } else if (num < 1000) { - int64_t hundreds = static_cast(num) / 100; - int64_t remainder = static_cast(num) % 100; + size_t hundreds = static_cast(std::floor(num / 100)); + double remainder = std::fmod(num, 100); words = (prev ? ", " : "") + few[hundreds] + " Hundred"; if (remainder > 0) { words += lookup(remainder, true, ord); @@ -230,9 +234,15 @@ std::string DateTimeUtils::lookup(int64_t num, bool prev, bool ord) { if (mag > static_cast(magnitudes.size())) { mag = static_cast(magnitudes.size()); // the largest word } - int64_t factor = static_cast(std::pow(10, mag * 3)); - int64_t mant = static_cast(std::floor(num / factor)); - int64_t remainder = num - mant * factor; + double factor = std::pow(10, static_cast(mag * 3)); + double mant = std::floor(num / factor); + // Compute the product in its own statement so it is rounded to a double + // before the subtraction. This matches the JS reference's semantics and + // prevents the compiler from fusing this into a single full-precision + // multiply-add (FMA), which would leave a spurious non-zero remainder + // for large magnitudes such as 1e46. + double product = mant * factor; + double remainder = num - product; words = (prev ? ", " : "") + lookup(mant, false, false) + " " + magnitudes[mag - 1]; if (remainder > 0) { @@ -399,7 +409,7 @@ int64_t DateTimeUtils::lettersToDecimal(const std::string& letters, char aChar) } // Basic formatInteger method (simplified for now) -std::string DateTimeUtils::formatInteger(int64_t value, +std::string DateTimeUtils::formatInteger(double value, const std::string& picture) { Format format = analyseIntegerPicture(picture); return formatInteger(value, format); @@ -549,10 +559,12 @@ DateTimeUtils::Format DateTimeUtils::analyseIntegerPicture( return format; } -std::string DateTimeUtils::formatInteger(int64_t value, const Format& format) { +std::string DateTimeUtils::formatInteger(double value, const Format& format) { std::string formattedInteger = ""; + // Match the JS reference: value = Math.floor(value) before formatting. + value = std::floor(value); bool negative = value < 0; - value = std::abs(value); + value = std::fabs(value); switch (format.primary) { case Formats::LETTERS: @@ -581,7 +593,13 @@ std::string DateTimeUtils::formatInteger(int64_t value, const Format& format) { break; case Formats::DECIMAL: { - formattedInteger = std::to_string(value); + // value is a non-negative, integral double here; render it as a + // plain decimal string (no scientific notation, no fractional part). + { + std::ostringstream oss; + oss << std::fixed << std::setprecision(0) << value; + formattedInteger = oss.str(); + } // Pad with zeros if needed int64_t padLength = format.mandatoryDigits - diff --git a/test/test-overrides.json b/test/test-overrides.json index 8ab15da..f604b67 100644 --- a/test/test-overrides.json +++ b/test/test-overrides.json @@ -7,11 +7,6 @@ }, - { - "name": "function-formatInteger/formatInteger.json_43", - "ignoreError": true, - "reason": "Do not support number to word for numbers exceeding the 64-bit range" - }, { "name": "function-parseInteger/parseInteger.json_11", "ignoreError": true,