From 0afffcccc484c7f3876d303a1cb627ef5f2a5cfd Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 7 Apr 2021 16:49:30 +0800 Subject: [PATCH] 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 84eeaa7299..2293037236 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 6511ed614b..636dce09de 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 ef0eb85eae..0abf1e7e7b 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 377face66b..4c18e71e0c 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -250,7 +250,7 @@ public: /** * 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 9b2541fd93..b697c0c0dd 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 f9b29f28e5..f8dd6f2cf7 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 678022e1f2..8eb6447fbf 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