Build: enable -Wdocumentation via isystem

build-aux/m4/bitcoin_qt.m4

@@ -421,17 +421,17 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITH_PKGCONFIG],[
     QT_LIB_PREFIX=Qt5
     qt5_modules="Qt5Core Qt5Gui Qt5Network Qt5Widgets"
     BITCOIN_QT_CHECK([
-      PKG_CHECK_MODULES([QT5], [$qt5_modules], [QT_INCLUDES="$QT5_CFLAGS"; QT_LIBS="$QT5_LIBS" have_qt=yes],[have_qt=no])
+      PKG_CHECK_MODULES([QT5], [$qt5_modules], [have_qt=yes; BITCOIN_SYSTEM_INCLUDE([QT_INCLUDES], [$QT5_CFLAGS]) QT_MOC_INCLUDES="$QT5_CFLAGS"; QT_LIBS="$QT5_LIBS"],[have_qt=no])
 
       if test "x$have_qt" != xyes; then
         have_qt=no
         BITCOIN_QT_FAIL([Qt dependencies not found])
       fi
     ])
     BITCOIN_QT_CHECK([
-      PKG_CHECK_MODULES([QT_TEST], [${QT_LIB_PREFIX}Test], [QT_TEST_INCLUDES="$QT_TEST_CFLAGS"; have_qt_test=yes], [have_qt_test=no])
+      PKG_CHECK_MODULES([QT_TEST], [${QT_LIB_PREFIX}Test], [have_qt_test=yes; BITCOIN_SYSTEM_INCLUDE([QT_TEST_INCLUDES], [$QT_TEST_CFLAGS])], [have_qt_test=no])
       if test "x$use_dbus" != xno; then
-        PKG_CHECK_MODULES([QT_DBUS], [${QT_LIB_PREFIX}DBus], [QT_DBUS_INCLUDES="$QT_DBUS_CFLAGS"; have_qt_dbus=yes], [have_qt_dbus=no])
+        PKG_CHECK_MODULES([QT_DBUS], [${QT_LIB_PREFIX}DBus], [have_qt_dbus=yes; BITCOIN_SYSTEM_INCLUDE([QT_DBUS_INCLUDES], [$QT_DBUS_CFLAGS])], [have_qt_dbus=no])
       fi
     ])
   ])
@@ -451,7 +451,8 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITHOUT_PKGCONFIG],[
   TEMP_LIBS="$LIBS"
   BITCOIN_QT_CHECK([
     if test "x$qt_include_path" != x; then
-      QT_INCLUDES="-I$qt_include_path -I$qt_include_path/QtCore -I$qt_include_path/QtGui -I$qt_include_path/QtWidgets -I$qt_include_path/QtNetwork -I$qt_include_path/QtTest -I$qt_include_path/QtDBus"
+      QT_MOC_INCLUDES="-I$qt_include_path -I$qt_include_path/QtCore -I$qt_include_path/QtGui -I$qt_include_path/QtWidgets -I$qt_include_path/QtNetwork -I$qt_include_path/QtTest -I$qt_include_path/QtDBus"
+      BITCOIN_SYSTEM_INCLUDE([QT_INCLUDES], [$QT_MOC_INCLUDES])
       CPPFLAGS="$QT_INCLUDES $CPPFLAGS"
     fi
   ])

build-aux/m4/bitcoin_subdir_to_include.m4

@@ -12,7 +12,8 @@ AC_DEFUN([BITCOIN_SUBDIR_TO_INCLUDE],[
     newinclpath=`${CXXCPP} ${CPPFLAGS} -M conftest.cpp 2>/dev/null | [ tr -d '\\n\\r\\\\' | sed -e 's/^.*[[:space:]:]\(\/[^[:space:]]*\)]$3[\.h[[:space:]].*$/\1/' -e t -e d`]
     AC_MSG_RESULT([${newinclpath}])
     if test "x${newinclpath}" != "x"; then
-      eval "$1=\"\$$1\"' -I${newinclpath}'"
+      BITCOIN_SYSTEM_INCLUDE([newincl], ["-I${newinclpath}"])
+      eval "$1=\"\$$1\"' ${newincl}'"
     fi
   fi
 ])

