Skip to content

Commit d4decd0

Browse files
mborlandmborland
authored
Merge pull request #1138 from cppalliance/1127
Improve handling of locales
2 parents 8102c85 + dcb1ef8 commit d4decd0

File tree

7 files changed

+290
-27
lines changed

7 files changed

+290
-27
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,8 @@ jobs:
389389
sudo apt-get -o Acquire::Retries=$NET_RETRY_COUNT update
390390
sudo apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y ${{join(matrix.install, ' ')}} locales libfmt-dev
391391
sudo locale-gen de_DE.UTF-8
392+
sudo locale-gen en_US.UTF-8
393+
sudo locale-gen fr_FR.UTF-8
392394
sudo update-locale
393395
- name: Setup GCC Toolchain
394396
if: matrix.gcc_toolchain

‎include/boost/decimal/cstdio.hpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,10 @@ inline auto snprintf_impl(char* buffer, const std::size_t buf_size, const char*
227227
{
228228
detail::make_uppercase(buffer, r.ptr);
229229
}
230-
convert_pointer_pair_to_local_locale(buffer, r.ptr);
230+
*r.ptr = '\0';
231+
const auto offset {convert_pointer_pair_to_local_locale(buffer, buffer + buf_size - byte_count)};
231232

232-
buffer = r.ptr;
233+
buffer = r.ptr + (offset == -1 ? 0 : offset);
233234

234235
if (value_iter != values_list.end())
235236
{
@@ -276,7 +277,7 @@ inline auto fprintf(std::FILE* buffer, const char* format, const T... values) no
276277
int bytes {};
277278
char char_buffer[1024];
278279

279-
if (format_len + value_space <= 1024U)
280+
if (format_len + value_space <= ((1024 * 2) / 3))
280281
{
281282
bytes = detail::snprintf_impl(char_buffer, sizeof(char_buffer), format, values...);
282283
if (bytes)
@@ -287,7 +288,8 @@ inline auto fprintf(std::FILE* buffer, const char* format, const T... values) no
287288
else
288289
{
289290
// LCOV_EXCL_START
290-
std::unique_ptr<char[]> longer_char_buffer(new(std::nothrow) char[format_len + value_space + 1]);
291+
// Add 50% overage in case we need to do locale conversion
292+
std::unique_ptr<char[]> longer_char_buffer(new(std::nothrow) char[(3 * (format_len + value_space + 1)) / 2]);
291293
if (longer_char_buffer == nullptr)
292294
{
293295
errno = ENOMEM;

‎include/boost/decimal/detail/io.hpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ auto operator>>(std::basic_istream<charT, traits>& is, DecimalType& d)
7979
std::memcpy(buffer, t_buffer.c_str(), t_buffer.size());
8080
}
8181

82-
detail::convert_string_to_c_locale(buffer);
82+
detail::convert_string_to_c_locale(buffer, is.getloc());
8383

8484
auto fmt {chars_format::general};
8585
const auto flags {is.flags()};
@@ -170,8 +170,7 @@ auto operator<<(std::basic_ostream<charT, traits>& os, const DecimalType& d)
170170
}
171171

172172
*r.ptr = '\0';
173-
174-
detail::convert_string_to_local_locale(buffer);
173+
detail::convert_pointer_pair_to_local_locale(buffer, buffer + sizeof(buffer), os.getloc());
175174

176175
BOOST_DECIMAL_IF_CONSTEXPR (!std::is_same<charT, char>::value)
177176
{

‎include/boost/decimal/detail/locale_conversion.hpp‎

Lines changed: 154 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,47 +15,182 @@ namespace boost {
1515
namespace decimal {
1616
namespace detail {
1717

18-
inline void convert_string_to_c_locale(char* buffer) noexcept
18+
// GCC-9 issues an erroneous warning for char == -30 being outside of type limits
19+
#if defined(__GNUC__) && __GNUC__ >= 9
20+
# pragma GCC diagnostic push
21+
# pragma GCC diagnostic ignored "-Wtype-limits"
22+
#endif
23+
24+
inline void convert_string_to_c_locale(char* buffer, const std::locale& loc) noexcept
1925
{
20-
const auto locale_decimal_point = *std::localeconv()->decimal_point;
26+
const std::numpunct<char>& np = std::use_facet<std::numpunct<char>>(loc);
27+
28+
const auto locale_decimal_point {np.decimal_point()};
29+
auto locale_thousands_sep {np.thousands_sep()};
30+
if (locale_thousands_sep == -30)
31+
{
32+
locale_thousands_sep = ' ';
33+
}
34+
const bool has_grouping {!np.grouping().empty() && np.grouping()[0] > 0};
35+
36+
// Remove thousands separator if it exists and grouping is enabled
37+
if (has_grouping && locale_thousands_sep != '\0')
38+
{
39+
// Find the decimal point first to know where the integer part ends
40+
const auto decimal_pos {std::strchr(buffer, static_cast<int>(locale_decimal_point))};
41+
const auto int_end {decimal_pos ? decimal_pos : (buffer + std::strlen(buffer))};
42+
43+
// Find the start of the number to include skipping sign
44+
auto start {buffer};
45+
if (*start == '-' || *start == '+')
46+
{
47+
++start;
48+
}
49+
50+
// Only remove thousands separators from the integer part
51+
auto read {start};
52+
auto write {start};
53+
54+
while (read < int_end)
55+
{
56+
const auto ch = *read;
57+
if (ch != locale_thousands_sep)
58+
{
59+
*write++ = *read;
60+
}
61+
++read;
62+
}
63+
64+
// Copy the rest of the string (decimal point and fractional part)
65+
while (*read != '\0')
66+
{
67+
*write++ = *read++;
68+
}
69+
*write = '\0';
70+
}
71+
2172
if (locale_decimal_point != '.')
2273
{
23-
auto p = std::strchr(buffer, static_cast<int>(locale_decimal_point));
74+
const auto p {std::strchr(buffer, static_cast<int>(locale_decimal_point))};
2475
if (p != nullptr)
2576
{
2677
*p = '.';
2778
}
2879
}
2980
}
3081

31-
inline void convert_string_to_local_locale(char* buffer) noexcept
82+
inline void convert_string_to_c_locale(char* buffer) noexcept
3283
{
33-
const auto locale_decimal_point = *std::localeconv()->decimal_point;
34-
if (locale_decimal_point != '.')
84+
convert_string_to_c_locale(buffer, std::locale());
85+
}
86+
87+
inline int convert_pointer_pair_to_local_locale(char* first, char* last, const std::locale& loc) noexcept
88+
{
89+
const std::numpunct<char>& np = std::use_facet<std::numpunct<char>>(loc);
90+
91+
const auto locale_decimal_point {np.decimal_point()};
92+
auto locale_thousands_sep {np.thousands_sep()};
93+
if (locale_thousands_sep == -30)
3594
{
36-
auto p = std::strchr(buffer, static_cast<int>('.'));
37-
if (p != nullptr)
95+
locale_thousands_sep = ' ';
96+
}
97+
const bool has_grouping {!np.grouping().empty() && np.grouping()[0] > 0};
98+
const int grouping_size {has_grouping ? np.grouping()[0] : 0};
99+
100+
// Find the start of the number (skip sign if present)
101+
char* start = first;
102+
if (start < last && (*start == '-' || *start == '+'))
103+
{
104+
++start;
105+
}
106+
107+
// Find the actual end of the string
108+
auto string_end {start};
109+
while (string_end < last && *string_end != '\0')
110+
{
111+
++string_end;
112+
}
113+
114+
// Find decimal point position
115+
char* decimal_pos {nullptr};
116+
for (char* p = start; p < string_end; ++p)
117+
{
118+
if (*p == '.')
38119
{
39-
*p = locale_decimal_point;
120+
decimal_pos = p;
121+
*decimal_pos = locale_decimal_point;
122+
break;
40123
}
41124
}
42-
}
43125

44-
inline void convert_pointer_pair_to_local_locale(char* first, const char* last) noexcept
45-
{
46-
const auto locale_decimal_point = *std::localeconv()->decimal_point;
47-
if (locale_decimal_point != '.')
126+
// Determine the end of the integer part
127+
const auto int_end {decimal_pos != nullptr ? decimal_pos : string_end};
128+
const auto int_digits {static_cast<int>(int_end - start)};
129+
130+
// Calculate how many separators we need
131+
int num_separators {};
132+
if (has_grouping && locale_thousands_sep != '\0' && int_digits > 0)
133+
{
134+
if (int_digits > grouping_size)
135+
{
136+
num_separators = (int_digits - 1) / grouping_size;
137+
}
138+
}
139+
140+
// If we need to add separators, shift content and insert them
141+
if (num_separators > 0)
48142
{
49-
while (first != last)
143+
const auto original_length {static_cast<int>(string_end - first)};
144+
const auto new_length {original_length + num_separators};
145+
146+
// Check if we have enough space in the buffer
147+
if (first + new_length >= last)
50148
{
51-
if (*first == '.')
149+
// Not enough space, return error indicator
150+
return -1;
151+
}
152+
153+
// Shift everything after the integer part to make room
154+
// Work backwards to avoid overwriting
155+
auto old_pos {string_end};
156+
auto new_pos {first + new_length};
157+
158+
// Copy from end (including null terminator) back to the end of integer part
159+
while (old_pos >= int_end)
160+
{
161+
*new_pos-- = *old_pos--;
162+
}
163+
164+
// Now insert the integer digits with separators
165+
// Count digits from right to left (from decimal point backwards)
166+
old_pos = int_end - 1;
167+
int digits_from_right {1};
168+
169+
while (old_pos >= start)
170+
{
171+
*new_pos-- = *old_pos--;
172+
173+
// Insert separator after every grouping_size digits from the right
174+
// but not after the leftmost digit
175+
if (old_pos >= start && digits_from_right % grouping_size == 0)
52176
{
53-
*first = locale_decimal_point;
177+
*new_pos-- = locale_thousands_sep;
54178
}
55-
56-
++first;
179+
++digits_from_right;
57180
}
58181
}
182+
183+
return num_separators;
184+
}
185+
186+
#if defined(__GNUC__) && __GNUC__ == 9
187+
# pragma GCC diagnostic pop
188+
#endif
189+
190+
inline int convert_pointer_pair_to_local_locale(char* first, char* last)
191+
{
192+
const auto loc {std::locale()};
193+
return convert_pointer_pair_to_local_locale(first, last, loc);
59194
}
60195

61196
} //namespace detail

‎test/Jamfile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ run test_sinh.cpp ;
175175
run test_snprintf.cpp ;
176176
run test_sqrt.cpp ;
177177
run test_string_construction.cpp ;
178+
run test_string_locale_conversion.cpp ;
178179
run test_strtod.cpp ;
179180
run test_tan.cpp ;
180181
run test_tanh.cpp ;

‎test/test_decimal32_fast_stream.cpp‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,30 @@ void test_ostream()
113113

114114
#ifndef BOOST_DECIMAL_DISABLE_EXCEPTIONS
115115

116+
void test_issue_1127_locales(const char* locale)
117+
{
118+
try
119+
{
120+
const std::locale a(locale);
121+
122+
std::stringstream out_double;
123+
out_double.imbue(a);
124+
out_double << 1122.89;
125+
126+
constexpr decimal_fast32_t val4{ 112289, -2 };
127+
std::stringstream out_decimal;
128+
out_decimal.imbue(a);
129+
out_decimal << val4;
130+
131+
BOOST_TEST_CSTR_EQ(out_decimal.str().c_str(), out_double.str().c_str());
132+
}
133+
catch (...)
134+
{
135+
std::cerr << "Locale not installed. Skipping test." << std::endl;
136+
return;
137+
}
138+
}
139+
116140
void test_locales()
117141
{
118142
const char buffer[] = "1,1897e+02";
@@ -152,7 +176,14 @@ int main()
152176
test_ostream();
153177

154178
// Homebrew GCC does not support locales
155-
#if !(defined(__GNUC__) && __GNUC__ >= 5 && defined(__APPLE__)) && !defined(BOOST_DECIMAL_QEMU_TEST) && !defined(BOOST_DECIMAL_DISABLE_EXCEPTIONS)
179+
#if !(defined(__GNUC__) && __GNUC__ >= 8 && defined(__APPLE__)) && !defined(BOOST_DECIMAL_QEMU_TEST) && !defined(BOOST_DECIMAL_DISABLE_EXCEPTIONS)
180+
#ifndef _MSC_VER
181+
test_issue_1127_locales("en_US.UTF-8"); // . decimal, , thousands
182+
test_issue_1127_locales("de_DE.UTF-8"); // , decimal, . thousands
183+
#if (defined(__clang__) && __clang_major__ > 9) || (defined(__GNUC__) && __GNUC__ > 9)
184+
test_issue_1127_locales("fr_FR.UTF-8"); // , decimal, . thousands
185+
#endif
186+
#endif
156187
test_locales();
157188
#endif
158189

0 commit comments

Comments
 (0)