This is for CI and devs only and doesn’t change that C++17 is the standard we are currently using. The option --enable-c++20 allows CI to check that the C++17 code in the repo is also valid C++20. (There are some cases where valid C++17 doesn’t compile under C++20).
Also, it allows developers to easily play with C++20 in the codebase.
You may or may not be interested in one of my PRs to dash that did this https://github.com/dashpay/dash/pull/4600
I was planning on doing the equivalent here, but didn’t want to until we could merge #23340 which if I recall correctly is required to fix cxx20 builds
ax_cxx_compile_stdcxx.m4 macro upstream: https://github.com/autoconf-archive/autoconf-archive/pull/244.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
50@@ -51,12 +51,26 @@ class path : public std::filesystem::path
51 // Disallow std::string conversion method to avoid locale-dependent encoding on windows.
52 std::string string() const = delete;
53
54+ std::string u8string() const
Code review ACK fa442133f1ea37edb7c769590da9592b541fce6c. All these changes look safe to me, and it is definitely better if the code is tested with c++20 and compiles without changes.
A lot of changes are made in the PR without an explanation about the reason, though. Like the enum-enum warning change and scheduler lambda change. Ideally the changes would be broken up more, with one change per commit, and the commit messages would explain the reasons behind the changes, or at least give hints.
Concept and code-review ACK fab63ca14298d37022e23d3e9747bc4bf94121c9 :two: :zero:
Tested --enable-c++20 compilation with clang 11.1.0 (OpenBSD 7.0).
-Wno commit.
ACK fab63ca14298d37022e23d3e9747bc4bf94121c9
I’d say the two pull requests are mostly orthogonal. If one is merged before the other, the one merged second can drop the temporary
-Wnocommit.
Agree.
~re-ACK fa6bc3b2ccb7386ed496dd8be6b748e95c28b957~
CI failure?
Without the changes, g++ will warn to compile under C++20:
scheduler.cpp:114:21: warning: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Wdeprecated]
114 | scheduleFromNow([=] { Repeat(*this, f, delta); }, delta);
| ^
scheduler.cpp:114:21: note: add explicit ‘this’ or ‘*this’ capture
Without the changes, the file will fail to compile under C++20 because
char8_t can not be converted to char implicitly.
This makes code that uses the helper less verbose.
Moreover, this makes net_processing C++20 compliant. Otherwise, it would
lead to a compile error (see below). C++20 disables aggregate
initialization when any constructor is declared. See
http://open-std.org/JTC1/SC22/WG21/docs/papers/2018/p1008r1.pdf
net_processing.cpp:1627:42: error: no matching constructor for initialization of 'CSerializedNetMsg'
m_connman.PushMessage(pnode, CSerializedNetMsg{ser_cmpctblock.data, ser_cmpctblock.m_type});
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~