build-aux/m4/bitcoin_system_include.m4

@@ -0,0 +1,20 @@
+dnl Copyright (c) 2018 The Bitcoin Core developers
+dnl Distributed under the MIT software license, see the accompanying
+dnl file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+dnl BITCOIN_SYSTEM_INCLUDE([DESTINATION-VARIABLE-NAME], [INCLUDES-STRING])
+dnl Populates the variable DESTINATION with the value from INCLUDES but
+dnl with -I includes switched to -isystem, with the exception of -I/usr/include,
+dnl which is left unmodified to avoid order issues with /usr/include. See:
+dnl https://stackoverflow.com/questions/37218953/isystem-on-a-system-include-directory-causes-errors
+AC_DEFUN([BITCOIN_SYSTEM_INCLUDE],[
+  dnl If use_isystem is enabled and INCLUDES-STRING is not empty, translate to isystem
+  if test "x$use_isystem" = "xyes" && test "x$2" != "x"; then
+    dnl Including /usr/include with -isystem is known to cause problems, e.g.
+    dnl breaking include_next due to, the directive changing inclusion order.
+    dnl https://bugreports.qt.io/browse/QTBUG-53367
+    $1=$(echo $2 | sed -e 's| -I| -isystem|g' -e 's|^-I|-isystem|g' -e 's|-isystem/usr/include|-I/usr/include|g')
+  else
+    $1=$2
+  fi
+])

ci/test/00_setup_env_mac_host.sh

@@ -13,7 +13,7 @@ export RUN_CI_ON_HOST=true
 export RUN_UNIT_TESTS=true
 export RUN_FUNCTIONAL_TESTS=false
 export GOAL="install"
-export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --enable-werror"
+export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --enable-werror --enable-isystem"
 # Run without depends
 export NO_DEPENDS=1
 export OSX_SDK=""

configure.ac

@@ -180,6 +180,12 @@ AC_ARG_ENABLE([ccache],
   [use_ccache=$enableval],
   [use_ccache=auto])
 
