Merge bitcoin/bitcoin#29690: clang-tidy: Enable misc-no-recursion

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

src/.clang-tidy

@@ -1,6 +1,7 @@
 Checks: '
 -*,
 bugprone-argument-comment,
+misc-no-recursion,
 modernize-use-nullptr,
 readability-const-return-type,
 readability-redundant-declaration,

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<uint256> &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<uint256> &vTxid, const std::vector<bool> &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<uint256> &vMatch, std::vector<unsigned int> &vnIndex) {
     if (nBitsUsed >= vBits.size()) {
         // overflowed the bits array - failure

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<RecursiveMutex>& lock, const CChain& active)
 {
     if (!index) return false;

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 + "}";

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<CScript> 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,

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<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache) const
     {
         std::vector<std::pair<CPubKey, KeyOriginInfo>> 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<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c
 }
 
 /** Parse a script in a particular context. */
+// NOLINTNEXTLINE(misc-no-recursion)
 std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
 {
     using namespace spanparsing;
@@ -1052,6 +1059,7 @@ std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptCo
     return key_provider;
 }
 
+// NOLINTNEXTLINE(misc-no-recursion)
 std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider)
 {
     std::vector<std::vector<unsigned char>> data;

src/test/validation_block_tests.cpp

@@ -126,6 +126,7 @@ std::shared_ptr<const CBlock> 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<std::shared_ptr<const CBlock>>& blocks)
 {
     if (height <= 0 || blocks.size() >= max_size) return;

src/univalue/include/univalue.h

@@ -16,6 +16,7 @@
 #include <type_traits>
 #include <vector>
 
+// NOLINTNEXTLINE(misc-no-recursion)
 class UniValue {
 public:
     enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VBOOL, };

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 += "{";

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 <typename C, typename S, typename UnaryOp>
+// NOLINTNEXTLINE(misc-no-recursion)
 auto Join(const C& container, const S& separator, UnaryOp unary_op)
 {
     decltype(unary_op(*container.begin())) ret;

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<uint256>& trusted_parents)
 {
     AssertLockHeld(wallet.cs_wallet);

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)

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:

src/wallet/scriptpubkeyman.cpp

@@ -84,6 +84,7 @@ bool HaveKeys(const std::vector<valtype>& 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;