util: Replace itostr/i64tostr with c++11 to_string #17808

pull ahook wants to merge 1 commits into bitcoin:master from ahook:akh_to_string changing 6 files +5 −19

ahook commented at 9:13 PM on December 27, 2019: none

C++11 introduced to_string methods for a variety of POD types, including int and int64_t. These are already used in a few places in the repo. This patch removes the ad-hoc functions in strencodings in favor of the new std versions.
util: Replace itostr/i64tostr with c++11 to_string fcb76edd9e
fanquake added the label Utils/log/libs on Dec 27, 2019
practicalswift commented at 9:23 PM on December 27, 2019: contributor

ACK fcb76edd9e2bbf788218ec5d88499e1464ebe9dc: nice to get rid of two functions and 14 LOC Invalidating due to locale dependence.

Good catch and nice first-time contribution @ahook. Hope to see more contributions from you! :)
MarcoFalke added the label Refactoring on Dec 27, 2019
sanjaykdragon commented at 10:28 PM on December 27, 2019: contributor

ACK fcb76ed: We should be modernizing our code, I would say moving to to_string is a smart idea.
promag commented at 11:00 PM on December 27, 2019: member

ACK fcb76edd9e2bbf788218ec5d88499e1464ebe9dc.
kristapsk approved
kristapsk commented at 12:46 AM on December 28, 2019: contributor

ACK fcb76edd9e2bbf788218ec5d88499e1464ebe9dc
laanwj commented at 8:13 PM on January 2, 2020: member

Concept ACK, there's one thing I have to ask though: is the output of std::to_string locale dependent in any way?
sipa commented at 8:21 PM on January 2, 2020: member

It looks like std::to_string is locale-dependent.

Reasoning: https://en.cppreference.com/w/cpp/string/basic_string/to_string claims it is equivalent to std::sprintf with the appropriate formatting string. std:sprintf behaves identical to C sprintf, which is locale-dependent.
practicalswift commented at 8:49 PM on January 2, 2020: contributor

Yes, unfortunately it seems std::to_string is locale dependent:

std::to_string relies on the current locale for formatting purposes, […]. C++17 provides std::to_chars as a higher-performance locale-independent alternative.

:(
ahook commented at 9:03 PM on January 2, 2020: none

Ah nuts. Guess that's a dealbreaker? I'll close this out then.
MarcoFalke commented at 10:14 PM on January 2, 2020: member

We might switch to C++17, so maybe revisit then?

#16684 (comment)
MarcoFalke referenced this in commit 816464198c on Jan 3, 2020
sidhujag referenced this in commit c053415b7c on Jan 4, 2020
fanquake commented at 5:46 AM on January 5, 2020: member

Closing as something we can revisit in the future. Commenters here can follow up with std::to_string usage in #17866.
fanquake closed this on Jan 5, 2020
sidhujag referenced this in commit a15cf46f3b on Nov 10, 2020
DrahtBot locked this on Feb 15, 2022