-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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.
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
build: ensure we aren't using GNU extensions #18088
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. If we can do without compiler-specific extensions we should. |
Concept ACK 👍 |
Here's a set of changes to remove the |
4fa56b2
to
612e8e1
Compare
Thanks, I've taken some of those changes. |
Concept ACK, this was for sure the intent of [noext]. |
Concept ACK -- inside the standard is better than outside the standard |
612e8e1
to
6af7c41
Compare
Rebased on master now that #18087 is in, fixed up the sanitizer issue in prevector (bad rebase) and re-ordered some commits. |
ACK 6af7c41 -- diff looks correct |
6af7c41
to
c9ab71a
Compare
Isn't saw it here again: 3cb2664#diff-998357bc40c89d644815a6fd52c99c9fR213 We can probably replace all usages of |
c9ab71a
to
1f6f29e
Compare
These are a gnu extension warned against by: gnu-zero-variadic-macro-arguments
When compiling with Clang, this will warn when GNU extensions are used. Info: https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu
05fb3da
to
0ae8f18
Compare
Rebased after #18591. |
utACK 0ae8f18 If we currently do not see any warnings due to the newly introduced |
ACK 0ae8f18 Read relevant gcc docs, built, and ran tests. |
ACK 0ae8f18 |
0ae8f18 build: add -Wgnu to compile flags (fanquake) 3a0fd77 Remove use of non-standard zero variadic macros (Ben Woosley) 49f6178 Drop unused LOG_TIME_MICROS helper (Ben Woosley) 5d49999 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes) Pull request description: Since we [started using](bitcoin#7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile". However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former. #### anonymous structs ```bash ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct] struct { ``` This is fixed in bitcoin@b849212. #### variadic macros ```bash ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] ::Unserialize(s, VARINT(nVersionDummy)); ``` This is taken care of in bitcoin#18087. The `LOG_TIME_*` macros introduced in bitcoin#16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html). ```bash In file included from validation.cpp:22: ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ 6 warnings generated. ``` This is fixed in 081a0ab and 612e8e1. #### prevention To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions. This would close bitcoin#14130. Also related to bitcoin#17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute. ACKs for top commit: practicalswift: ACK 0ae8f18 -- diff looks correct MarcoFalke: ACK 0ae8f18 vasild: utACK 0ae8f18 dongcarl: ACK 0ae8f18 Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
@vasild Would you mind taking on the |
Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to #18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin/bitcoin#18088 build: ensure we aren't using GNU extensions
Summary: Remove the use of gnu extensions: - anonymous structs - variadic macros with no argument And add a Clang warning for detecting their future use. Backport of core [[bitcoin/bitcoin#18088 | PR18088]]. Test Plan: With clang: ninja all check Check there is no warning. Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D9109
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
a30b0a2 build: enable -Werror=gnu (Vasil Dimov) Pull request description: Stop the build if a warning is emitted due to `-Wgnu` and `--enable-werror` has been used. As usual - this would help notice such a warning that is about to be introduced in new code. This is a followup to bitcoin#18088 build: ensure we aren't using GNU extensions ACKs for top commit: practicalswift: ACK a30b0a2 Empact: ACK bitcoin@a30b0a2 Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
Since we started using the
ax_cxx_compile_stdcxx.m4
macro we've been passing[noext]
to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn off extension support so they would fail to compile".However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.
anonymous structs
This is fixed in b849212.
variadic macros
This is taken care of in #18087.
The
LOG_TIME_*
macros introduced in #16805 make use of a GNU extension.This is fixed in 081a0ab and 612e8e1.
prevention
To ensure that usage doesn't creep back in we can add
-Wgnu
to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.This would close #14130.
Also related to #17230, where it's suggested we use a GNU extension, the
gnu::pure
attribute.