There are no plans to switch to C++23 anytime soon in the next couple of years. The only place right now that is known to benefit is src/compat/byteswap.h.
However, it is still useful to test with the option, because deprecated, removed or changed language features, as well as compiler changes that are guarded by the language version will be tested and developers can learn about them upfront.
Also includes a minor doc fixup commit.
To match the format of the previous section about the Ubuntu 22.04 EOL.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | TheCharlatan, davidgumberg |
| Concept ACK | theuni, jonatack |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
core_interface as a property and remove the global? Then the user can just pick their own standard with -DCMAKE_CXX_STANDARD on the command line. Since it a global set, the user choice is currently overridden.
-DCMAKE_CXX_STANDARD=23 at configuration time.
Does this work for you?
0diff --git a/CMakeLists.txt b/CMakeLists.txt
1index 32db4189d4..f292830798 100644
2--- a/CMakeLists.txt
3+++ b/CMakeLists.txt
4@@ -74 +73,0 @@ enable_language(CXX)
5-set(CMAKE_CXX_STANDARD 20)
6@@ -563,0 +563 @@ target_compile_definitions(core_interface_debug INTERFACE ${DEPENDS_COMPILE_DEFI
7+target_compile_features(core_interface INTERFACE cxx_std_20)
Maybe @hebasto has a suggestion for also printing it in the summary.
Looks like this also runs into: bitcoin-core/leveldb-subtree#43.
Force pushed trivial CI fixup to temporarily work around the issue for now.
Users shouldn’t be able to modify this at all,
In general this is impossible, because they can always just override the compilation flags.
Users shouldn’t be able to modify this at all,
In general this is impossible, because they can always just override the compilation flags.
Are you sure. Locally when using -DCMAKE_CXX_FLAGS='-std=c++17', it doesn’t have any effect, because they are reset by cmake:
0C++ compiler flags .................... -std=c++17 -O2 -g -std=c++20 -fPIC ...
I guess I can use the existing non-standard append hack instead, which users shouldn’t know about to shoot themselves into the foot.
I guess I can use the existing non-standard append hack instead
Force pushed that instead, so that this is a doc/ci-only change.
Other changes can be done in a follow-up, if needed.
it doesn’t have any effect, because they are reset by cmake:
Heh, right, I forgot CMake makes a mess of this. Note that this behaviour is itself a footgun, because a user might rightly expect something like -DCMAKE_CXX_FLAGS='-O1' to work, but CMake will just ignore it.
Fwiw APPEND_CXXFLAGS was added specifically for cases like this where CMake sets a flag internally that we want to override.
Edit: Nm, I see you’re using that now :)
Concept ACK.
If we wanted, we could actually start conditionally enabling c++23 features for these builds. For byteswap, something like:
0#if __cplusplus >= 202302L && defined(__cpp_lib_byteswap) && __cpp_lib_byteswap >= 202110L
1# define bitcoin_builtin_bswap16(x) std::byteswap(x)
2# define bitcoin_builtin_bswap32(x) std::byteswap(x)
3# define bitcoin_builtin_bswap64(x) std::byteswap(x)
4# elif defined __has_builtin
5...
Another I’ve found useful while working on another project is static operator(), which could be gated/used like:
0#if __cplusplus >= 202302L && defined(__cpp_static_call_operator) && __cpp_static_call_operator >= 202207L
1#define CPP23_STATIC static
2#define CONST_IF_NOT_CPP23_STATIC
3#else
4#define CPP23_STATIC
5#define CONST_IF_NOT_CPP23_STATIC const
6#endif
0class CScriptVisitor
1{
2public:
3 CPP23_STATIC CScript operator()(const CNoDestination& dest) CONST_IF_NOT_CPP23_STATIC
4 {
5 return dest.GetScript();
6 }
7...
27@@ -28,7 +28,8 @@ export BITCOIN_CONFIG="\
28 -DCMAKE_C_COMPILER=clang-18 \
29 -DCMAKE_CXX_COMPILER=clang++-18 \
30 -DCMAKE_C_FLAGS='-ftrivial-auto-var-init=pattern' \
31- -DCMAKE_CXX_FLAGS='-ftrivial-auto-var-init=pattern' \
32+ -DCMAKE_CXX_FLAGS='-ftrivial-auto-var-init=pattern -Wno-error=deprecated-declarations' \
A note for other readers:
Wno-error=deprecated-declarations is here because the leveldb subtree currently uses std::aligned_storage. (deprecated in C++23)
Patches introducing features deprecated in C++20 or earlier will still be caught by other CI tasks.
ACK https://github.com/bitcoin/bitcoin/pull/30735/commits/fac587ea070fe1354708aacce33ebb9cebd35e5b
tested with:
0cmake -B build -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CC_COMPILER=clang -DAPPEND_CXXFLAGS='-std=c++23'