Make Parse{Int,UInt}{32,64} use locale independent std::from_chars(…) (C++17) instead of locale dependent strto{l,ll,ul,ull}.
About std::from_chars: “Unlike other parsing functions in C++ and C libraries, std::from_chars is locale-independent, non-allocating, and non-throwing.”
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
std::fs.
util: Avoid locale dependent functions strtol/strtoll/strtoul/strtoull in ParseInt32/ParseInt64/ParseUInt32/ParseUInt64
fuzz: Assert equivalence between new and old Parse{Int,Uint}{8,32,64} functions
test: Add unit tests for ToIntegral<T>(const std::string&)109+ T result;
110+ const auto [first_nonmatching, error_condition] = std::from_chars(str.data(), str.data() + str.size(), result);
111+ if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) {
112+ return std::nullopt;
113+ }
114+ return {result};
{ are supposed to do? I think they can be removed, as they are confusing as-is. If you want them to check (for whatever reason) that result fits in type T, you will have to type return T{result};
In this specific case return result; should be equivalent to return {result}; since the return type is std::optional<T> and result is of type T. I can see your point about braces looking a bit odd here: feel free to change :)
Some context behind the “braced return” style more generally: it can help avoid nasty implicit narrowing conversions like in the following example.
0$ cat without-list-initialization.cpp
1#include <cstdint>
2
3struct F {
4 F(uint8_t) {}
5};
6
7F f() { return 1024; }
8$ clang++ -c without-list-initialization.cpp
9# No errors
10$ echo $?
110
0$ cat with-list-initialization.cpp
1#include <cstdint>
2
3struct F {
4 F(uint8_t) {}
5};
6
7F f() { return {1024}; }
8$ clang++ -c with-list-initialization.cpp
9with-list-initialization.cpp:7:18: error: constant expression evaluates to 1024 which cannot be narrowed to type 'uint8_t' (aka 'unsigned char') [-Wc++11-narrowing]
10$ echo $?
111
See item (8) and “narrowing conversions” in https://en.cppreference.com/w/cpp/language/list_initialization :)
FWIW also: https://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html
return {baz}; fails when explicit Foo(Baz)
101+ *
102+ * @returns std::nullopt if the entire string could not be parsed, or if the
103+ * parsed value is not in the range representable by the type T.
104+ */
105+template <typename T>
106+std::optional<T> ToIntegral(const std::string& str)
-?[0-9]+?
0 CXX qt/qt_bitcoin_qt-main.o
1In file included from qt/main.cpp:5:
2In file included from ./qt/bitcoin.h:12:
3In file included from ./interfaces/node.h:10:
4In file included from ./net.h:9:
5In file included from ./addrman.h:9:
6In file included from ./netaddress.h:19:
7./util/strencodings.h:16:10: fatal error: 'charconv' file not found
8#include <charconv>
9 ^~~~~~~~~~
101 error generated.
11Makefile:12673: recipe for target 'qt/qt_bitcoin_qt-main.o' failed
12make: *** [qt/qt_bitcoin_qt-main.o] Error 1
04747db876154ddd828c03d9eda10ecf8b25d8dc8 is the first bad commit
1commit 4747db876154ddd828c03d9eda10ecf8b25d8dc8
2Author: practicalswift <practicalswift@users.noreply.github.com>
3Date: Sat Sep 18 04:30:30 2021 +0000
4
5 util: Introduce ToIntegral<T>(const std::string&) for locale independent parsing using std::from_chars(…) (C++17)
6
7 util: Avoid locale dependent functions strtol/strtoll/strtoul/strtoull in ParseInt32/ParseInt64/ParseUInt32/ParseUInt64
8
9 fuzz: Assert equivalence between new and old Parse{Int,Uint}{8,32,64} functions
10
11 test: Add unit tests for ToIntegral<T>(const std::string&)
12
13:040000 040000 ec09e7aa0a2bec6f5021c2bf2994a0fc67e65c1f aa5170295604fa240939c86c02f26a6e436be964 M src
14:040000 040000 aa260b3d4bb2c554e68a7b0baa241a2cfc97818a c5d3df2e7337c8cc6a5a2141844b5c1b81a6ba51 M test
Labels
Utils/log/libs
Milestone
23.0