build: add -Wundef
#29876
Merged
build: add -Wundef
#29876
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Concept ACK, issues with misspelled or otherwise missing defines can be really sneaky (and have caused some hard to find bugs in the past), this ia good check to have. |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
laanwj
reviewed
Apr 16, 2024
laanwj
approved these changes
Apr 26, 2024
hebasto
approved these changes
Apr 29, 2024
|
Rebased on #25972. Please review that first.
I don't touch that here, and it's not clear why we need to change a single instsance out of many help strings that could be similarly updated, so going to leave things as they are. |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
|
Concept ACK |
|
Any reason this is still in draft? I agree it seems useful do enable before the CMake switch. |
Rebased, and fixed the conflict. Added another change for the Win64 CI. We'll probably need to fixup the failing ARM and previous releases job upstream: In file included from minisketch/src/fields/generic_common_impl.h:12,
from minisketch/src/fields/generic_4bytes.cpp:12:
minisketch/src/fields/../int_utils.h:162:7: error: "HAVE_CLZ" is not defined, evaluates to 0 [-Werror=undef]
162 | #elif HAVE_CLZ
| ^~~~~~~~This was the multiprocess job failure: /bin/bash ../libtool --tag=CXX --preserve-dup-deps --mode=link /usr/bin/ccache clang++ -m32 -std=c++20 -g -O2 -O0 -g3 -ftrapv -fdebug-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wundef -Wno-unused-parameter -Wno-self-assign -Werror -fPIE -pipe -std=c++20 -O1 -g -Wno-error=documentation -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -pie -pthread -lpthread -L/ci_container_base/depends/i686-pc-linux-gnu/lib -o bitcoind bitcoind-bitcoind.o init/bitcoind-bitcoind.o libbitcoin_node.a libbitcoin_wallet.a libbitcoin_common.a libbitcoin_util.a libunivalue.la libbitcoin_zmq.a libbitcoin_consensus.a crypto/libbitcoin_crypto_base.la crypto/libbitcoin_crypto_sse41.la crypto/libbitcoin_crypto_avx2.la crypto/libbitcoin_crypto_x86_shani.la leveldb/libleveldb.la crc32c/libcrc32c.la leveldb/libmemenv.la secp256k1/libsecp256k1.la -ldb_cxx-4.8 -lminiupnpc -lnatpmp -L/ci_container_base/depends/i686-pc-linux-gnu/lib -levent_pthreads -levent -L/ci_container_base/depends/i686-pc-linux-gnu/lib -levent -L/ci_container_base/depends/i686-pc-linux-gnu/lib -lzmq -lpthread -lrt -L/ci_container_base/depends/i686-pc-linux-gnu/lib -lsqlite3 -lm -latomic
libtool: link: /usr/bin/ccache clang++ -m32 -std=c++20 -g -O2 -O0 -g3 -ftrapv -fdebug-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wundef -Wno-unused-parameter -Wno-self-assign -Werror -fPIE -pipe -std=c++20 -O1 -g -Wno-error=documentation -Wl,-z -Wl,relro -Wl,-z -Wl,now -Wl,-z -Wl,separate-code -pie -o bitcoind bitcoind-bitcoind.o init/bitcoind-bitcoind.o -lpthread -L/ci_container_base/depends/i686-pc-linux-gnu/lib libbitcoin_node.a libbitcoin_wallet.a libbitcoin_common.a libbitcoin_util.a ./.libs/libunivalue.a libbitcoin_zmq.a libbitcoin_consensus.a crypto/.libs/libbitcoin_crypto_base.a crypto/.libs/libbitcoin_crypto_sse41.a crypto/.libs/libbitcoin_crypto_avx2.a crypto/.libs/libbitcoin_crypto_x86_shani.a leveldb/.libs/libleveldb.a crc32c/.libs/libcrc32c.a leveldb/.libs/libmemenv.a secp256k1/.libs/libsecp256k1.a -ldb_cxx-4.8 -lminiupnpc -lnatpmp -levent_pthreads -levent -levent -lzmq -lpthread -lrt -lsqlite3 -lm -latomic -pthread
/usr/bin/ccache clang++ -m32 -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config -DDEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DRPC_DOC_CHECK -DABORT_ON_FAILED_ASSUME -fmacro-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -DHAVE_BUILD_INFO -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -I/ci_container_base/depends/i686-pc-linux-gnu/include/ -DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -g -O2 -O0 -g3 -ftrapv -fdebug-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wundef -Wno-unused-parameter -Wno-self-assign -Werror -fPIE -I/ci_container_base/depends/i686-pc-linux-gnu/include -pthread -pipe -std=c++20 -O1 -g -Wno-error=documentation -c -o ipc/capnp/libbitcoin_ipc_a-protocol.o `test -f 'ipc/capnp/protocol.cpp' || echo './'`ipc/capnp/protocol.cpp
In file included from ipc/capnp/protocol.cpp:7:
In file included from ./ipc/capnp/init.capnp.h:6:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/generated-header-support.h:26:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/raw-schema.h:24:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/common.h:31:
/ci_container_base/depends/i686-pc-linux-gnu/include/kj/windows-sanity.h:39:6: error: '_WIN32' is not defined, evaluates to 0 [-Werror,-Wundef]
39 | #if !_WIN32 && !__CYGWIN__
| ^
/ci_container_base/depends/i686-pc-linux-gnu/include/kj/windows-sanity.h:39:17: error: '__CYGWIN__' is not defined, evaluates to 0 [-Werror,-Wundef]
39 | #if !_WIN32 && !__CYGWIN__
| ^
In file included from ipc/capnp/protocol.cpp:7:
In file included from ./ipc/capnp/init.capnp.h:6:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/generated-header-support.h:26:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/raw-schema.h:24:
/ci_container_base/depends/i686-pc-linux-gnu/include/capnp/common.h:33:5: error: 'CAPNP_DEBUG_TYPES' is not defined, evaluates to 0 [-Werror,-Wundef]
33 | #if CAPNP_DEBUG_TYPES
| ^
In file included from ipc/capnp/protocol.cpp:7:
In file included from ./ipc/capnp/init.capnp.h:6:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/generated-header-support.h:26:
/ci_container_base/depends/i686-pc-linux-gnu/include/capnp/raw-schema.h:26:5: error: '_MSC_VER' is not defined, evaluates to 0 [-Werror,-Wundef]
26 | #if _MSC_VER && !defined(__clang__)
| ^
In file included from ipc/capnp/protocol.cpp:7:
In file included from ./ipc/capnp/init.capnp.h:9:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/capability.h:28:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/kj/async.h:24:
/ci_container_base/depends/i686-pc-linux-gnu/include/kj/async-prelude.h:34:6: error: 'KJ_NO_EXCEPTIONS' is not defined, evaluates to 0 [-Werror,-Wundef]
34 | #if !KJ_NO_EXCEPTIONS
| ^
In file included from ipc/capnp/protocol.cpp:7:
In file included from ./ipc/capnp/init.capnp.h:9:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/capnp/capability.h:28:
In file included from /ci_container_base/depends/i686-pc-linux-gnu/include/kj/async.h:26:
/ci_container_base/depends/i686-pc-linux-gnu/include/kj/refcount.h:26:5: error: '_MSC_VER' is not defined, evaluates to 0 [-Werror,-Wundef]
26 | #if _MSC_VER
| ^
6 errors generated.
make[2]: *** [Makefile:9738: ipc/capnp/libbitcoin_ipc_a-protocol.o] Error 1
make[2]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/src'
make[1]: *** [Makefile:20118: install-recursive] Error 1
make[1]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/src' |
cc @ryanofsky |
|
This is ready for further review. |
Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]
|
Without this change there are errors from boost like: /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/expired_slot.hpp:23:28: error: 'what' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/detail/signal_template.hpp:750:32: error: 'lock_pimpl' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/connection.hpp:150:22: error: 'connected' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] There do not seem to be errors from capnproto currently, but add a suppression for it, too, to be consistent with other libraries.
randomenv.cpp:48:5: warning: 'HAVE_VM_VM_PARAM_H' is not defined, evaluates to 0 [-Wundef] randomenv.cpp:51:5: warning: 'HAVE_SYS_RESOURCES_H' is not defined, evaluates to 0 [-Wundef] randomenv.cpp:424:5: error: 'HAVE_SYSCTL' is not defined, evaluates to 0 [-Werror,-Wundef]
```bash
init.cpp:526:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
526 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
init.cpp:541:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
541 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
init.cpp:1318:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
1318 | #if HAVE_SOCKADDR_UN
```
```
netbase.cpp:26:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
26 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
netbase.cpp:221:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
221 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
netbase.cpp:496:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
496 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
netbase.cpp:531:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
531 | #if HAVE_SOCKADDR_UN
| ^~~~~~~~~~~~~~~~
netbase.cpp:639:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef]
639 | #if HAVE_SOCKADDR_UN
```
"Warn if an undefined identifier is evaluated in an #if directive. Such identifiers are replaced with zero."
|
Rebased to fix conflicts. |
|
Ported to the CMake-based build system in hebasto#264. |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Turn on
-Wundef.> Warn if an undefined identifier is evaluated in an #if directive. Such identifiers are replaced with zero..
Note that this is still beneficial with CMake, and may even be nice to have enabled prior, to catch any change in behaviour.
If we end up with this enabled, it should probably be enough to fix #16419.