build: add `-Wleading-whitespace`

fanquake commented at 3:13 pm on May 13, 2025: member

GCC 15 now has options to turn leading & trailing whitespace into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable -Wleading-whitespace and -Wtrailing-whitespace.

DrahtBot commented at 3:13 pm on May 13, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32482.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc
Stale ACK	hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Build system on May 13, 2025

hebasto commented at 3:19 pm on May 13, 2025: member

GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options.

Concept ACK on that.

Unfortunately, CMake/Qt will generate code that contains trailing whitespace:

0[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
1/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing whitespace [-Wtrailing-whitespace=]
2 5603 | 
3/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:8254:2: warning: trailing whitespace [-Wtrailing-whitespace=]
4 8254 | 
5/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:9840:2: warning: trailing whitespace [-Wtrailing-whitespace=]

Will need to resolve that somehow.

I’ll look into it.

fanquake referenced this in commit c521192d8b on May 16, 2025

fanquake force-pushed on May 16, 2025

hebasto commented at 4:36 pm on May 30, 2025: member

GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options.

Concept ACK on that.
Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
0[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
1/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing whitespace [-Wtrailing-whitespace=]
2 5603 | 
3/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:8254:2: warning: trailing whitespace [-Wtrailing-whitespace=]
4 8254 | 
5/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:9840:2: warning: trailing whitespace [-Wtrailing-whitespace=]
Will need to resolve that somehow.
I’ll look into it.

Addressed in #32648.

fanquake force-pushed on Jul 2, 2025

fanquake renamed this:
~~build: add `-W[leading|trailing]-whitespace`~~
build: add `-Wleading-whitespace`
on Jul 2, 2025

fanquake marked this as ready for review on Jul 2, 2025

fanquake commented at 1:08 pm on July 2, 2025: member

Dropped -Wtrailing-whitespace for now. That could be added when Qts tools are fixed.

maflcko added the label DrahtBot Guix build requested on Aug 2, 2025

DrahtBot removed the label DrahtBot Guix build requested on Aug 3, 2025

fanquake force-pushed on Aug 3, 2025

hebasto commented at 3:14 pm on August 8, 2025: member

GCC 15 now has options to turn leading & trailing whitespace into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable -Wleading-whitespace. Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
0[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
1/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing whitespace [-Wtrailing-whitespace=]
2 5603 | 
3/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:8254:2: warning: trailing whitespace [-Wtrailing-whitespace=]
4 8254 | 
5/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:9840:2: warning: trailing whitespace [-Wtrailing-whitespace=]
So -Wtrailing-whitespace could be enabled later, once Qts tooling is fixed.

CMake 3.27 and newer handle this. The CMP0151 policy must be set to NEW.

hebasto approved

hebasto commented at 3:34 pm on August 8, 2025: member

ACK 5e2620443d13b58efb38310263d67e2b823d6a1e.

The first two commits were reviewed using git diff --ignore-space-change. Commit f2385191520f35fdef07743e5a12feaa87085dbe additionally removes an empty line, which is OK.

So -Wtrailing-whitespace could be enabled later, once Qts tooling is fixed.

CMake 3.27 and newer handle this. The CMP0151 policy must be set to NEW.

As it is reasonable to expect CMake ≥3.27 on systems with GCC 15, I believe -Wtrailing-whitespace can be reintroduced.

fanquake force-pushed on Aug 13, 2025

fanquake force-pushed on Aug 21, 2025

univalue: change leading whitespace to spaces 22ab605953

crypto: remove leading/trailing whitespace in sha256_sse4.cpp d7615afc28

build: add -Wleading-whitespace=spaces

This is available in GCC 15. See
https://gcc.gnu.org/gcc-15/changes.html.

f24e889717

build: add -Wtrailing-whitespace=any

This is available in GCC 15. See
https://gcc.gnu.org/gcc-15/changes.html.

8c60dc39a3

fanquake force-pushed on Aug 29, 2025

in src/crypto/sha256_sse4.cpp:1030 in 8c60dc39a3 outdated

1028 ; addm [mem], reg
1029 ; Add reg to mem using reg-mem add and store
1030 %macro addm 2
1031-    add	%2, %1
1032-    mov	%1, %2
1033+    add %2, %1

l0rinc commented at 7:52 pm on August 29, 2025:

nit: given that we’re fixing non-trailing tabs as well, the only remaining .cpp/.h file that still contains tabs is https://github.com/bitcoin/bitcoin/blob/75a5c8258ec5309fe506438aa3815608430b53d6/src/util/subprocess.h#L1149, if you edit again, consider including that

fanquake commented at 3:55 pm on September 1, 2025:

Sure. https://github.com/arun11299/cpp-subprocess/pull/121.

in src/crypto/sha256_sse4.cpp:972 in 8c60dc39a3 outdated

968@@ -969,27 +969,27 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks)
969 
970 /*
971 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
972-; Copyright (c) 2012, Intel Corporation 
973-; 
974-; All rights reserved. 
975-; 
976+; Copyright (c) 2012, Intel Corporation

l0rinc commented at 7:54 pm on August 29, 2025:

nit: there are a few remaining trailing spaces e.g. in https://github.com/bitcoin/bitcoin/blob/07f0a61ef711a2f75ded3d73545bfabdf2a64fef/src/minisketch/src/fields/clmul_common_impl.h#L39, we could add a PR to https://github.com/bitcoin-core/minisketch to get rid of those as well

fanquake commented at 4:00 pm on September 1, 2025:

https://github.com/bitcoin-core/minisketch/pull/98.

l0rinc approved

l0rinc commented at 7:57 pm on August 29, 2025: contributor

Core review ACK 8c60dc39a353c9d66cbe3a5a0a934eca6b2a287a

DrahtBot requested review from hebasto on Aug 29, 2025

subprocess: replace tab with space

See: https://github.com/arun11299/cpp-subprocess/pull/121.

b271b342e3

l0rinc commented at 6:49 pm on September 1, 2025: contributor

ACK b271b342e36000952a389c05b409bab691a4e4d2

fanquake referenced this in commit d1bd01e189 on Sep 2, 2025

bitcoin deleted a comment on Sep 2, 2025

build: add -Wleading-whitespace #32482

Code Coverage & Benchmarks

Reviews

build: add `-Wleading-whitespace` #32482