diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 9824956fe908..bc3eb825512b 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -110,6 +110,8 @@ code. - `nullptr` is preferred over `NULL` or `(void*)0`. - `static_assert` is preferred over `assert` where possible. Generally; compile-time checking is preferred over run-time checking. - Align pointers and references to the left i.e. use `type& var` and not `type &var`. + - Recursion is checked by clang-tidy and thus must be made explicit. Use + `NOLINTNEXTLINE(misc-no-recursion)` to suppress the check. For function calls a namespace should be specified explicitly, unless such functions have been declared within it. Otherwise, [argument-dependent lookup](https://en.cppreference.com/w/cpp/language/adl), also known as ADL, could be diff --git a/src/.clang-tidy b/src/.clang-tidy index 3d8b242963ff..e336b08b1af2 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,6 +1,7 @@ Checks: ' -*, bugprone-argument-comment, +misc-no-recursion, modernize-use-nullptr, readability-const-return-type, readability-redundant-declaration, diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index cfd164c64544..cfbbf17987bf 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -68,6 +68,7 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std: txn = CPartialMerkleTree(vHashes, vMatch); } +// NOLINTNEXTLINE(misc-no-recursion) uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::vector &vTxid) { //we can never have zero txs in a merkle block, we always need the coinbase tx //if we do not have this assert, we can hit a memory access violation when indexing into vTxid @@ -88,6 +89,7 @@ uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::ve } } +// NOLINTNEXTLINE(misc-no-recursion) void CPartialMerkleTree::TraverseAndBuild(int height, unsigned int pos, const std::vector &vTxid, const std::vector &vMatch) { // determine whether this node is the parent of at least one matched txid bool fParentOfMatch = false; @@ -106,6 +108,7 @@ void CPartialMerkleTree::TraverseAndBuild(int height, unsigned int pos, const st } } +// NOLINTNEXTLINE(misc-no-recursion) uint256 CPartialMerkleTree::TraverseAndExtract(int height, unsigned int pos, unsigned int &nBitsUsed, unsigned int &nHashUsed, std::vector &vMatch, std::vector &vnIndex) { if (nBitsUsed >= vBits.size()) { // overflowed the bits array - failure diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 493c8606c86c..d4cf4897dd26 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -608,6 +608,7 @@ class NodeImpl : public Node NodeContext* m_context{nullptr}; }; +// NOLINTNEXTLINE(misc-no-recursion) bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock& lock, const CChain& active) { if (!index) return false; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 482caccec770..c62ac04bbae7 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -814,6 +814,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const NONFATAL_UNREACHABLE(); } +// NOLINTNEXTLINE(misc-no-recursion) bool RPCResult::MatchesType(const UniValue& result) const { switch (m_type) { @@ -861,6 +862,7 @@ void RPCResult::CheckInnerDoc() const CHECK_NONFATAL(inner_needed != m_inner.empty()); } +// NOLINTNEXTLINE(misc-no-recursion) std::string RPCArg::ToStringObj(const bool oneline) const { std::string res; @@ -898,6 +900,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const NONFATAL_UNREACHABLE(); } +// NOLINTNEXTLINE(misc-no-recursion) std::string RPCArg::ToString(const bool oneline) const { if (oneline && !m_oneline_description.empty()) return m_oneline_description; @@ -915,6 +918,7 @@ std::string RPCArg::ToString(const bool oneline) const } case Type::OBJ: case Type::OBJ_USER_KEYS: { + // NOLINTNEXTLINE(misc-no-recursion) const std::string res = Join(m_inner, ",", [&](const RPCArg& i) { return i.ToStringObj(oneline); }); if (m_type == Type::OBJ) { return "{" + res + "}"; diff --git a/src/rpc/util.h b/src/rpc/util.h index a9f5d6246fd1..4bf3fcedcec2 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -133,6 +133,7 @@ enum class OuterType { /** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */ std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider); +// NOLINTNEXTLINE(misc-no-recursion) struct RPCArg { enum class Type { OBJ, @@ -238,6 +239,7 @@ struct RPCArg { std::string ToDescriptionString() const; }; +// NOLINTNEXTLINE(misc-no-recursion) struct RPCResult { enum class Type { OBJ, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index c6d2c5a82a15..cdd1026c5f69 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -522,6 +522,7 @@ class DescriptorImpl : public Descriptor NORMALIZED, }; + // NOLINTNEXTLINE(misc-no-recursion) bool IsSolvable() const override { for (const auto& arg : m_subdescriptor_args) { @@ -530,6 +531,7 @@ class DescriptorImpl : public Descriptor return true; } + // NOLINTNEXTLINE(misc-no-recursion) bool IsRange() const final { for (const auto& pubkey : m_pubkey_args) { @@ -541,6 +543,7 @@ class DescriptorImpl : public Descriptor return false; } + // NOLINTNEXTLINE(misc-no-recursion) virtual bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const { size_t pos = 0; @@ -553,6 +556,7 @@ class DescriptorImpl : public Descriptor return true; } + // NOLINTNEXTLINE(misc-no-recursion) bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const { std::string extra = ToStringExtra(); @@ -602,6 +606,7 @@ class DescriptorImpl : public Descriptor return ret; } + // NOLINTNEXTLINE(misc-no-recursion) bool ExpandHelper(int pos, const SigningProvider& arg, const DescriptorCache* read_cache, std::vector& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache) const { std::vector> entries; @@ -643,6 +648,7 @@ class DescriptorImpl : public Descriptor return ExpandHelper(pos, DUMMY_SIGNING_PROVIDER, &read_cache, output_scripts, out, nullptr); } + // NOLINTNEXTLINE(misc-no-recursion) void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const final { for (const auto& p : m_pubkey_args) { @@ -930,6 +936,7 @@ std::unique_ptr ParsePubkey(uint32_t key_exp_index, const Span ParseScript(uint32_t& key_exp_index, Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { using namespace spanparsing; @@ -1052,6 +1059,7 @@ std::unique_ptr InferPubkey(const CPubKey& pubkey, ParseScriptCo return key_provider; } +// NOLINTNEXTLINE(misc-no-recursion) std::unique_ptr InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider) { std::vector> data; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 99a6109fee68..af05660acb84 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -126,6 +126,7 @@ std::shared_ptr MinerTestingSetup::BadBlock(const uint256& prev_ha return ret; } +// NOLINTNEXTLINE(misc-no-recursion) void MinerTestingSetup::BuildChain(const uint256& root, int height, const unsigned int invalid_rate, const unsigned int branch_rate, const unsigned int max_size, std::vector>& blocks) { if (height <= 0 || blocks.size() >= max_size) return; diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index ec4a1aae64b8..d933d662f217 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -16,6 +16,7 @@ #include #include +// NOLINTNEXTLINE(misc-no-recursion) class UniValue { public: enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VBOOL, }; diff --git a/src/univalue/lib/univalue_write.cpp b/src/univalue/lib/univalue_write.cpp index 4a3cbba20fda..4a2219061a7a 100644 --- a/src/univalue/lib/univalue_write.cpp +++ b/src/univalue/lib/univalue_write.cpp @@ -27,6 +27,7 @@ static std::string json_escape(const std::string& inS) return outS; } +// NOLINTNEXTLINE(misc-no-recursion) std::string UniValue::write(unsigned int prettyIndent, unsigned int indentLevel) const { @@ -66,6 +67,7 @@ static void indentStr(unsigned int prettyIndent, unsigned int indentLevel, std:: s.append(prettyIndent * indentLevel, ' '); } +// NOLINTNEXTLINE(misc-no-recursion) void UniValue::writeArray(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const { s += "["; @@ -88,6 +90,7 @@ void UniValue::writeArray(unsigned int prettyIndent, unsigned int indentLevel, s s += "]"; } +// NOLINTNEXTLINE(misc-no-recursion) void UniValue::writeObject(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const { s += "{"; diff --git a/src/util/string.h b/src/util/string.h index e87d0a6e02ab..6d73ab9fb9de 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -72,6 +72,7 @@ void ReplaceAll(std::string& in_out, const std::string& search, const std::strin * @param unary_op Apply this operator to each item */ template +// NOLINTNEXTLINE(misc-no-recursion) auto Join(const C& container, const S& separator, UnaryOp unary_op) { decltype(unary_op(*container.begin())) ret; diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index a5d0bcc59988..3c752b6a726b 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -254,6 +254,7 @@ bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminef return (CachedTxGetDebit(wallet, wtx, filter) > 0); } +// NOLINTNEXTLINE(misc-no-recursion) bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set& trusted_parents) { AssertLockHeld(wallet.cs_wallet); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index a23ba9e7c0cc..6e9cedc28527 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -343,6 +343,7 @@ class DescribeWalletAddressVisitor public: const SigningProvider * const provider; + // NOLINTNEXTLINE(misc-no-recursion) void ProcessSubScript(const CScript& subscript, UniValue& obj) const { // Always present: script type and redeemscript @@ -394,6 +395,7 @@ class DescribeWalletAddressVisitor return obj; } + // NOLINTNEXTLINE(misc-no-recursion) UniValue operator()(const ScriptHash& scriptHash) const { CScriptID scriptID(scriptHash); UniValue obj(UniValue::VOBJ); @@ -403,6 +405,7 @@ class DescribeWalletAddressVisitor } return obj; } + }; static UniValue DescribeWalletAddress(const CWallet& wallet, const CTxDestination& dest) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 617d905170df..ba91fd238e2b 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1115,6 +1115,7 @@ enum class ScriptContext // Analyse the provided scriptPubKey, determining which keys and which redeem scripts from the ImportData struct are needed to spend it, and mark them as used. // Returns an error string, or the empty string for success. +// NOLINTNEXTLINE(misc-no-recursion) static std::string RecurseImportData(const CScript& script, ImportData& import_data, const ScriptContext script_ctx) { // Use Solver to obtain script type and parsed pubkeys or hashes: diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d8ef16cab882..4669b1522cc7 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -84,6 +84,7 @@ bool HaveKeys(const std::vector& pubkeys, const LegacyScriptPubKeyMan& //! @param recurse_scripthash whether to recurse into nested p2sh //! scripts or simply treat any script that has been //! stored in the keystore as spendable +// NOLINTNEXTLINE(misc-no-recursion) IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion, bool recurse_scripthash=true) { IsMineResult ret = IsMineResult::NO;