diff options
author | MarcoFalke <falke.marco@gmail.com> | 2022-03-31 08:18:02 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2022-03-31 08:18:30 +0200 |
commit | 87dc1dc55ffaffd782a16c7b50e188433912a617 (patch) | |
tree | 132af052b0e40f0977c18a3284232b228c1edbfb /src | |
parent | 74b011bbfa3b607606cc7c0ce6e2d22cfd07605a (diff) | |
parent | 2ef47ba6c57a12840499a13908ab61aefca6cb55 (diff) | |
download | bitcoin-87dc1dc55ffaffd782a16c7b50e188433912a617.tar.xz |
Merge bitcoin/bitcoin#24714: util/check: Don't use a lambda for Assert/Assume
2ef47ba6c57a12840499a13908ab61aefca6cb55 util/check: stop using lambda for Assert/Assume (Anthony Towns)
7c9fe25c16d48b53a61fa2f6ff77eaf8820cb1f6 wallet: move Assert() check into constructor (Anthony Towns)
Pull request description:
Using a lambda creates a couple of odd namespacing issues, in particular making clang's thread safety analysis less helpful, and confusing gcc when calling member functions. Fix this by not using a lambda.
Fixes #21596
Fixes #24654
ACKs for top commit:
MarcoFalke:
ACK 2ef47ba6c57a12840499a13908ab61aefca6cb55 🚢
jonatack:
Tested re-ACK 2ef47ba6c57a12840499a13908ab61aefca6cb55
Tree-SHA512: 4bdbf3215f3d14472df0552362c5eebe8b7eea2d0928a8a41109edd4e0c5f95de6f8220eb2fee8506874e352c003907faf5ef344174795939306a618157b1bae
Diffstat (limited to 'src')
-rw-r--r-- | src/Makefile.am | 2 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 33 | ||||
-rw-r--r-- | src/util/check.cpp | 14 | ||||
-rw-r--r-- | src/util/check.h | 24 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.cpp | 1 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.h | 2 |
6 files changed, 67 insertions, 9 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index d3ae9a8d8c..e14b5ec040 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -620,6 +620,7 @@ libbitcoin_util_a_SOURCES = \ util/asmap.cpp \ util/bip32.cpp \ util/bytevectorhash.cpp \ + util/check.cpp \ util/error.cpp \ util/fees.cpp \ util/getuniquepath.cpp \ @@ -838,6 +839,7 @@ bitcoin_chainstate_SOURCES = \ uint256.cpp \ util/asmap.cpp \ util/bytevectorhash.cpp \ + util/check.cpp \ util/getuniquepath.cpp \ util/hasher.cpp \ util/moneystr.cpp \ diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 1881573e7a..b5d8411e1d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -78,6 +78,31 @@ BOOST_AUTO_TEST_CASE(util_datadir) BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); } +namespace { +class NoCopyOrMove +{ +public: + int i; + explicit NoCopyOrMove(int i) : i{i} { } + + NoCopyOrMove() = delete; + NoCopyOrMove(const NoCopyOrMove&) = delete; + NoCopyOrMove(NoCopyOrMove&&) = delete; + NoCopyOrMove& operator=(const NoCopyOrMove&) = delete; + NoCopyOrMove& operator=(NoCopyOrMove&&) = delete; + + operator bool() const { return i != 0; } + + int get_ip1() { return i + 1; } + bool test() + { + // Check that Assume can be used within a lambda and still call methods + [&]() { Assume(get_ip1()); }(); + return Assume(get_ip1() != 5); + } +}; +} // namespace + BOOST_AUTO_TEST_CASE(util_check) { // Check that Assert can forward @@ -89,6 +114,14 @@ BOOST_AUTO_TEST_CASE(util_check) // Check that Assume can be used as unary expression const bool result{Assume(two == 2)}; Assert(result); + + // Check that Assert doesn't require copy/move + NoCopyOrMove x{9}; + Assert(x).i += 3; + Assert(x).test(); + + // Check nested Asserts + BOOST_CHECK_EQUAL(Assert((Assert(x).test() ? 3 : 0)), 3); } BOOST_AUTO_TEST_CASE(util_criticalsection) diff --git a/src/util/check.cpp b/src/util/check.cpp new file mode 100644 index 0000000000..2a9f885560 --- /dev/null +++ b/src/util/check.cpp @@ -0,0 +1,14 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <util/check.h> + +#include <tinyformat.h> + +void assertion_fail(const char* file, int line, const char* func, const char* assertion) +{ + auto str = strprintf("%s:%s %s: Assertion `%s' failed.\n", file, line, func, assertion); + fwrite(str.data(), 1, str.size(), stderr); + std::abort(); +} diff --git a/src/util/check.h b/src/util/check.h index a443c13cf2..80e973e7e2 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -47,14 +47,26 @@ class NonFatalCheckError : public std::runtime_error #endif /** Helper for Assert() */ -template <typename T> -T get_pure_r_value(T&& val) +void assertion_fail(const char* file, int line, const char* func, const char* assertion); + +/** Helper for Assert()/Assume() */ +template <bool IS_ASSERT, typename T> +T&& inline_assertion_check(T&& val, const char* file, int line, const char* func, const char* assertion) { + if constexpr (IS_ASSERT +#ifdef ABORT_ON_FAILED_ASSUME + || true +#endif + ) { + if (!val) { + assertion_fail(file, line, func, assertion); + } + } return std::forward<T>(val); } /** Identity function. Abort if the value compares equal to zero */ -#define Assert(val) ([&]() -> decltype(get_pure_r_value(val)) { auto&& check = (val); assert(#val && check); return std::forward<decltype(get_pure_r_value(val))>(check); }()) +#define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val) /** * Assume is the identity function. @@ -66,10 +78,6 @@ T get_pure_r_value(T&& val) * - For non-fatal errors in interactive sessions (e.g. RPC or command line * interfaces), CHECK_NONFATAL() might be more appropriate. */ -#ifdef ABORT_ON_FAILED_ASSUME -#define Assume(val) Assert(val) -#else -#define Assume(val) ([&]() -> decltype(get_pure_r_value(val)) { auto&& check = (val); return std::forward<decltype(get_pure_r_value(val))>(check); }()) -#endif +#define Assume(val) inline_assertion_check<false>(val, __FILE__, __LINE__, __func__, #val) #endif // BITCOIN_UTIL_CHECK_H diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index cb006dea3a..38d23b6f91 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -9,6 +9,7 @@ namespace wallet { WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), + m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args))}, m_wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()) { m_wallet.LoadWallet(); diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index d4b855b145..e0b38a40e7 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -22,7 +22,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr<interfaces::WalletLoader> m_wallet_loader = interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args)); + std::unique_ptr<interfaces::WalletLoader> m_wallet_loader; CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; |