Skip to content

Commit 9446de1

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#24831: tidy: add include-what-you-use
9b0a13a tidy: Add include-what-you-use (fanquake) 74cd038 refactor: fix includes in src/init (fanquake) c79ad93 refactor: fix includes in src/compat (fanquake) Pull request description: We recently added a [`clang-tidy` job](https://github.com/bitcoin/bitcoin/blob/master/ci/test/00_setup_env_native_tidy.sh) to the CI, which generates a compilation database. We can leverage that now existing database to begin running [include-what-you-use](https://include-what-you-use.org/) over the codebase. This PR demonstrates using a mapping_file to indicate fixups / includes that may differ from IWYU suggestions. In this case, I've added some fixups for glibc includes that I've [upstreamed changes for](include-what-you-use/include-what-you-use#1026): ```bash # Fixups / upstreamed changes [ { include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] }, ] ``` The include "fixing" commits of this PR: * Adds missing includes. * Swaps C headers for their C++ counterparts. * Removes the pointless / unmaintainable `//for abc, xyz` comments. When using IWYU, if anyone wants to see / generate those comments, to see why something is included, it is trivial to do so (IWYU outputs them by default). i.e: ```cpp // The full include-list for compat/stdin.cpp: #include <compat/stdin.h> #include <poll.h> // for poll, pollfd, POLLIN #include <termios.h> // for tcgetattr, tcsetattr #include <unistd.h> // for isatty, STDIN_FILENO ``` TODO: - [ ] Qt mapping_file. There is one in the IWYU repo, but it's for Qt 5.11. Needs testing. - [ ] Boost mapping_file. There is one in the IWYU repo, but it's for Boost 1.75. Needs testing. I'm not suggesting we turn this on the for entire codebase, or immediately go-nuts refactoring all includes. However I think our dependency includes are now slim enough, and our CI infrastructure in place such that we can start doing this in some capacity, and just automate away include fixups / refactorings etc. ACKs for top commit: MarcoFalke: review ACK 9b0a13a jonatack: ACK 9b0a13a reviewed changes and run CI output in https://cirrus-ci.com/task/4750910332076032 Tree-SHA512: 00beab5a5f2a6fc179abf08321a15391ecccaa91ab56f3c50c511e7b29a0d7c95d8bb43eac2c31489711086f6f77319d43d803cf8ea458e7cd234a780d9ae69e
2 parents 4381681 + 9b0a13a commit 9446de1

16 files changed

+40
-14
lines changed

‎ci/test/00_setup_env_native_tidy.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
88

99
export DOCKER_NAME_TAG="ubuntu:22.04"
1010
export CONTAINER_NAME=ci_native_tidy
11-
export PACKAGES="clang llvm clang-tidy bear libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev"
11+
export PACKAGES="clang libclang-dev llvm-dev clang-tidy bear cmake libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev"
1212
export NO_DEPENDS=1
1313
export RUN_UNIT_TESTS=false
1414
export RUN_FUNCTIONAL_TESTS=false

‎ci/test/04_install.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then
111111
CI_EXEC "cd ${BASE_SCRATCH_DIR}/msan/build/ && make $MAKEJOBS cxx"
112112
fi
113113

114+
if [[ "${RUN_TIDY}" == "true" ]]; then
115+
CI_EXEC "mkdir -p ${BASE_SCRATCH_DIR}/iwyu/build/"
116+
CI_EXEC "git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_14 ${BASE_SCRATCH_DIR}/iwyu/include-what-you-use"
117+
CI_EXEC "cd ${BASE_SCRATCH_DIR}/iwyu/build && cmake -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-14 ../include-what-you-use"
118+
CI_EXEC "cd ${BASE_SCRATCH_DIR}/iwyu/build && make install $MAKEJOBS"
119+
fi
120+
114121
if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
115122
echo "Create $BASE_ROOT_DIR"
116123
CI_EXEC rsync -a /ro_base/ "$BASE_ROOT_DIR"

‎ci/test/06_script_b.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ fi
3737
if [ "${RUN_TIDY}" = "true" ]; then
3838
export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
3939
CI_EXEC run-clang-tidy "${MAKEJOBS}"
40+
export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/"
41+
CI_EXEC "python3 ${BASE_SCRATCH_DIR}/iwyu/include-what-you-use/iwyu_tool.py src/compat src/init -p . ${MAKEJOBS} -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp"
4042
fi
4143

4244
if [ "$RUN_SECURITY_TESTS" = "true" ]; then

‎configure.ac

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1933,6 +1933,7 @@ AC_CONFIG_LINKS([contrib/devtools/security-check.py:contrib/devtools/security-ch
19331933
AC_CONFIG_LINKS([contrib/devtools/symbol-check.py:contrib/devtools/symbol-check.py])
19341934
AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-security-check.py])
19351935
AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py])
1936+
AC_CONFIG_LINKS([contrib/devtools/iwyu/bitcoin.core.imp:contrib/devtools/iwyu/bitcoin.core.imp])
19361937
AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
19371938
AC_CONFIG_LINKS([contrib/macdeploy/background.tiff:contrib/macdeploy/background.tiff])
19381939
AC_CONFIG_LINKS([src/.bear-tidy-config:src/.bear-tidy-config])

‎contrib/devtools/iwyu/bitcoin.core.imp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Fixups / upstreamed changes
2+
[
3+
{ include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] },
4+
{ include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] },
5+
{ include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] },
6+
]

‎src/compat/byteswap.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <config/bitcoin-config.h>
1010
#endif
1111

12-
#include <stdint.h>
12+
#include <cstdint>
1313

1414
#if defined(HAVE_BYTESWAP_H)
1515
#include <byteswap.h>

‎src/compat/cpuid.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
#include <cpuid.h>
1212

13+
#include <cstdint>
14+
1315
// We can't use cpuid.h's __get_cpuid as it does not support subleafs.
1416
void static inline GetCPUID(uint32_t leaf, uint32_t subleaf, uint32_t& a, uint32_t& b, uint32_t& c, uint32_t& d)
1517
{

‎src/compat/endian.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
#include <compat/byteswap.h>
1313

14-
#include <stdint.h>
14+
#include <cstdint>
1515

1616
#if defined(HAVE_ENDIAN_H)
1717
#include <endian.h>

‎src/compat/glibcxx_sanity.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <list>
66
#include <locale>
77
#include <stdexcept>
8+
#include <string>
89

910
namespace
1011
{

‎src/compat/stdin.cpp

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,19 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5-
#if defined(HAVE_CONFIG_H)
6-
#include <config/bitcoin-config.h>
7-
#endif
5+
#include <compat/stdin.h>
86

9-
#include <cstdio> // for fileno(), stdin
7+
#include <cstdio>
108

119
#ifdef WIN32
12-
#include <windows.h> // for SetStdinEcho()
13-
#include <io.h> // for isatty()
10+
#include <windows.h>
11+
#include <io.h>
1412
#else
15-
#include <termios.h> // for SetStdinEcho()
16-
#include <unistd.h> // for SetStdinEcho(), isatty()
17-
#include <poll.h> // for StdinReady()
13+
#include <termios.h>
14+
#include <unistd.h>
15+
#include <poll.h>
1816
#endif
1917

20-
#include <compat/stdin.h>
21-
2218
// https://stackoverflow.com/questions/1413445/reading-a-password-from-stdcin
2319
void SetStdinEcho(bool enable)
2420
{

0 commit comments

Comments
 (0)