From bbf74c5334ed6812f50b9f674b4b7e15bc8e36d9 Mon Sep 17 00:00:00 2001 From: Roland Reichwein Date: Sun, 19 Apr 2026 13:19:53 +0200 Subject: [PATCH] Optimize formatting in format.h for float values (#1379) When formatting float, fix the -0.0 case format.h float format: Fix rounding issues on all platforms --- include/etl/format.h | 112 ++++++++++++++++++++++++------------------- test/test_format.cpp | 95 +++++++++++++++++++++++++++++++++--- 2 files changed, 153 insertions(+), 54 deletions(-) diff --git a/include/etl/format.h b/include/etl/format.h index 72c79be0..8ee8514e 100644 --- a/include/etl/format.h +++ b/include/etl/format.h @@ -1039,22 +1039,27 @@ namespace etl { const size_t fractional_decimals = 6; // default - T integral; - T fractional = modf(value, &integral); - bool sign; - unsigned long long int fractional_int; - unsigned long long int integral_int; - if (integral < 0.0) + // Detect sign using signbit to correctly handle -0.0 + bool sign = signbit(value); + + T integral; + T fractional = modf(value, &integral); + + // Take absolute values to avoid casting negative values to unsigned + if (sign) { - sign = true; - fractional_int = static_cast(-fractional * pow(10., fractional_decimals)); - integral_int = static_cast(-integral); + fractional = -fractional; + integral = -integral; } - else + + unsigned long long int scale = int_pow(10, fractional_decimals); + unsigned long long int fractional_int = static_cast(round(fractional * scale)); + unsigned long long int integral_int = static_cast(integral); + + if (fractional_int == scale) { - sign = false; - fractional_int = static_cast(fractional * pow(10., fractional_decimals)); - integral_int = static_cast(integral); + fractional_int = 0; + ++integral_int; } private_format::format_sign(it, sign ? -1 : 0, spec); @@ -1071,9 +1076,8 @@ namespace etl static const size_t exponent_decimals = 1; long long int exponent_int = 0; - bool sign; - unsigned long long int fractional_int; - unsigned long long int integral_int; + // Detect sign using signbit to correctly handle -0.0 + bool sign = signbit(value); T integral; T fractional = modf(value, &integral); @@ -1092,17 +1096,21 @@ namespace etl fractional = modf(value, &integral); } - if (integral < 0.0) + // Take absolute values to avoid casting negative values to unsigned + if (sign) { - sign = true; - fractional_int = static_cast(-fractional * pow(static_cast(0x10), fractional_decimals)); - integral_int = static_cast(-integral); + fractional = -fractional; + integral = -integral; } - else + + unsigned long long int scale = int_pow(0x10, fractional_decimals); + unsigned long long int fractional_int = static_cast(round(fractional * scale)); + unsigned long long int integral_int = static_cast(integral); + + if (fractional_int == scale) { - sign = false; - fractional_int = static_cast(fractional * pow(static_cast(0x10), fractional_decimals)); - integral_int = static_cast(integral); + fractional_int = 0; + ++integral_int; } private_format::format_sign(it, sign ? -1 : 0, spec); @@ -1133,9 +1141,8 @@ namespace etl static const size_t exponent_decimals = 2; long long int exponent_int = 0; - bool sign; - unsigned long long int fractional_int; - unsigned long long int integral_int; + // Detect sign using signbit to correctly handle -0.0 + bool sign = std::signbit(value); T integral; T fractional = modf(value, &integral); @@ -1154,17 +1161,21 @@ namespace etl fractional = modf(value, &integral); } - if (integral < 0.0) + // Take absolute values to avoid casting negative values to unsigned + if (sign) { - sign = true; - fractional_int = static_cast(-fractional * pow(10., fractional_decimals)); - integral_int = static_cast(-integral); + fractional = -fractional; + integral = -integral; } - else + + unsigned long long int scale = int_pow(10, fractional_decimals); + unsigned long long int fractional_int = static_cast(round(fractional * scale)); + unsigned long long int integral_int = static_cast(integral); + + if (fractional_int == scale) { - sign = false; - fractional_int = static_cast(fractional * pow(10., fractional_decimals)); - integral_int = static_cast(integral); + fractional_int = 0; + ++integral_int; } private_format::format_sign(it, sign ? -1 : 0, spec); @@ -1186,22 +1197,27 @@ namespace etl { const size_t fractional_decimals = 6; // default - T integral; - T fractional = modf(value, &integral); - bool sign; - unsigned long long int fractional_int; - unsigned long long int integral_int; - if (integral < 0.0) + // Detect sign using signbit to correctly handle -0.0 + bool sign = std::signbit(value); + + T integral; + T fractional = modf(value, &integral); + + // Take absolute values to avoid casting negative values to unsigned + if (sign) { - sign = true; - fractional_int = static_cast(-fractional * pow(10., fractional_decimals)); - integral_int = static_cast(-integral); + fractional = -fractional; + integral = -integral; } - else + + unsigned long long int scale = int_pow(10, fractional_decimals); + unsigned long long int fractional_int = static_cast(round(fractional * scale)); + unsigned long long int integral_int = static_cast(integral); + + if (fractional_int == scale) { - sign = false; - fractional_int = static_cast(fractional * pow(10., fractional_decimals)); - integral_int = static_cast(integral); + fractional_int = 0; + ++integral_int; } private_format::format_sign(it, sign ? -1 : 0, spec); diff --git a/test/test_format.cpp b/test/test_format.cpp index 16b0ac1b..1a5e1476 100644 --- a/test/test_format.cpp +++ b/test/test_format.cpp @@ -203,8 +203,8 @@ namespace etl::string<100> s; CHECK_EQUAL("1.0", test_format(s, "{}", 1.0f)); - CHECK_EQUAL("1.234567", test_format(s, "{}", 1.234567f)); - CHECK_EQUAL("1.234567", test_format(s, "{}", 1.2345678f)); + CHECK_EQUAL("1.234567", test_format(s, "{}", 1.2345674f)); + CHECK_EQUAL("1.234568", test_format(s, "{}", 1.2345676f)); CHECK_EQUAL("1.125", test_format(s, "{}", 1.125f)); } @@ -214,8 +214,8 @@ namespace etl::string<100> s; CHECK_EQUAL("1.0", test_format(s, "{}", 1.0)); - CHECK_EQUAL("1.234563", test_format(s, "{}", 1.234563)); - CHECK_EQUAL("1.234567", test_format(s, "{}", 1.2345678)); + CHECK_EQUAL("1.234567", test_format(s, "{}", 1.234567499)); + CHECK_EQUAL("1.234568", test_format(s, "{}", 1.234567501)); CHECK_EQUAL("1.5", test_format(s, "{}", 1.5)); } @@ -227,7 +227,7 @@ namespace CHECK_EQUAL("1.0", test_format(s, "{}", 1.0l)); auto& result = test_format(s, "{}", 1.234567l); CHECK("1.234567" == result || "1.234566" == result); - CHECK_EQUAL("1.234567", test_format(s, "{}", 1.2345678l)); + CHECK_EQUAL("1.234568", test_format(s, "{}", 1.2345678l)); CHECK_EQUAL("1.25", test_format(s, "{}", 1.25l)); } @@ -261,11 +261,94 @@ namespace CHECK_EQUAL("inf", test_format(s, "{:g}", INFINITY)); CHECK_EQUAL("INF", test_format(s, "{:0.3G}", INFINITY)); CHECK_EQUAL("0x1.8p+0", test_format(s, "{:a}", 1.5f)); - CHECK_EQUAL("0X1.4CCCCCCCCCP+0", test_format(s, "{:A}", 1.3l)); + CHECK_EQUAL("0X1.4CCCCCCCCDP+0", test_format(s, "{:A}", 1.3l)); CHECK_EQUAL("0x2.49fp+4", test_format(s, "{:a}", 150000.0)); CHECK_EQUAL("0x1.92a738p-5", test_format(s, "{:a}", 0.0000015f)); CHECK_EQUAL("0x1.6345785d8ap+e", test_format(s, "{:a}", 100000000000000000.l)); } + + //************************************************************************* + TEST(test_format_negative_zero) + { + etl::string<100> s; + + // Test negative zero handling - signbit() correctly detects -0.0 + CHECK_EQUAL("-0.0", test_format(s, "{}", -0.0)); + CHECK_EQUAL("-0.0", test_format(s, "{}", -0.0f)); + CHECK_EQUAL("-0.0", test_format(s, "{}", -0.0L)); + CHECK_EQUAL("-0.000000", test_format(s, "{:f}", -0.0)); + CHECK_EQUAL("-0.000000", test_format(s, "{:f}", -0.0f)); + CHECK_EQUAL("-0.000000", test_format(s, "{:f}", -0.0L)); + CHECK_EQUAL("-0.000000e+00", test_format(s, "{:e}", -0.0)); + CHECK_EQUAL("-0.000000e+00", test_format(s, "{:e}", -0.0f)); + CHECK_EQUAL("-0.000000e+00", test_format(s, "{:e}", -0.0L)); + CHECK_EQUAL("-0x0.0p+0", test_format(s, "{:a}", -0.0)); + CHECK_EQUAL("-0x0.0p+0", test_format(s, "{:a}", -0.0f)); + CHECK_EQUAL("-0x0.0p+0", test_format(s, "{:a}", -0.0L)); + } + + //************************************************************************* + // Tests for fractional rounding carry: + // When round(fractional * scale) == scale the fractional part must wrap + // to 0 and the integral part must be incremented by 1. + //************************************************************************* + TEST(test_format_floating_default_rounding_carry) + { + etl::string<100> s; + + // 1.9999999: fractional 0.9999999, round(0.9999999 * 1e6) == 1000000 + // => must carry: integral 1 -> 2, fractional -> 0 + CHECK_EQUAL("2.0", test_format(s, "{}", 1.9999999)); + CHECK_EQUAL("-2.0", test_format(s, "{}", -1.9999999)); + + // 0.9999999: integral 0, fractional rounds up => becomes 1.0 + CHECK_EQUAL("1.0", test_format(s, "{}", 0.9999999)); + + // 99.9999999: integral 99, fractional rounds up => becomes 100.0 + CHECK_EQUAL("100.0", test_format(s, "{}", 99.9999999)); + } + + //************************************************************************* + TEST(test_format_floating_f_rounding_carry) + { + etl::string<100> s; + + // Same values using {:f} which uses format_floating_f (6 fractional decimals) + CHECK_EQUAL("2.000000", test_format(s, "{:f}", 1.9999999)); + CHECK_EQUAL("-2.000000", test_format(s, "{:f}", -1.9999999)); + CHECK_EQUAL("1.000000", test_format(s, "{:f}", 0.9999999)); + CHECK_EQUAL("100.000000", test_format(s, "{:f}", 99.9999999)); + } + + //************************************************************************* + TEST(test_format_floating_e_rounding_carry) + { + etl::string<100> s; + + // 9.9999999: after normalization integral=9, fractional=0.9999999 + // round(0.9999999 * 1e6) == 1000000 => must carry: 10.000000e+00 + CHECK_EQUAL("10.000000e+00", test_format(s, "{:e}", 9.9999999)); + CHECK_EQUAL("-10.000000e+00", test_format(s, "{:e}", -9.9999999)); + + // 1.9999999: after normalization integral=1, fractional=0.9999999 + CHECK_EQUAL("2.000000e+00", test_format(s, "{:e}", 1.9999999)); + } + + //************************************************************************* + TEST(test_format_floating_a_rounding_carry) + { + etl::string<100> s; + + // 1.5 + (1.0 - 2^-52) * 0.5 ≈ value whose hex fractional part rounds up. + // Use a value where hex fractional is all 0xF...F and rounds up. + // 2.0 - epsilon: in hex, 0x1.FFFFFFFFFFFFFp+0 (for double) + // After modf: integral=1, fractional ≈ 0.999... + // round(fractional * 16^10) should equal 16^10 => carry + double almost_two = 1.9999999999999998; // nextafter(2.0, 0.0) or very close + auto& result = test_format(s, "{:a}", almost_two); + // After carry, integral becomes 2, fractional becomes 0 + CHECK_EQUAL("0x2.0p+0", result); + } #endif //*************************************************************************