+AC_ARG_ENABLE([isystem],
+  [AS_HELP_STRING([--enable-isystem],
+  [enable isystem includes for dependencies and stricter warning checks (default is no)])],
+  [use_isystem=$enableval],
+  [use_isystem=no])
+
 AC_ARG_ENABLE([lcov],
   [AS_HELP_STRING([--enable-lcov],
   [enable lcov testing (default is no)])],
@@ -338,6 +344,12 @@ if test "x$enable_werror" = "xyes"; then
   AX_CHECK_COMPILE_FLAG([-Werror=unused-variable],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unused-variable"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Werror=date-time],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=date-time"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Werror=return-type],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=return-type"],,[[$CXXFLAG_WERROR]])
+
+  ## Some warnings alert on violations in our dependencies. Only enable them when
+  ## isystem is in use to keep the build clean
+  if test "x$use_isystem" = "xyes"; then
+    AX_CHECK_COMPILE_FLAG([-Werror=documentation],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=documentation"],,[[$CXXFLAG_WERROR]])
+  fi
 fi
 
 if test "x$CXXFLAGS_overridden" = "xno"; then
@@ -361,6 +373,12 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
   AX_CHECK_COMPILE_FLAG([-Wunused-local-typedef],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-local-typedef"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wdeprecated-register],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-register"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-implicit-fallthrough"],,[[$CXXFLAG_WERROR]])
+
+  ## Some warnings alert on violations in our dependencies. Only enable them when
+  ## isystem is in use to keep the build clean
+  if test "x$use_isystem" = "xyes" && test "x$enable_werror" == "xno"; then
+    AX_CHECK_COMPILE_FLAG([-Wdocumentation],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"],,[[$CXXFLAG_WERROR]])
+  fi
 fi
 
 enable_sse42=no
@@ -574,7 +592,8 @@ case $host in
          bdb_prefix=$($BREW --prefix berkeley-db4 2>/dev/null)
          qt5_prefix=$($BREW --prefix qt5 2>/dev/null)
          if test x$bdb_prefix != x; then
-           CPPFLAGS="$CPPFLAGS -I$bdb_prefix/include"
+           BITCOIN_SYSTEM_INCLUDE([bdb_cppflags], ["-I$bdb_prefix/include"])
+           CPPFLAGS="$CPPFLAGS $bdb_cppflags"
            LIBS="$LIBS -L$bdb_prefix/lib"
          fi
          if test x$qt5_prefix != x; then
@@ -1255,9 +1274,9 @@ if test x$use_pkgconfig = xyes; then
         BITCOIN_QT_CHECK([PKG_CHECK_MODULES([QR], [libqrencode], [have_qrencode=yes], [have_qrencode=no])])
       fi
       if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests != xnononono; then
-        PKG_CHECK_MODULES([EVENT], [libevent],, [AC_MSG_ERROR(libevent not found.)])
+        PKG_CHECK_MODULES([EVENT], [libevent], [BITCOIN_SYSTEM_INCLUDE([EVENT_CFLAGS], [$EVENT_CFLAGS])], [AC_MSG_ERROR(libevent not found.)])
         if test x$TARGET_OS != xwindows; then
-          PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads],, [AC_MSG_ERROR(libevent_pthreads not found.)])
+          PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads], [BITCOIN_SYSTEM_INCLUDE([EVENT_PTHREADS_CFLAGS], [$EVENT_PTHREADS_CFLAGS])], [AC_MSG_ERROR(libevent_pthreads not found.)])
         fi
       fi
 
@@ -1366,6 +1385,7 @@ fi
 
 if test x$need_bundled_univalue = xyes ; then
   UNIVALUE_CFLAGS='-I$(srcdir)/univalue/include'
+  BITCOIN_SYSTEM_INCLUDE([UNIVALUE_CFLAGS], [$UNIVALUE_CFLAGS])
   UNIVALUE_LIBS='univalue/libunivalue.la'
 fi
 
@@ -1667,6 +1687,7 @@ fi
 echo "  with bench    = $use_bench"
 echo "  with upnp     = $use_upnp"
 echo "  use asm       = $use_asm"
+echo "  use isystem   = $use_isystem"
 echo "  sanitizers    = $use_sanitizers"
 echo "  debug enabled = $enable_debug"
 echo "  gprof enabled = $enable_gprof"

src/Makefile.qt.include

@@ -358,11 +358,11 @@ ui_%.h: %.ui
 	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(UIC) -o $@ $< || (echo "Error creating $@"; false)
 
 %.moc: %.cpp
-	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(MOC) $(DEFAULT_INCLUDES) $(QT_INCLUDES) $(MOC_DEFS) $< | \
+	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(MOC) $(DEFAULT_INCLUDES) $(QT_MOC_INCLUDES) $(MOC_DEFS) $< | \
 	  $(SED) -e '/^\*\*.*Created:/d' -e '/^\*\*.*by:/d' > $@
 
 moc_%.cpp: %.h
-	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(MOC) $(DEFAULT_INCLUDES) $(QT_INCLUDES) $(MOC_DEFS) $< | \
+	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(MOC) $(DEFAULT_INCLUDES) $(QT_MOC_INCLUDES) $(MOC_DEFS) $< | \
 	  $(SED) -e '/^\*\*.*Created:/d' -e '/^\*\*.*by:/d' > $@
 
 %.qm: %.ts

src/wallet/scriptpubkeyman.cpp

@@ -73,7 +73,7 @@ bool HaveKeys(const std::vector<valtype>& 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 / witnessscript)
 //! @param recurse_scripthash  whether to recurse into nested p2sh and p2wsh
 //!                            scripts or simply treat any script that has been