ci: add unused-using-decls to clang-tidy

fanquake commented at 4:26 pm on June 24, 2022: member

Adds https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html to our clang-tidy. PR’d after the discussion in #25433 (which it includes).

DrahtBot added the label Refactoring on Jun 24, 2022

DrahtBot commented at 3:39 am on June 25, 2022: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25308 (refactor: Reduce number of LoadChainstate parameters and return values by ryanofsky)
#24149 (Signing support for Miniscript Descriptors by darosior)
#23319 (rpc: Return fee and prevout (utxos) to getrawtransaction by dougEfresh)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/test/miniscript_tests.cpp:119 in 6b301a648b outdated

115@@ -116,7 +116,7 @@ struct KeyConverter {
116 //! Singleton instance of KeyConverter.
117 const KeyConverter CONVERTER{};
118 
119-using miniscript::operator"" _mst;
120+using miniscript::operator"" _mst; // NOLINT(misc-unused-using-decls)

maflcko commented at 9:35 am on June 27, 2022:

Is there an upstream bug report?

fanquake commented at 9:48 am on June 27, 2022:

Looks like there is: https://github.com/llvm/llvm-project/issues/53444 https://github.com/llvm/llvm-project/issues/55095

Seems like the false-positive may have begun appearing from LLVM 14.0.0. I haven’t yet tested if it’s fixed in 15.0.0.

maflcko commented at 1:49 pm on June 27, 2022:

Looks like this passes with clang-13:

0clang-tidy-13 --use-color -p=/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu /bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/miniscript_tests.cpp
1651 warnings generated.
21302 warnings generated.
3Suppressed 1302 warnings (1302 in non-user code).
4Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

 0diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh
 1index 453a34ca7..064bee2ca 100755
 2--- a/ci/test/04_install.sh
 3+++ b/ci/test/04_install.sh
 4@@ -115,8 +115,8 @@ if [[ "${RUN_TIDY}" == "true" ]]; then
 5   export DIR_IWYU="${BASE_SCRATCH_DIR}/iwyu"
 6   if [ ! -d "${DIR_IWYU}" ]; then
 7     CI_EXEC "mkdir -p ${DIR_IWYU}/build/"
 8-    CI_EXEC "git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_14 ${DIR_IWYU}/include-what-you-use"
 9-    CI_EXEC "cd ${DIR_IWYU}/build && cmake -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-14 ../include-what-you-use"
10+    CI_EXEC "git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_13 ${DIR_IWYU}/include-what-you-use"
11+    CI_EXEC "cd ${DIR_IWYU}/build && cmake -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-13 ../include-what-you-use"
12     CI_EXEC "cd ${DIR_IWYU}/build && make install $MAKEJOBS"
13   fi
14 fi
15diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
16index 1679a5f96..3877fea90 100644
17--- a/src/test/miniscript_tests.cpp
18+++ b/src/test/miniscript_tests.cpp
19@@ -116,7 +116,7 @@ struct KeyConverter {
20 //! Singleton instance of KeyConverter.
21 const KeyConverter CONVERTER{};
22 
23-using miniscript::operator"" _mst; // NOLINT(misc-unused-using-decls)
24+using miniscript::operator"" _mst;
25 
26 enum TestMode : int {
27     TESTMODE_INVALID = 0,

And switch jammy for bookworm, see

https://packages.debian.org/bookworm/clang

maflcko commented at 2:59 pm on June 27, 2022:

or alternatively add a link to the bug report in this line

fanquake commented at 9:34 am on June 28, 2022:

or alternatively add a link to the bug report in this line

I’d rather not revert to using LLVM / clang 13, so have added a link to the upstream report.

fanquake force-pushed on Jun 28, 2022

fanquake force-pushed on Jun 29, 2022

in src/test/miniscript_tests.cpp:119 in 3c61799d36 outdated

115@@ -116,7 +116,9 @@ struct KeyConverter {
116 //! Singleton instance of KeyConverter.
117 const KeyConverter CONVERTER{};
118 
119-using miniscript::operator"" _mst; // NOLINT(misc-unused-using-decls)

maflcko commented at 4:00 pm on July 1, 2022:

please squash this

fanquake commented at 4:03 pm on July 1, 2022:

not sure how that ended up in the wrong commit. fixed now

fanquake force-pushed on Jul 1, 2022

DrahtBot added the label Needs rebase on Jul 18, 2022

fanquake force-pushed on Jul 18, 2022

validation: remove unused using directives

The following were unused from the node namespace:
- BLOCKFILE_CHUNK_SIZE
- nPruneTarget
- OpenBlockFile
- UNDOFILE_CHUNK_SIZE

3617634324

fanquake force-pushed on Jul 18, 2022

refactor: remove unused using directives d6787bc19b

tidy: use misc-unused-using-decls

https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html

a02f3f19f5

fanquake force-pushed on Jul 18, 2022

jamesob approved

jamesob commented at 5:05 pm on July 18, 2022: member

Github ACK https://github.com/bitcoin/bitcoin/pull/25466/commits/a02f3f19f52e628248f81acc2410e67f3d49baf5

DrahtBot removed the label Needs rebase on Jul 18, 2022

maflcko merged this on Jul 19, 2022

maflcko closed this on Jul 19, 2022

fanquake deleted the branch on Jul 19, 2022

sidhujag referenced this in commit 96daa89807 on Jul 19, 2022

in src/test/miniscript_tests.cpp:124 in a02f3f19f5

119@@ -120,6 +120,8 @@ struct KeyConverter {
120 //! Singleton instance of KeyConverter.
121 const KeyConverter CONVERTER{};
122 
123+// https://github.com/llvm/llvm-project/issues/53444
124+// NOLINTNEXTLINE(misc-unused-using-decls)

fanquake commented at 10:25 am on July 27, 2022:

The issue necessitating the NOLINTNEXTLINE added here is fixed in llvm 16. See https://github.com/llvm/llvm-project/issues/55095.

fanquake commented at 10:48 am on April 5, 2023:

Followed up with in #27404.

PastaPastaPasta referenced this in commit b6c18ae577 on Oct 18, 2022

bitcoin locked this on Apr 4, 2024

ci: add unused-using-decls to clang-tidy #25466

Conflicts