From 87a315b803742e463196db2ba3a965f2af1ebac7 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 18:23:11 +0200 Subject: [PATCH] Stop calling std::localeconv() on every dump() The serializer constructor snapshotted std::localeconv() into a locale_chars member on every dump(), even though the only reader is dump_float(number_float_t, std::false_type)'s snprintf path, taken only for a number_float_t that is neither IEEE single nor double. localeconv() is not required to be thread-safe with setlocale(), so every dump() paid for and raced on a lookup that almost never mattered. Remove locale_chars and the locale member. Right before the thousands-separator/decimal-point fixups in the snprintf path, read std::localeconv() into local thousands_sep/decimal_point variables (null-checked, first byte only, as before) - the same way lexer::get_decimal_point() already does since #5597. Output is unchanged unless the locale changes during a single dump(); in that case the fixups now match what snprintf just produced, instead of a value snapshotted before the call. Overlaps draft PR #5608, which touches the same constructor and dump_float() lines to move this code into a new dump_float_snprintf(); this lands the lookup change now as #5709 asks, and #5608 can do the lookup inside dump_float_snprintf() when it rebases. Verification: the full json_test_data corpus (742 files, dump(), dump(4) and dump(-1,' ',true)) is byte-identical to before the change under the C locale. Added a test pinning the new per-conversion lookup: it switches LC_NUMERIC mid-dump() (via a streambuf that switches on its first write, after the serializer's write buffer has been flushed once but before a later float is converted) and checks the decimal point is still normalized using the locale active at conversion time. On a platform where long double is IEEE-754 double (e.g. 64-bit Arm), dump_float() takes the locale-independent to_chars() path and the test is a no-op there; it is meaningful on a platform where long double is extended precision (most x86 targets). Part of #5709 item 3 Signed-off-by: Niels Lohmann --- include/nlohmann/detail/output/serializer.hpp | 33 +++---- single_include/nlohmann/json.hpp | 33 +++---- tests/src/unit-locale-cpp.cpp | 92 +++++++++++++++++++ 3 files changed, 114 insertions(+), 44 deletions(-) diff --git a/include/nlohmann/detail/output/serializer.hpp b/include/nlohmann/detail/output/serializer.hpp index 45ea5a250..9715a7896 100644 --- a/include/nlohmann/detail/output/serializer.hpp +++ b/include/nlohmann/detail/output/serializer.hpp @@ -81,7 +81,6 @@ class serializer const std::size_t indent_step_ = 0, error_handler_t error_handler_ = error_handler_t::strict) : o(&s) - , locale(std::localeconv()) , indent_char(ichar) , pretty_print(pretty_print_) , ensure_ascii(ensure_ascii_) @@ -1459,21 +1458,28 @@ class serializer // check if the buffer was large enough JSON_ASSERT(static_cast(len) < number_buffer.size()); + // look up the locale's thousands separator and decimal point now, + // matching what snprintf_float() just used (see lexer::get_decimal_point()) + const auto* loc = std::localeconv(); + JSON_ASSERT(loc != nullptr); + const char thousands_sep = (loc->thousands_sep == nullptr) ? '\0' : *loc->thousands_sep; + const char decimal_point = (loc->decimal_point == nullptr) ? '\0' : *loc->decimal_point; + // erase thousands separators - if (locale.thousands_sep != '\0') + if (thousands_sep != '\0') { // NOLINTNEXTLINE(readability-qualified-auto,llvm-qualified-auto): std::remove returns an iterator, see https://github.com/nlohmann/json/issues/3081 - const auto end = std::remove(number_buffer.begin(), number_buffer.begin() + len, locale.thousands_sep); + const auto end = std::remove(number_buffer.begin(), number_buffer.begin() + len, thousands_sep); std::fill(end, number_buffer.end(), '\0'); JSON_ASSERT((end - number_buffer.begin()) <= len); len = (end - number_buffer.begin()); } // convert decimal point to '.' - if (locale.decimal_point != '\0' && locale.decimal_point != '.') + if (decimal_point != '\0' && decimal_point != '.') { // NOLINTNEXTLINE(readability-qualified-auto,llvm-qualified-auto): std::find returns an iterator, see https://github.com/nlohmann/json/issues/3081 - const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), locale.decimal_point); + const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), decimal_point); if (dec_pos != number_buffer.end()) { *dec_pos = '.'; @@ -1523,29 +1529,12 @@ class serializer } private: - /// the locale's thousand separator and decimal point characters - struct locale_chars - { - explicit locale_chars(const std::lconv* loc) noexcept - : thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits::to_char_type(* (loc->thousands_sep))) - , decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits::to_char_type(* (loc->decimal_point))) - {} - - const char thousands_sep; - const char decimal_point; - }; - /// the output of the serializer (non-owning; the adapter lives at the call site) output_adapter_protocol* o = nullptr; /// a (hopefully) large enough character buffer std::array number_buffer{{}}; - /// computed once from std::localeconv() at construction; @ref - /// locale_chars keeps std::localeconv()'s pointer from having to be held - /// past the constructor, while still letting these stay const - const locale_chars locale; - /// string buffer std::array string_buffer{{}}; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 7629385de..263b962c1 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -24163,7 +24163,6 @@ class serializer const std::size_t indent_step_ = 0, error_handler_t error_handler_ = error_handler_t::strict) : o(&s) - , locale(std::localeconv()) , indent_char(ichar) , pretty_print(pretty_print_) , ensure_ascii(ensure_ascii_) @@ -25541,21 +25540,28 @@ class serializer // check if the buffer was large enough JSON_ASSERT(static_cast(len) < number_buffer.size()); + // look up the locale's thousands separator and decimal point now, + // matching what snprintf_float() just used (see lexer::get_decimal_point()) + const auto* loc = std::localeconv(); + JSON_ASSERT(loc != nullptr); + const char thousands_sep = (loc->thousands_sep == nullptr) ? '\0' : *loc->thousands_sep; + const char decimal_point = (loc->decimal_point == nullptr) ? '\0' : *loc->decimal_point; + // erase thousands separators - if (locale.thousands_sep != '\0') + if (thousands_sep != '\0') { // NOLINTNEXTLINE(readability-qualified-auto,llvm-qualified-auto): std::remove returns an iterator, see https://github.com/nlohmann/json/issues/3081 - const auto end = std::remove(number_buffer.begin(), number_buffer.begin() + len, locale.thousands_sep); + const auto end = std::remove(number_buffer.begin(), number_buffer.begin() + len, thousands_sep); std::fill(end, number_buffer.end(), '\0'); JSON_ASSERT((end - number_buffer.begin()) <= len); len = (end - number_buffer.begin()); } // convert decimal point to '.' - if (locale.decimal_point != '\0' && locale.decimal_point != '.') + if (decimal_point != '\0' && decimal_point != '.') { // NOLINTNEXTLINE(readability-qualified-auto,llvm-qualified-auto): std::find returns an iterator, see https://github.com/nlohmann/json/issues/3081 - const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), locale.decimal_point); + const auto dec_pos = std::find(number_buffer.begin(), number_buffer.end(), decimal_point); if (dec_pos != number_buffer.end()) { *dec_pos = '.'; @@ -25605,29 +25611,12 @@ class serializer } private: - /// the locale's thousand separator and decimal point characters - struct locale_chars - { - explicit locale_chars(const std::lconv* loc) noexcept - : thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits::to_char_type(* (loc->thousands_sep))) - , decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits::to_char_type(* (loc->decimal_point))) - {} - - const char thousands_sep; - const char decimal_point; - }; - /// the output of the serializer (non-owning; the adapter lives at the call site) output_adapter_protocol* o = nullptr; /// a (hopefully) large enough character buffer std::array number_buffer{{}}; - /// computed once from std::localeconv() at construction; @ref - /// locale_chars keeps std::localeconv()'s pointer from having to be held - /// past the constructor, while still letting these stay const - const locale_chars locale; - /// string buffer std::array string_buffer{{}}; diff --git a/tests/src/unit-locale-cpp.cpp b/tests/src/unit-locale-cpp.cpp index 14f743a66..1a1f77c5d 100644 --- a/tests/src/unit-locale-cpp.cpp +++ b/tests/src/unit-locale-cpp.cpp @@ -14,7 +14,10 @@ using nlohmann::json; #include #include +#include #include +#include +#include #include #include #include @@ -385,3 +388,92 @@ TEST_CASE("locale with a multi-byte decimal point") CHECK(std::setlocale(LC_NUMERIC, "C") != nullptr); } + +namespace +{ +// a streambuf that switches LC_NUMERIC the first time anything is written to +// it, so a dump() in progress can be made to change locale mid-flight: after +// the serializer was constructed (and, before #5709 item 3, after it had +// cached std::localeconv() for the whole call) but before a later float is +// converted +struct LocaleSwitchingStreambuf final : std::streambuf +{ + explicit LocaleSwitchingStreambuf(const char* switch_to) + : locale_after_first_write(switch_to) + {} + + std::string data {}; // NOLINT(readability-redundant-member-init) + std::string locale_after_first_write; + bool switched = false; + + std::streamsize xsputn(const char* s, std::streamsize n) override + { + if (!switched) + { + switched = std::setlocale(LC_NUMERIC, locale_after_first_write.c_str()) != nullptr; + } + data.append(s, static_cast(n)); + return n; + } +}; +} // namespace + +TEST_CASE("locale changes during a single dump() (#5709 item 3)") +{ + // dump_float() only reads the locale on the snprintf path, taken for a + // number_float_t that is not an IEEE-754 single or double, i.e. not + // (is_iec559 && digits == 24 && max_exponent == 128) and not (is_iec559 + // && digits == 53 && max_exponent == 1024) - see dump_float(). Checking + // is_iec559 alone is not enough: on x86_64, long double is a 64-bit + // (80-bit extended) format for which is_iec559 is also true, so it still + // takes the snprintf path this test means to exercise. Only a + // number_float_t whose digits/max_exponent match float or double (e.g. + // long double on 64-bit Arm, where it is IEEE-754 double) takes the + // locale-independent to_chars() path instead, and this test is a no-op + // there. + using long_double_json = nlohmann::basic_json; + using ld_limits = std::numeric_limits; + const bool is_ieee_single_or_double = + (ld_limits::is_iec559 && ld_limits::digits == 24 && ld_limits::max_exponent == 128) || + (ld_limits::is_iec559 && ld_limits::digits == 53 && ld_limits::max_exponent == 1024); + if (is_ieee_single_or_double) + { + MESSAGE("long double is IEEE-754 single or double on this platform; dump_float()'s snprintf/locale path is not exercised here"); + } + + const char* de_DE_name = "de_DE.UTF-8"; + if (std::setlocale(LC_NUMERIC, de_DE_name) == nullptr) + { + de_DE_name = "de_DE"; + if (std::setlocale(LC_NUMERIC, de_DE_name) == nullptr) + { + MESSAGE("locale de_DE is not usable"); + return; + } + } + const std::string decimal_point = std::localeconv()->decimal_point; + REQUIRE(std::setlocale(LC_NUMERIC, "C") != nullptr); + if (decimal_point != ",") + { + MESSAGE("de_DE's decimal point is not ',' on this platform, skipping"); + return; + } + + // a string long enough to overflow the serializer's internal write + // buffer, so that it is flushed to the output adapter - and the locale + // switched - before the number after it is converted + const std::string padding(5000, 'a'); + const long_double_json j = { padding, 1234.5L }; + + LocaleSwitchingStreambuf buf(de_DE_name); + std::ostream os(&buf); + os << j; + CHECK(std::setlocale(LC_NUMERIC, "C") != nullptr); + + REQUIRE(buf.switched); + // whatever locale was in effect when the float was actually converted, + // the output is normalized to use '.' as the decimal point: it must be + // looked up at conversion time, not once for the whole dump() - the same + // fix #5597 made on the parser side + CHECK(buf.data == "[\"" + padding + "\",1234.5]"); +}