Skip to content

Commit

Permalink
Merge bitcoin#28349: build: Require C++20 compiler
Browse files Browse the repository at this point in the history
fa6e50d fuzz: Use C++20 starts_with in rpc.cpp (MarcoFalke)
faa4838 Revert "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings" (MarcoFalke)
fae3b77 refactor: Drop unused _Pragma to ignore -Wgnu-zero-variadic-macro-arguments (MarcoFalke)
fa02fc0 refactor: modernize-use-default-member-init for bit-fields (C++20) (MarcoFalke)
fa67f09 build: Require C++20 compiler (MarcoFalke)

Pull request description:

  C++20 allows to write safer code, because it allows to enforce more stuff at compile time (`constinit`, `conteval`, `constexpr`, `std::span`, ...).

  Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts).

  See bitcoin#23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20

  With g++-10 (bitcoin#28348) and clang-13 (bitcoin#28210), there is broad support for almost all features of C++20.

  It should be fine to require a C++20 compiler for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

  This pull request includes three small cleanups to make use of C++20 features. If any issues are detected before or after merge, this should be easy to revert. If no issues arise, it should be fine to make use of more involved C++20 features later on.

ACKs for top commit:
  fanquake:
    ACK fa6e50d

Tree-SHA512: 244d79bfb0b750a4bdd713f40573b9ca33816fb84b6c84a58f027b9d7d4bb0cc4f18642959e4cf3d094808a69e5b8a327ca8521d7c0c08af27dacb5da3e78e71
  • Loading branch information
fanquake committed Dec 8, 2023
2 parents 03042fb + fa6e50d commit 3e69125
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 51 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ jobs:
echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD ^$(git rev-list -n1 --merges HEAD)^@ | head -1)" >> "$GITHUB_ENV"
- run: |
sudo apt-get update
sudo apt-get install clang ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
sudo apt-get install clang-15 ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
- name: Compile and run tests
run: |
# Run tests on commits after the last merge commit and before the PR head commit
# Use clang++, because it is a bit faster and uses less memory than g++
git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang CXX=clang++ ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang-15 CXX=clang++-15 ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
macos-native-x86_64:
name: 'macOS 13 native, x86_64, no depends, sqlite only, gui'
Expand Down
2 changes: 1 addition & 1 deletion build-aux/m4/ax_cxx_compile_stdcxx.m4
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ m4_define([_AX_CXX_COMPILE_STDCXX_testbody_new_in_20], [[
#error "This is not a C++ compiler"
#elif __cplusplus < 202002L
#elif __cplusplus < 201709L // Temporary patch on top of upstream to allow g++-10
#error "This is not a C++20 compiler"
Expand Down
12 changes: 1 addition & 11 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,8 @@ case $host in
;;
esac

AC_ARG_ENABLE([c++20],
[AS_HELP_STRING([--enable-c++20],
[enable compilation in c++20 mode (disabled by default)])],
[use_cxx20=$enableval],
[use_cxx20=no])

dnl Require C++17 compiler (no GNU extensions)
if test "$use_cxx20" = "no"; then
AX_CXX_COMPILE_STDCXX([17], [noext], [mandatory])
else
dnl Require C++20 compiler (no GNU extensions)
AX_CXX_COMPILE_STDCXX([20], [noext], [mandatory])
fi

dnl Unless the user specified OBJCXX, force it to be the same as CXX. This ensures
dnl that we get the same -std flags for both.
Expand Down
2 changes: 1 addition & 1 deletion depends/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ NO_HARDEN ?=
FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources

C_STANDARD ?= c11
CXX_STANDARD ?= c++17
CXX_STANDARD ?= c++20

BUILD = $(shell ./config.guess)
HOST ?= $(BUILD)
Expand Down
2 changes: 1 addition & 1 deletion depends/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ The following can be set when running make: `make FOO=bar`
- `SDK_PATH`: Path where SDKs can be found (used by macOS)
- `FALLBACK_DOWNLOAD_PATH`: If a source file can't be fetched, try here before giving up
- `C_STANDARD`: Set the C standard version used. Defaults to `c11`.
- `CXX_STANDARD`: Set the C++ standard version used. Defaults to `c++17`.
- `CXX_STANDARD`: Set the C++ standard version used. Defaults to `c++20`.
- `NO_BOOST`: Don't download/build/cache Boost
- `NO_LIBEVENT`: Don't download/build/cache Libevent
- `NO_QT`: Don't download/build/cache Qt and its dependencies
Expand Down
2 changes: 1 addition & 1 deletion depends/packages/qt.mk
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ $(package)_config_opts_release += -silent
$(package)_config_opts_debug = -debug
$(package)_config_opts_debug += -optimized-tools
$(package)_config_opts += -bindir $(build_prefix)/bin
$(package)_config_opts += -c++std c++17
$(package)_config_opts += -c++std c++2a
$(package)_config_opts += -confirm-license
$(package)_config_opts += -hostprefix $(build_prefix)
$(package)_config_opts += -no-compile-examples
Expand Down
2 changes: 1 addition & 1 deletion src/.clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,5 @@ SpacesInAngles: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
Standard: c++17
Standard: c++20
UseTab: Never
4 changes: 0 additions & 4 deletions src/test/fuzz/fuzz.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@ struct FuzzTargetOptions {

void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, FuzzTargetOptions opts);

#if defined(__clang__)
#define FUZZ_TARGET(...) _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wgnu-zero-variadic-macro-arguments\"") DETAIL_FUZZ(__VA_ARGS__) _Pragma("clang diagnostic pop")
#else
#define FUZZ_TARGET(...) DETAIL_FUZZ(__VA_ARGS__)
#endif

#define DETAIL_FUZZ(name, ...) \
void name##_fuzz_target(FuzzBufferType); \
Expand Down
4 changes: 1 addition & 3 deletions src/test/fuzz/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,7 @@ FUZZ_TARGET(rpc, .init = initialize_rpc)
rpc_testing_setup->CallRPC(rpc_command, arguments);
} catch (const UniValue& json_rpc_error) {
const std::string error_msg{json_rpc_error.find_value("message").get_str()};
// Once c++20 is allowed, starts_with can be used.
// if (error_msg.starts_with("Internal bug detected")) {
if (0 == error_msg.rfind("Internal bug detected", 0)) {
if (error_msg.starts_with("Internal bug detected")) {
// Only allow the intentional internal bug
assert(error_msg.find("trigger_internal_bug") != std::string::npos);
}
Expand Down
8 changes: 4 additions & 4 deletions src/txrequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ struct Announcement {
const bool m_is_wtxid : 1;

/** What state this announcement is in. */
State m_state : 3;
State m_state : 3 {State::CANDIDATE_DELAYED};
State GetState() const { return m_state; }
void SetState(State state) { m_state = state; }

Expand All @@ -97,9 +97,9 @@ struct Announcement {

/** Construct a new announcement from scratch, initially in CANDIDATE_DELAYED state. */
Announcement(const GenTxid& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime,
SequenceNumber sequence) :
m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred),
m_is_wtxid{gtxid.IsWtxid()}, m_state{State::CANDIDATE_DELAYED} {}
SequenceNumber sequence)
: m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred),
m_is_wtxid{gtxid.IsWtxid()} {}
};

//! Type alias for priorities.
Expand Down
35 changes: 13 additions & 22 deletions src/util/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,19 @@

#include <sys/sdt.h>

// Disabling this warning can be removed once we switch to C++20
#if defined(__clang__) && __cplusplus < 202002L
#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wgnu-zero-variadic-macro-arguments\"")
#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP _Pragma("clang diagnostic pop")
#else
#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH
#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#endif

#define TRACE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP
#define TRACE(context, event) DTRACE_PROBE(context, event)
#define TRACE1(context, event, a) DTRACE_PROBE1(context, event, a)
#define TRACE2(context, event, a, b) DTRACE_PROBE2(context, event, a, b)
#define TRACE3(context, event, a, b, c) DTRACE_PROBE3(context, event, a, b, c)
#define TRACE4(context, event, a, b, c, d) DTRACE_PROBE4(context, event, a, b, c, d)
#define TRACE5(context, event, a, b, c, d, e) DTRACE_PROBE5(context, event, a, b, c, d, e)
#define TRACE6(context, event, a, b, c, d, e, f) DTRACE_PROBE6(context, event, a, b, c, d, e, f)
#define TRACE7(context, event, a, b, c, d, e, f, g) DTRACE_PROBE7(context, event, a, b, c, d, e, f, g)
#define TRACE8(context, event, a, b, c, d, e, f, g, h) DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h)
#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i)
#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j)
#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k)
#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l)

#else

Expand Down

0 comments on commit 3e69125

Please sign in to comment.