From c97920c4733fa81bd00aab4924fc2cbc5a3dbd2d Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Thu, 28 Aug 2025 14:40:01 -0400 Subject: [PATCH] format utility improvements --- benchmarks/bench_format_comparison.cpp | 7 +- src/format.cpp | 31 ++++---- src/format.hpp | 97 ++------------------------ 3 files changed, 24 insertions(+), 111 deletions(-) diff --git a/benchmarks/bench_format_comparison.cpp b/benchmarks/bench_format_comparison.cpp index b5c41e3..c3ed617 100644 --- a/benchmarks/bench_format_comparison.cpp +++ b/benchmarks/bench_format_comparison.cpp @@ -175,12 +175,11 @@ void benchmark_error_messages() { constexpr int line_number = 123; const std::string error_msg = "File not found"; - // Arena-based static_format (using std::string_view) + // Arena-based static_format (using string literals only) bench.run("static_format", [&] { ArenaAllocator arena(128); - std::string_view error_msg_sv = error_msg; - auto result = static_format(arena, "Error ", error_code, ": ", error_msg_sv, - " (line ", line_number, ")"); + auto result = static_format(arena, "Error ", error_code, ": ", + "File not found", " (line ", line_number, ")"); ankerl::nanobench::doNotOptimizeAway(result); }); diff --git a/src/format.cpp b/src/format.cpp index 60e6ac4..9b9c9dd 100644 --- a/src/format.cpp +++ b/src/format.cpp @@ -2,6 +2,8 @@ #include #include +#include +#include // Copied from simdjson namespace { @@ -980,30 +982,25 @@ std::string_view format(ArenaAllocator &arena, const char *fmt, ...) { static_cast(bytes_written)); } - // Fallback: Two-pass approach when speculative formatting fails - va_list args_fallback; - va_start(args_fallback, fmt); - const int required_size = std::vsnprintf(nullptr, 0, fmt, args_fallback); - va_end(args_fallback); - - if (required_size < 0) { - // Handle formatting error by returning empty string_view - return std::string_view{}; + // Fallback: vsnprintf failed or didn't fit, use two-pass approach + if (bytes_written < 0) { + std::fprintf(stderr, "vsnprintf failed in format()\n"); + std::abort(); } - // Allocate exact buffer size needed (+1 for null terminator) - const int buffer_size = required_size + 1; + // We know bytes_written is the required size + const int buffer_size = bytes_written + 1; char *result = arena.allocate(static_cast(buffer_size)); // Format into the exact-sized buffer - va_start(args_fallback, fmt); - const int actual_size = std::vsnprintf( - result, static_cast(buffer_size), fmt, args_fallback); - va_end(args_fallback); + va_start(args, fmt); + const int actual_size = + std::vsnprintf(result, static_cast(buffer_size), fmt, args); + va_end(args); if (actual_size < 0 || actual_size >= buffer_size) { - // Handle formatting error - return std::string_view{}; + std::fprintf(stderr, "vsnprintf failed in format() fallback\n"); + std::abort(); } // Shrink to exclude null terminator for string_view diff --git a/src/format.hpp b/src/format.hpp index f3eb73e..0db9158 100644 --- a/src/format.hpp +++ b/src/format.hpp @@ -42,7 +42,7 @@ * @param fmt Printf-style format string * @param ... Variable arguments matching format specifiers * @return std::string_view pointing to arena-allocated formatted string - * @note Returns empty string_view on formatting errors + * @note Aborts program on formatting errors (never returns invalid data) * @note GCC format attribute enables compile-time format string validation * * ## Usage Examples: @@ -118,8 +118,9 @@ template constexpr int decimal_length(IntType x) { if constexpr (std::is_signed_v) { // Handle negative values by using unsigned equivalent using Unsigned = std::make_unsigned_t; - auto abs_x = - static_cast(x < 0 ? ~static_cast(x) + 1 : x); + // Safe conversion: cast to unsigned first, then negate in unsigned + // arithmetic + auto abs_x = x < 0 ? -static_cast(x) : static_cast(x); int result = 0; do { ++result; @@ -147,12 +148,12 @@ template struct IntTerm { void write(char *&buf) const { char itoa_buf[kMaxLength]; - Unsigned x = static_cast(v); + auto x = static_cast(v); if constexpr (kSigned) { if (v < 0) { *buf++ = '-'; - x = ~x + 1; + x = -static_cast(v); } } @@ -175,70 +176,6 @@ template constexpr IntTerm term(IntType s) { return IntTerm{s}; } -// Template specializations for common integer types for faster compilation -template <> struct IntTerm { - static constexpr bool kSigned = true; - static constexpr int kMaxLength = 11; // -2147483648 = 11 chars - - explicit constexpr IntTerm(int v) : v(v) {} - - void write(char *&buf) const { - if (v < 0) { - *buf++ = '-'; - write_unsigned( - buf, static_cast(~static_cast(v) + 1)); - } else { - write_unsigned(buf, static_cast(v)); - } - } - -private: - int v; - - static void write_unsigned(char *&buf, unsigned int x) { - char digits[10]; - int i = 0; - do { - digits[i++] = static_cast('0' + (x % 10)); - x /= 10; - } while (x); - while (i > 0) { - *buf++ = digits[--i]; - } - } -}; - -template <> struct IntTerm { - static constexpr bool kSigned = true; - static constexpr int kMaxLength = 20; // -9223372036854775808 = 20 chars - - explicit constexpr IntTerm(int64_t v) : v(v) {} - - void write(char *&buf) const { - if (v < 0) { - *buf++ = '-'; - write_unsigned(buf, static_cast(~static_cast(v) + 1)); - } else { - write_unsigned(buf, static_cast(v)); - } - } - -private: - int64_t v; - - static void write_unsigned(char *&buf, uint64_t x) { - char digits[19]; - int i = 0; - do { - digits[i++] = static_cast('0' + (x % 10)); - x /= 10; - } while (x); - while (i > 0) { - *buf++ = digits[--i]; - } - } -}; - struct DoubleTerm { explicit constexpr DoubleTerm(double s) : s(s) {} static constexpr int kMaxLength = 24; @@ -254,25 +191,6 @@ inline constexpr int max_decimal_length_v = decltype(term(T{}))::kMaxLength; inline constexpr DoubleTerm term(double s) { return DoubleTerm(s); } -// Runtime string term for std::string_view (works with std::string, const -// char*, etc.) -struct StringViewTerm { - explicit constexpr StringViewTerm(std::string_view s) : s(s) {} - static constexpr int kMaxLength = - 512; // Conservative upper bound for runtime strings - void write(char *&buf) const { - std::memcpy(buf, s.data(), s.size()); - buf += s.size(); - } - -private: - std::string_view s; -}; - -inline constexpr StringViewTerm term(std::string_view s) { - return StringViewTerm(s); -} - } // namespace detail /** @@ -347,8 +265,7 @@ inline constexpr StringViewTerm term(std::string_view s) { */ template std::string_view static_format(ArenaAllocator &arena, Ts &&...ts) { - constexpr int upper_bound = - (decltype(detail::term(ts))::kMaxLength + ...) + 1; + constexpr int upper_bound = (decltype(detail::term(ts))::kMaxLength + ...); char *result = arena.allocate(upper_bound); char *buf = result; (detail::term(ts).write(buf), ...);