Skip to content

Build: enable -Wdocumentation via isystem #14920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Empact wants to merge 2 commits into from
Closed

Build: enable -Wdocumentation via isystem #14920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
422dcd5
build: Optionally include dependency headers with -isystem
Empact Nov 16, 2018
c8f4620
build: Enable -Wdocumentation if isystem is enabled
Empact Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions build-aux/m4/bitcoin_qt.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
])
])
Expand All @@ -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
])
Expand Down
3 changes: 2 additions & 1 deletion build-aux/m4/bitcoin_subdir_to_include.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
])
20 changes: 20 additions & 0 deletions build-aux/m4/bitcoin_system_include.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

@vasild vasild Feb 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: test "x$2" != "x" seems unnecessary? If $2 is empty, then the below echo | sed ... would still assign an empty string to $1?

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
])
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_mac_host.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=""
27 changes: 24 additions & 3 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)])],
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/Makefile.qt.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/scriptpubkeyman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down