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 0: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

Contributors
ahook practicalswift sanjaykdragon promag kristapsk laanwj sipa MarcoFalke fanquake

Labels
Refactoring Utils/log/libs