From 0afffcccc484c7f3876d303a1cb627ef5f2a5cfd Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 7 Apr 2021 16:49:30 +0800 Subject: [PATCH 1/3] Merge #21613: build: enable -Wdocumentation a4e970adb6de8425025ae3f62fb89d9e27a8ab1f build: enable -Wdocumentation if suppressing external warnings (fanquake) 3b0078f958c46e94b468c829522ba965f5549f11 doc: fixup -Wdocumentation issues (fanquake) c6edcf1c710e4aaf1cafdbf8e86fe209b57bdeb8 build: suppress libevent warnings if supressing external warnings (fanquake) Pull request description: Enable `-Wdocumentation` by taking advantage of our `--enable-suppress-external-warnings` flag. Most of the CIs are using this flag now, so any regressions should be caught. This also required modifying libevents flags when suppressing warnings, as depending on the version being built against, that could generate a large number of warnings. i.e: ```bash In file included from httpserver.cpp:34: In file included from ./support/events.h:12: /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:464:11: warning: parameter 'req' not found in the function declaration [-Wdocumentation] @param req a request object ^~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:465:11: warning: parameter 'databuf' not found in the function declaration [-Wdocumentation] @param databuf the data chunk to send as part of the reply. ^~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:467:11: warning: parameter 'call' not found in the function declaration [-Wdocumentation] @param call back's argument. ^~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:939:4: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated; you probably want to use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:946:1: note: add a deprecation attribute to the declaration to silence this warning char *evhttp_decode_uri(const char *uri); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:979:5: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated as of Libevent 2.0.9. Use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:987:1: note: add a deprecation attribute to the declaration to silence this warning int evhttp_parse_query(const char *uri, struct evkeyvalq *headers); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: warning: parameter 'query_parse' not found in the function declaration [-Wdocumentation] @param query_parse the query portion of the URI ^~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: note: did you mean 'uri'? @param query_parse the query portion of the URI ^~~~~~~~~~~ uri 69 warnings generated. ``` Note that a lot of these have already been fixed upstream. ACKs for top commit: laanwj: Concept and code review ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f practicalswift: cr ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f: automatic compiler feedback comes sooner and is more reliable than manual reviewer feedback jonatack: Light ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f skimmed the changes, clang 11 build is clean with the change, verified -Wdocumentation build warnings with this change when a doc fix was reverted Tree-SHA512: 57a1e30cffcc8bcceee72d85f58ebe29eae525861c70acb237541bd480c51ede89875c033042c0af376fdbb49fb7f588ef9282a47c6e78f9d4501c41f1b21eb6 --- configure.ac | 12 ++++++++++++ src/net_processing.cpp | 2 +- src/netbase.h | 3 +-- src/util/sock.h | 2 +- src/validation.h | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/scriptpubkeyman.cpp | 2 +- 7 files changed, 18 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 84eeaa7299d3..22930372360a 100644 --- a/configure.ac +++ b/configure.ac @@ -469,6 +469,10 @@ if test "x$enable_werror" = "xyes"; then AX_CHECK_COMPILE_FLAG([-Werror=unreachable-code-loop-increment],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unreachable-code-loop-increment"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=mismatched-tags], [ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=mismatched-tags"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Werror=implicit-fallthrough], [ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=implicit-fallthrough"], [], [$CXXFLAG_WERROR]) + + if test x$suppress_external_warnings != xno ; then + AX_CHECK_COMPILE_FLAG([-Werror=documentation],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=documentation"],,[[$CXXFLAG_WERROR]]) + fi fi if test "x$CXXFLAGS_overridden" = "xno"; then @@ -494,6 +498,10 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wunreachable-code-loop-increment],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code-loop-increment"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wimplicit-fallthrough"], [], [$CXXFLAG_WERROR]) + if test x$suppress_external_warnings != xno ; then + AX_CHECK_COMPILE_FLAG([-Wdocumentation],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"],,[[$CXXFLAG_WERROR]]) + fi + dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all dnl unknown options if any other warning is produced. Test the -Wfoo case, and dnl set the -Wno-foo case if it works. @@ -1527,6 +1535,10 @@ if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench if test x$TARGET_OS != xwindows; then PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads >= 2.0.21],, [AC_MSG_ERROR([libevent_pthreads version 2.0.21 or greater not found.])]) fi + + if test x$suppress_external_warnings != xno; then + EVENT_CFLAGS=SUPPRESS_WARNINGS($EVENT_CFLAGS) + fi fi if test x$use_libevent = xyes; then diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6511ed614b40..636dce09de65 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2986,7 +2986,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, /** * Reconsider orphan transactions after a parent has been accepted to the mempool. * - * @param[in/out] orphan_work_set The set of orphan transactions to reconsider. Generally only one + * @param[in,out] orphan_work_set The set of orphan transactions to reconsider. Generally only one * orphan will be reconsidered on each call of this function. This set * may be added to if accepting an orphan causes its children to be * reconsidered. diff --git a/src/netbase.h b/src/netbase.h index ef0eb85eae8e..0abf1e7e7b49 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -176,7 +176,6 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo * @param strSubnet A string representation of a subnet of the form `network * address [ "/", ( CIDR-style suffix | netmask ) ]`(e.g. * `2001:db8::/32`, `192.0.2.0/255.255.255.0`, or `8.8.8.8`). - * @param ret The resulting internal representation of a subnet. * * @returns Whether the operation succeeded or not. */ @@ -237,7 +236,7 @@ void InterruptSocks5(bool interrupt); * @param port The destination port. * @param auth The credentials with which to authenticate with the specified * SOCKS5 proxy. - * @param sock The SOCKS5 proxy socket. + * @param socket The SOCKS5 proxy socket. * * @returns Whether or not the operation succeeded. * diff --git a/src/util/sock.h b/src/util/sock.h index 377face66b8f..4c18e71e0cf0 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -250,7 +250,7 @@ class Sock /** * Check if still connected. - * @param[out] err The error string, if the socket has been disconnected. + * @param[out] errmsg The error string, if the socket has been disconnected. * @return true if connected */ [[nodiscard]] virtual bool IsConnected(std::string& errmsg) const; diff --git a/src/validation.h b/src/validation.h index 9b2541fd933a..b697c0c0dd4f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1153,7 +1153,7 @@ inline bool IsBlockPruned(const CBlockIndex* pblockindex) /** * Return the expected assumeutxo value for a given height, if one exists. * - * @param height[in] Get the assumeutxo value for this height. + * @param[in] height Get the assumeutxo value for this height. * * @returns empty if no assumeutxo configuration exists for the given height. */ diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f9b29f28e5aa..f8dd6f2cf7f4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1348,7 +1348,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) /** * List transactions based on the given criteria. * - * @param pwallet The wallet. + * @param wallet The wallet. * @param wtx The wallet transaction. * @param nMinDepth The minimum confirmation depth. * @param fLong Whether to include the JSON version of the transaction. diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 678022e1f22e..8eb6447fbf47 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -77,7 +77,7 @@ bool HaveKeys(const std::vector& pubkeys, const LegacyScriptPubKeyMan& //! Recursively solve script and return spendable/watchonly/invalid status. //! //! @param keystore legacy key and script store -//! @param script script to solve +//! @param scriptPubKey script to solve //! @param sigversion script type (top-level / redeemscript) //! @param recurse_scripthash whether to recurse into nested p2sh //! scripts or simply treat any script that has been From 4f260cd4b1b318d0ad4285de9f2cb2dc6703a551 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 13 Jul 2024 01:29:37 +0700 Subject: [PATCH 2/3] fix: ignore warnings for dashbls/ --- src/Makefile.am | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index f1a3cda67d21..e0b108f22c6a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -37,8 +37,8 @@ endif endif #ENABLE_STACKTRACES BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/secp256k1/include -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) $(BDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -BITCOIN_INCLUDES+=-I$(srcdir)/dashbls/include -I$(srcdir)/dashbls/depends/relic/include -I$(srcdir)/dashbls/depends/minialloc/include -BITCOIN_INCLUDES+=-I$(srcdir)/immer +BITCOIN_INCLUDES+=-isystem$(srcdir)/dashbls/include -isystem$(srcdir)/dashbls/depends/relic/include -isystem$(srcdir)/dashbls/depends/minialloc/include +BITCOIN_INCLUDES+=-isystem$(srcdir)/immer LIBBITCOIN_SERVER=libbitcoin_server.a LIBBITCOIN_COMMON=libbitcoin_common.a @@ -921,8 +921,8 @@ libdashconsensus_la_SOURCES = support/cleanse.cpp $(crypto_libbitcoin_crypto_bas libdashconsensus_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) libdashconsensus_la_LIBADD = $(LIBDASHBLS) $(LIBSECP256K1) $(GMP_LIBS) libdashconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL -libdashconsensus_la_CPPFLAGS += -I$(srcdir)/dashbls/include -I$(srcdir)/dashbls/depends/relic/include -I$(srcdir)/dashbls/depends/minialloc/include -libdashconsensus_la_CPPFLAGS += -I$(srcdir)/immer +libdashconsensus_la_CPPFLAGS += -isystem$(srcdir)/dashbls/include -isystem$(srcdir)/dashbls/depends/relic/include -isystem$(srcdir)/dashbls/depends/minialloc/include +libdashconsensus_la_CPPFLAGS += -isystem$(srcdir)/immer libdashconsensus_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) endif From 252ffee81576a539eb2859ae58e1ba859dcb2043 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 13 Jul 2024 01:58:21 +0700 Subject: [PATCH 3/3] fix: adjust doxygen for dash codebase for -Wdocumentation --- src/cxxtimer.hpp | 2 +- src/dbwrapper.h | 2 +- src/evo/deterministicmns.h | 9 --------- src/node/transaction.h | 1 - src/rpc/rawtransaction_util.h | 3 +-- src/util/enumerate.h | 2 +- src/validation.h | 1 - src/wallet/rpcwallet.cpp | 2 +- src/wallet/scriptpubkeyman.h | 2 +- src/wallet/wallet.h | 3 +-- 10 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/cxxtimer.hpp b/src/cxxtimer.hpp index 155c5051ff4c..0b58b5d97134 100644 --- a/src/cxxtimer.hpp +++ b/src/cxxtimer.hpp @@ -107,7 +107,7 @@ class Timer { /** * Return the elapsed time. * - * @param duration_t + * @tparam duration_t * The duration type used to return the time elapsed. If not * specified, it returns the time as represented by * std::chrono::milliseconds. diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 9d9a64b1cfa0..f9247c1df9b7 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -61,7 +61,7 @@ class CDBBatch public: /** - * @param[in] parent CDBWrapper that this batch is to be submitted to + * @param[in] _parent CDBWrapper that this batch is to be submitted to */ explicit CDBBatch(const CDBWrapper &_parent) : parent(_parent), ssKey(SER_DISK, CLIENT_VERSION), ssValue(SER_DISK, CLIENT_VERSION), size_estimate(0) { }; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 9202f19b2209..59c7673d52e2 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -346,15 +346,11 @@ class CDeterministicMNList * Calculates the projected MN payees for the next *count* blocks. The result is not guaranteed to be correct * as PoSe banning might occur later * @param nCount the number of payees to return. "nCount = max()"" means "all", use it to avoid calling GetValidWeightedMNsCount twice. - * @return */ [[nodiscard]] std::vector GetProjectedMNPayees(gsl::not_null pindexPrev, int nCount = std::numeric_limits::max()) const; /** * Calculate a quorum based on the modifier. The resulting list is deterministically sorted by score - * @param maxSize - * @param modifier - * @return */ [[nodiscard]] std::vector CalculateQuorum(size_t maxSize, const uint256& modifier, const bool onlyEvoNodes = false) const; [[nodiscard]] std::vector> CalculateScores(const uint256& modifier, const bool onlyEvoNodes) const; @@ -362,7 +358,6 @@ class CDeterministicMNList /** * Calculates the maximum penalty which is allowed at the height of this MN list. It is dynamic and might change * for every block. - * @return */ [[nodiscard]] int CalcMaxPoSePenalty() const; @@ -371,8 +366,6 @@ class CDeterministicMNList * value later passed to PoSePunish. The percentage should be high enough to take per-block penalty decreasing for MNs * into account. This means, if you want to accept 2 failures per payment cycle, you should choose a percentage that * is higher then 50%, e.g. 66%. - * @param percent - * @return */ [[nodiscard]] int CalcPenalty(int percent) const; @@ -380,8 +373,6 @@ class CDeterministicMNList * Punishes a MN for misbehavior. If the resulting penalty score of the MN reaches the max penalty, it is banned. * Penalty scores are only increased when the MN is not already banned, which means that after banning the penalty * might appear lower then the current max penalty, while the MN is still banned. - * @param proTxHash - * @param penalty */ void PoSePunish(const uint256& proTxHash, int penalty, bool debugLogs); diff --git a/src/node/transaction.h b/src/node/transaction.h index 7da45f602865..3867401586fa 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -36,7 +36,6 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; * @param[in] node reference to node context * @param[in] tx the transaction to broadcast * @param[out] err_string reference to std::string to fill with error string if available - * @param[in] max_tx_fee reject txs with fees higher than this (if 0, accept any fee) * @param[in] relay flag if both mempool insertion and p2p relay are requested * @param[in] wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC. * return error diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index f36a7c092b23..c8865588bc07 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -19,7 +19,6 @@ class SigningProvider; * Sign a transaction with the given keystore and previous transactions * * @param mtx The transaction to-be-signed - * @param prevTxsUnival Array of previous txns outputs that tx depends on but may not yet be in the block chain * @param keystore Temporary keystore containing signing keys * @param coins Map of unspent outputs * @param hashType The signature hash type @@ -31,7 +30,7 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const /** * Parse a prevtxs UniValue array and get the map of coins from it * - * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain + * @param prevTxsUnival Array of previous txns outputs that tx depends on but may not yet be in the block chain * @param keystore A pointer to the temprorary keystore if there is one * @param coins Map of unspent outputs - coins in mempool and current chain UTXO set, may be extended by previous txns outputs after call */ diff --git a/src/util/enumerate.h b/src/util/enumerate.h index bc826ebe2b11..0a6d5e68f287 100644 --- a/src/util/enumerate.h +++ b/src/util/enumerate.h @@ -10,7 +10,7 @@ /** * similar to python's enumerate(iterable). * @tparam T type of iterable, automatically deduced - * @tparam TIter + * @tparam TIter begin of containter * @param iterable an iterable object, can be a temporary * @return struct containing a size_t index, and it's element in iterable */ diff --git a/src/validation.h b/src/validation.h index b697c0c0dd4f..a5d891db06aa 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1099,7 +1099,6 @@ class ChainstateManager * @param[out] state This may be set to an Error state if any error occurred processing them * @param[in] chainparams The params for the chain we want to connect to * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers - * @param[out] first_invalid First header that fails validation, if one exists */ bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f8dd6f2cf7f4..f9b29f28e5aa 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1348,7 +1348,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) /** * List transactions based on the given criteria. * - * @param wallet The wallet. + * @param pwallet The wallet. * @param wtx The wallet transaction. * @param nMinDepth The minimum confirmation depth. * @param fLong Whether to include the JSON version of the transaction. diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 9c639b73ebfb..4b3d24d9fae0 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -250,7 +250,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv CryptedKeyMap mapCryptedKeys GUARDED_BY(cs_KeyStore); WatchOnlySet setWatchOnly GUARDED_BY(cs_KeyStore); WatchKeyMap mapWatchKeys GUARDED_BY(cs_KeyStore); - HDPubKeyMap mapHdPubKeys GUARDED_BY(cs_KeyStore); // GetAllScriptPubKeyMans() const; - //! Get the ScriptPubKeyMan for the given OutputType and internal/external chain. + //! Get the ScriptPubKeyMan for internal/external chain. ScriptPubKeyMan* GetScriptPubKeyMan(bool internal) const; //! Get the ScriptPubKeyMan for a script @@ -1472,7 +1472,6 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Remove specified ScriptPubKeyMan from set of active SPK managers. Writes the change to the wallet file. //! @param[in] id The unique id for the ScriptPubKeyMan - //! @param[in] type The OutputType this ScriptPubKeyMan provides addresses for //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses void DeactivateScriptPubKeyMan(uint256 id, bool internal);