Improve handling of locales

.github/workflows/ci.yml

@@ -389,6 +389,8 @@ jobs:
             sudo apt-get -o Acquire::Retries=$NET_RETRY_COUNT update
             sudo apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y ${{join(matrix.install, ' ')}} locales libfmt-dev
             sudo locale-gen de_DE.UTF-8
+            sudo locale-gen en_US.UTF-8
+            sudo locale-gen fr_FR.UTF-8
             sudo update-locale
       - name: Setup GCC Toolchain
         if: matrix.gcc_toolchain

include/boost/decimal/cstdio.hpp

@@ -227,9 +227,10 @@ inline auto snprintf_impl(char* buffer, const std::size_t buf_size, const char*
         {
             detail::make_uppercase(buffer, r.ptr);
         }
-        convert_pointer_pair_to_local_locale(buffer, r.ptr);
+        *r.ptr = '\0';
+        const auto offset {convert_pointer_pair_to_local_locale(buffer, buffer + buf_size - byte_count)};
 
-        buffer = r.ptr;
+        buffer = r.ptr + (offset == -1 ? 0 : offset);
 
         if (value_iter != values_list.end())
         {
@@ -276,7 +277,7 @@ inline auto fprintf(std::FILE* buffer, const char* format, const T... values) no
     int bytes {};
     char char_buffer[1024];
 
-    if (format_len + value_space <= 1024U)
+    if (format_len + value_space <= ((1024 * 2) / 3))
     {
         bytes = detail::snprintf_impl(char_buffer, sizeof(char_buffer), format, values...);
         if (bytes)
@@ -287,7 +288,8 @@ inline auto fprintf(std::FILE* buffer, const char* format, const T... values) no
     else
     {
         // LCOV_EXCL_START
-        std::unique_ptr<char[]> longer_char_buffer(new(std::nothrow) char[format_len + value_space + 1]);
+        // Add 50% overage in case we need to do locale conversion
+        std::unique_ptr<char[]> longer_char_buffer(new(std::nothrow) char[(3 * (format_len + value_space + 1)) / 2]);
         if (longer_char_buffer == nullptr)
         {
             errno = ENOMEM;

include/boost/decimal/detail/io.hpp

@@ -79,7 +79,7 @@ auto operator>>(std::basic_istream<charT, traits>& is, DecimalType& d)
         std::memcpy(buffer, t_buffer.c_str(), t_buffer.size());
     }
 
-    detail::convert_string_to_c_locale(buffer);
+    detail::convert_string_to_c_locale(buffer, is.getloc());
 
     auto fmt {chars_format::general};
     const auto flags {is.flags()};
@@ -170,8 +170,7 @@ auto operator<<(std::basic_ostream<charT, traits>& os, const DecimalType& d)
     }
 
     *r.ptr = '\0';
-
-    detail::convert_string_to_local_locale(buffer);
+    detail::convert_pointer_pair_to_local_locale(buffer, buffer + sizeof(buffer), os.getloc());
 
     BOOST_DECIMAL_IF_CONSTEXPR (!std::is_same<charT, char>::value)
     {

include/boost/decimal/detail/locale_conversion.hpp

@@ -15,47 +15,182 @@ namespace boost {
 namespace decimal {
 namespace detail {
 
-inline void convert_string_to_c_locale(char* buffer) noexcept
+// GCC-9 issues an erroneous warning for char == -30 being outside of type limits
+#if defined(__GNUC__) && __GNUC__ >= 9
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Wtype-limits"
+#endif
+
+inline void convert_string_to_c_locale(char* buffer, const std::locale& loc) noexcept
 {
-    const auto locale_decimal_point = *std::localeconv()->decimal_point;
+    const std::numpunct<char>& np = std::use_facet<std::numpunct<char>>(loc);
+
+    const auto locale_decimal_point {np.decimal_point()};
+    auto locale_thousands_sep {np.thousands_sep()};
+    if (locale_thousands_sep == -30)
+    {
+        locale_thousands_sep = ' ';
+    }
+    const bool has_grouping {!np.grouping().empty() && np.grouping()[0] > 0};
+
+    // Remove thousands separator if it exists and grouping is enabled
+    if (has_grouping && locale_thousands_sep != '\0')
+    {
+        // Find the decimal point first to know where the integer part ends
+        const auto decimal_pos {std::strchr(buffer, static_cast<int>(locale_decimal_point))};
+        const auto int_end {decimal_pos ? decimal_pos : (buffer + std::strlen(buffer))};
+
+        // Find the start of the number to include skipping sign
+        auto start {buffer};
+        if (*start == '-' || *start == '+')
+        {
+            ++start;
+        }
+
+        // Only remove thousands separators from the integer part
+        auto read {start};
+        auto write {start};
+
+        while (read < int_end)
+        {
+            const auto ch = *read;
+            if (ch != locale_thousands_sep)
+            {
+                *write++ = *read;
+            }
+            ++read;
+        }
+
+        // Copy the rest of the string (decimal point and fractional part)
+        while (*read != '\0')
+        {
+            *write++ = *read++;
+        }
+        *write = '\0';
+    }
+
     if (locale_decimal_point != '.')
     {
-        auto p = std::strchr(buffer, static_cast<int>(locale_decimal_point));
+        const auto p {std::strchr(buffer, static_cast<int>(locale_decimal_point))};
         if (p != nullptr)
         {
             *p = '.';
         }
     }
 }
 
-inline void convert_string_to_local_locale(char* buffer) noexcept
+inline void convert_string_to_c_locale(char* buffer) noexcept
 {
-    const auto locale_decimal_point = *std::localeconv()->decimal_point;
-    if (locale_decimal_point != '.')
+    convert_string_to_c_locale(buffer, std::locale());
+}
+
+inline int convert_pointer_pair_to_local_locale(char* first, char* last, const std::locale& loc) noexcept
+{
+    const std::numpunct<char>& np = std::use_facet<std::numpunct<char>>(loc);
+
+    const auto locale_decimal_point {np.decimal_point()};
+    auto locale_thousands_sep {np.thousands_sep()};
+    if (locale_thousands_sep == -30)
     {
-        auto p = std::strchr(buffer, static_cast<int>('.'));
-        if (p != nullptr)
+        locale_thousands_sep = ' ';
+    }
+    const bool has_grouping {!np.grouping().empty() && np.grouping()[0] > 0};
+    const int grouping_size {has_grouping ? np.grouping()[0] : 0};
+
+    // Find the start of the number (skip sign if present)
+    char* start = first;
+    if (start < last && (*start == '-' || *start == '+'))
+    {
+        ++start;
+    }
+
+    // Find the actual end of the string
+    auto string_end {start};
+    while (string_end < last && *string_end != '\0')
+    {
+        ++string_end;
+    }
+
+    // Find decimal point position
+    char* decimal_pos {nullptr};
+    for (char* p = start; p < string_end; ++p)
+    {
+        if (*p == '.')
         {
-            *p = locale_decimal_point;
+            decimal_pos = p;
+            *decimal_pos = locale_decimal_point;
+            break;
         }
     }
-}
 
-inline void convert_pointer_pair_to_local_locale(char* first, const char* last) noexcept
-{
-    const auto locale_decimal_point = *std::localeconv()->decimal_point;
-    if (locale_decimal_point != '.')
+    // Determine the end of the integer part
+    const auto int_end {decimal_pos != nullptr ? decimal_pos : string_end};
+    const auto int_digits {static_cast<int>(int_end - start)};
+
+    // Calculate how many separators we need
+    int num_separators {};
+    if (has_grouping && locale_thousands_sep != '\0' && int_digits > 0)
+    {
+        if (int_digits > grouping_size)
+        {
+            num_separators = (int_digits - 1) / grouping_size;
+        }
+    }
+
+    // If we need to add separators, shift content and insert them
+    if (num_separators > 0)
     {
-        while (first != last)
+        const auto original_length {static_cast<int>(string_end - first)};
+        const auto new_length {original_length + num_separators};
+
+        // Check if we have enough space in the buffer
+        if (first + new_length >= last)
         {
-            if (*first == '.')
+            // Not enough space, return error indicator
+            return -1;
+        }
+
+        // Shift everything after the integer part to make room
+        // Work backwards to avoid overwriting
+        auto old_pos {string_end};
+        auto new_pos {first + new_length};
+
+        // Copy from end (including null terminator) back to the end of integer part
+        while (old_pos >= int_end)
+        {
+            *new_pos-- = *old_pos--;
+        }
+
+        // Now insert the integer digits with separators
+        // Count digits from right to left (from decimal point backwards)
+        old_pos = int_end - 1;
+        int digits_from_right {1};
+
+        while (old_pos >= start)
+        {
+            *new_pos-- = *old_pos--;
+
+            // Insert separator after every grouping_size digits from the right
+            // but not after the leftmost digit
+            if (old_pos >= start && digits_from_right % grouping_size == 0)
             {
-                *first = locale_decimal_point;
+                *new_pos-- = locale_thousands_sep;
             }
-
-            ++first;
+            ++digits_from_right;
         }
     }
+
+    return num_separators;
+}
+
+#if defined(__GNUC__) && __GNUC__ == 9
+#  pragma GCC diagnostic pop
+#endif
+
+inline int convert_pointer_pair_to_local_locale(char* first, char* last)
+{
+    const auto loc {std::locale()};
+    return convert_pointer_pair_to_local_locale(first, last, loc);
 }
 
 } //namespace detail

test/Jamfile

@@ -175,6 +175,7 @@ run test_sinh.cpp ;
 run test_snprintf.cpp ;
 run test_sqrt.cpp ;
 run test_string_construction.cpp ;
+run test_string_locale_conversion.cpp ;
 run test_strtod.cpp ;
 run test_tan.cpp ;
 run test_tanh.cpp ;

test/test_decimal32_fast_stream.cpp

@@ -113,6 +113,30 @@ void test_ostream()
 
 #ifndef BOOST_DECIMAL_DISABLE_EXCEPTIONS
 
+void test_issue_1127_locales(const char* locale)
+{
+    try
+    {
+        const std::locale a(locale);
+
+        std::stringstream out_double;
+        out_double.imbue(a);
+        out_double << 1122.89;
+
+        constexpr decimal_fast32_t val4{ 112289, -2 };
+        std::stringstream out_decimal;
+        out_decimal.imbue(a);
+        out_decimal << val4;
+
+        BOOST_TEST_CSTR_EQ(out_decimal.str().c_str(), out_double.str().c_str());
+    }
+    catch (...)
+    {
+        std::cerr << "Locale not installed. Skipping test." << std::endl;
+        return;
+    }
+}
+
 void test_locales()
 {
     const char buffer[] = "1,1897e+02";
@@ -152,7 +176,14 @@ int main()
     test_ostream();
 
     // Homebrew GCC does not support locales
-    #if !(defined(__GNUC__) && __GNUC__ >= 5 && defined(__APPLE__)) && !defined(BOOST_DECIMAL_QEMU_TEST) && !defined(BOOST_DECIMAL_DISABLE_EXCEPTIONS)
+    #if !(defined(__GNUC__) && __GNUC__ >= 8 && defined(__APPLE__)) && !defined(BOOST_DECIMAL_QEMU_TEST) && !defined(BOOST_DECIMAL_DISABLE_EXCEPTIONS)
+    #ifndef _MSC_VER
+    test_issue_1127_locales("en_US.UTF-8"); // . decimal, , thousands
+    test_issue_1127_locales("de_DE.UTF-8"); // , decimal, . thousands
+    #if (defined(__clang__) && __clang_major__ > 9) || (defined(__GNUC__) && __GNUC__ > 9)
+    test_issue_1127_locales("fr_FR.UTF-8"); // , decimal, . thousands
+    #endif
+    #endif
     test_locales();
     #endif