Limited to integral types to avoid inconsistencies noted in the examples here: https://en.cppreference.com/w/cpp/string/basic_string/to_string

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• qt: Kill the locale dependency bug class by not allowing Qt to mess with LC_NUMERIC #18147 (qt: Kill the locale dependency bug class by not allowing Qt to mess with LC_NUMERIC by practicalswift)
• Replace uses of boost::trim* with locale-independent alternatives #18130 (Replace uses of boost::trim* with locale-independent alternatives by Empact)
• net: Add NAT-PMP port forwarding support #18077 (net: Add NAT-PMP port forwarding support by hebasto)
• wallet: Keep inactive seeds after sethdseed and derive keys from them as needed #17681 (wallet: Keep inactive seeds after sethdseed and derive keys from them as needed by achow101)
• More thread safety annotation coverage #16127 (More thread safety annotation coverage by ajtowns)
• Add fee_est tool for debugging fee estimation code #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Concept ACK: simple and robust (locale independent) is better than gotcha-filled and fragile (locale dependent).

Thanks for working on reducing the usage of locale dependent functions. Hope to see more PR's of this type from you :)

Will review.

Concept ACK.

@Empact Are you still working on this PR? :)

I think it would be really good to have ToString(…) in our code base since std::to_string(…) usage keeps popping up in new code. Latest case in #18261 (see #18261 (comment)).

Rebased and incorporated your feedback, @practicalswift

ACK 97eefd7c25f03e898071171eacc45ac5fb5ada6b

Rebased

ACK 0add74b0b5e11124612e6d07b3f68266a4ace66e

Addressed nits.

ACK d056df0

Very pleased to see the removal of no less than 16(!) instances of calls to locale dependent functions. Thanks! :)

ACK d056df0

So for tinyformat, do we want to do this as well?

 // Added for Bitcoin Core
 template<typename... Args>
 std::string format(const std::string &fmt, const Args&... args)
 {
     std::ostringstream oss;
+    oss.imbue(std::locale::classic());
     format(oss, fmt.c_str(), args...);
     return oss.str();
 }

@laanwj Technically we don't technically that in the tfm::format case since we're already implicitly assuming that the C++ locale is always the classic locale. If that was not the case a lot of things would break as demonstrated in #18281. However I think we should make that implicit assumption explicit - PR coming up! :)