build: enable -Wdocumentation #21613

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:

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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/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](/bitcoin-bitcoin/contributor/param/) query_parse the query portion of the URI
          ^~~~~~~~~~~
          uri
69 warnings generated.

Note that a lot of these have already been fixed upstream.

Concept ACK

Nice! I was thinking about a doxygen linter at some point but happy this is part of C++ compilers now.

Concept and code review ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f

cr ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f: automatic compiler feedback comes sooner and is more reliable than manual reviewer feedback

Approach ACK and nice fixes that this finds. This is handy indeed.

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

In file included from test/addrman_tests.cpp:10:
./netbase.h:237:11: warning: parameter 'sock' not found in the function declaration [-Wdocumentation]
 * [@param](/bitcoin-bitcoin/contributor/param/) sock The SOCKS5 proxy socket.
          ^~~~
./netbase.h:237:11: note: did you mean 'socket'?
 * [@param](/bitcoin-bitcoin/contributor/param/) sock The SOCKS5 proxy socket.
          ^~~~
          socket
1 warning generated.

Approach ACK

I think the new flags -Wdocumentation and -Werror=documentation could have been enabled unconditionally, regardless of whether --suppress-external-warnings is used. Similarly to other -Wfoo which cause warnings with 3rd party libs and we have enabled unconditionally.

I think the new flags -Wdocumentation and -Werror=documentation could have been enabled unconditionally, regardless of whether --suppress-external-warnings is used. Similarly to other -Wfoo which cause warnings with 3rd party libs and we have enabled unconditionally.

We can't do this for the reasons outlined in the PR description. Anyone building with versions of upstream libraries where all -Wdocumentation issues hadn't been fixed already, would be inundated with warnings at compile time.