Skip to content

Conversation

fanquake
Copy link
Member

This remove the addition of flags that are already part of other options, such as -Wall or -Wextra; see each commit message for details. All of the flags being removed here already exist as part of -Wall as of GCC 8, or, for Clang, all exist in -Wmost (included in -Wall), or as part of -Wextra as of Clang 7. Both of which are our minimum required compilers.

Also cherry-picks one change from #21458.

To give an example of how GCCs -Wall has changed over the last few releases:

11.x to trunk (12.x)

Added:

-Wzero-length-bounds
-Wmismatched-dealloc
-Wmismatched-new-delete (only for C/C++)

10.x to 11.x

Added:

-Warray-parameter=2 (C and Objective-C only)
-Wrange-loop-construct (only for C++)
-Wsizeof-array-div
-Wvla-parameter (C and Objective-C only)

Removed:

-Wenum-conversion in C/ObjC;

9.x to 10.x

Added:

-Wenum-conversion in C/ObjC;
-Wformat-overflow
-Wformat-truncation
-Wzero-length-bounds

8.x to 9.x

Added:

-Wpessimizing-move

Removed:

-Wstringop-truncation

7.x to 8.x

Added:

-Wcatch-value (C++ and Objective-C++ only)
-Wmissing-attributes
-Wmultistatement-macros
-Wrestrict
-Wsizeof-pointer-div
-Wstringop-truncation

Clang Warning Options
GCC Warning Options

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, but would be nice to not add random binary files in intermediate commits and then remove them again.

fanquake and others added 5 commits October 13, 2021 13:59
This is part of -Wall in GCC and -Wextra in Clang.
To turn on all (future) loop analysis options. Note that
-Wfor-loop-analysis is also part of -Wmost, which is in -Wall.
This is enabled by -Wall in GCC and Clang.
This is enabled via -Wall in GCC, and is
part of -Wunused, which is included in -Wmost, which is included in
-Wall in Clang.
-Wunused-local-typedef(s) is covered by -Wunused in both gcc and clang.
No new warnings fire when compiling.
@fanquake fanquake force-pushed the further_flag_cleanup branch from c4f5168 to aa69fd6 Compare October 13, 2021 06:00
@fanquake
Copy link
Member Author

ACK, but would be nice to not add random binary files in intermediate commits and then remove them again.

Ugh. Minisketch testing and overzealous use of git add .. Fixed up now.

@maflcko
Copy link
Member

maflcko commented Oct 13, 2021

git add --patch fixes this ;) (or enumerating the files by hand)

@fanquake
Copy link
Member Author

Guix Build:

bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
ef08b72ba5c8b3d2c78a61e0420307b44c03a9f6d0818943569d456e2dc866dd  guix-build-aa69fd6caf1b/output/aarch64-linux-gnu/SHA256SUMS.part
157750561eb3ede7035a83b3aa193a6ac82ef89c10815f6d62714654c1991466  guix-build-aa69fd6caf1b/output/aarch64-linux-gnu/bitcoin-aa69fd6caf1b-aarch64-linux-gnu-debug.tar.gz
7ea854af1d889a55a3858358a80635a8a2968fe7b2859a6d0ccab7d9c93505e6  guix-build-aa69fd6caf1b/output/aarch64-linux-gnu/bitcoin-aa69fd6caf1b-aarch64-linux-gnu.tar.gz
3e15116fdbdf38ae33bd9ff7ba2ccc730046289e2750faa0282f700e5c34793c  guix-build-aa69fd6caf1b/output/arm-linux-gnueabihf/SHA256SUMS.part
33e30217d669565de32d77b1ce2ea4294c4699a204ec4ec4674a999fce3fe16e  guix-build-aa69fd6caf1b/output/arm-linux-gnueabihf/bitcoin-aa69fd6caf1b-arm-linux-gnueabihf-debug.tar.gz
819da671bf3f887f5bd039b332f9fc7d00f7abc3f81b106862ab884da9f1a5f9  guix-build-aa69fd6caf1b/output/arm-linux-gnueabihf/bitcoin-aa69fd6caf1b-arm-linux-gnueabihf.tar.gz
e1a9852821ecfdefde4396b174bbc8ae27ad603bebff7e459a88e78d7b9c568b  guix-build-aa69fd6caf1b/output/dist-archive/bitcoin-aa69fd6caf1b.tar.gz
6e7b721d749a2d4673ec60202ae8897c0f7fe5dadca5f171944323c92fecb604  guix-build-aa69fd6caf1b/output/powerpc64-linux-gnu/SHA256SUMS.part
286f7d7be35197eb171178d41749a01ba748bbc9f31b29db40b0a06f70b12100  guix-build-aa69fd6caf1b/output/powerpc64-linux-gnu/bitcoin-aa69fd6caf1b-powerpc64-linux-gnu-debug.tar.gz
62ae643edb24d56c5b436d4d233c7a6e2a4722a71dfefb57c18009ad3d5f65fe  guix-build-aa69fd6caf1b/output/powerpc64-linux-gnu/bitcoin-aa69fd6caf1b-powerpc64-linux-gnu.tar.gz
b98c329adc97c300a5aca577ebc2f879c738b49a50579bca57da9b318294132b  guix-build-aa69fd6caf1b/output/powerpc64le-linux-gnu/SHA256SUMS.part
9dd09c643db51881c3ec663902ac1925026cbef5821c3c2021a95344a0175e80  guix-build-aa69fd6caf1b/output/powerpc64le-linux-gnu/bitcoin-aa69fd6caf1b-powerpc64le-linux-gnu-debug.tar.gz
1b318eaf652036d86d799740fba602068e58207f8114ad5a21501ef9962781c2  guix-build-aa69fd6caf1b/output/powerpc64le-linux-gnu/bitcoin-aa69fd6caf1b-powerpc64le-linux-gnu.tar.gz
d16d3579f01ef020414461cca6934d1e5b65ebc39e235511c959f9a5cdd050a4  guix-build-aa69fd6caf1b/output/riscv64-linux-gnu/SHA256SUMS.part
00937f0556a723f49a680c0652483bdaabe09a0c15d40b22aa68c82c1a80128d  guix-build-aa69fd6caf1b/output/riscv64-linux-gnu/bitcoin-aa69fd6caf1b-riscv64-linux-gnu-debug.tar.gz
bc7fd09e563e7ad8e6ba345b4c16fccddc3e48a25d51d86131c5a10bae305a2f  guix-build-aa69fd6caf1b/output/riscv64-linux-gnu/bitcoin-aa69fd6caf1b-riscv64-linux-gnu.tar.gz
15074a0b4e6dfb14ad58fc378e36b565432072f21da1090b294488b7e53c1ad7  guix-build-aa69fd6caf1b/output/x86_64-apple-darwin19/SHA256SUMS.part
e357da800c24102dfe304e7c0f895e4cee211fb82aab75bb806335a661aa9a28  guix-build-aa69fd6caf1b/output/x86_64-apple-darwin19/bitcoin-aa69fd6caf1b-osx-unsigned.dmg
5210753c29b8902bd2e53c18d4f662fb47803781da91d7b3d8a4116ff4999643  guix-build-aa69fd6caf1b/output/x86_64-apple-darwin19/bitcoin-aa69fd6caf1b-osx-unsigned.tar.gz
db43dca1e3f209912e2e8e8da4cc8bb54b66a7bfe156b08005a7fc6a7b814b91  guix-build-aa69fd6caf1b/output/x86_64-apple-darwin19/bitcoin-aa69fd6caf1b-osx64.tar.gz
35f022a7349517929b0f139020393861d4ec298fb03a8ec1d8b5bb8463186859  guix-build-aa69fd6caf1b/output/x86_64-linux-gnu/SHA256SUMS.part
4a23170e848fb239e1dda50e1d6945b57d3027000b6af8aa48aa8cc9778ca755  guix-build-aa69fd6caf1b/output/x86_64-linux-gnu/bitcoin-aa69fd6caf1b-x86_64-linux-gnu-debug.tar.gz
7f3d0ecedf46ef4dab7a750d7ce77d62ba165550ea9788ffba1d50f493b4a757  guix-build-aa69fd6caf1b/output/x86_64-linux-gnu/bitcoin-aa69fd6caf1b-x86_64-linux-gnu.tar.gz
a208af491bf757347301d81fd3af94f81322a5790051f06a6f1602394b86a73f  guix-build-aa69fd6caf1b/output/x86_64-w64-mingw32/SHA256SUMS.part
835ed81163c6bc4df7dee17fb9f641efd0a6dac576875e5d8486ddf7240073d9  guix-build-aa69fd6caf1b/output/x86_64-w64-mingw32/bitcoin-aa69fd6caf1b-win-unsigned.tar.gz
c25f542b1ca5acb84bd6b036ab44da3f45011ab5b50b0c48019f609743c7c9fb  guix-build-aa69fd6caf1b/output/x86_64-w64-mingw32/bitcoin-aa69fd6caf1b-win64-debug.zip
6b841d1f8ac23ad7a4effe0130ae0f4b4d53069ab1ad288313a59a2e565e95b4  guix-build-aa69fd6caf1b/output/x86_64-w64-mingw32/bitcoin-aa69fd6caf1b-win64-setup-unsigned.exe
405d7308339bc2d9d10e85ef1c4732d4a6c4ba245b67d015109aedd3ce9a9422  guix-build-aa69fd6caf1b/output/x86_64-w64-mingw32/bitcoin-aa69fd6caf1b-win64.zip

@practicalswift
Copy link
Contributor

Concept ACK

@DrahtBot
Copy link
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:

  • #23191 (build: Disable -Wbraced-scalar-init, which is incompatible with -Wc++11-narrowing by MarcoFalke)

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.

Copy link
Contributor

@meshcollider meshcollider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK aa69fd6

@fanquake fanquake merged commit a845f1c into bitcoin:master Oct 14, 2021
@fanquake fanquake deleted the further_flag_cleanup branch October 14, 2021 13:40
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 14, 2021
aa69fd6 build: Drop -Wno-unused-local-typedef (Hennadii Stepanov)
672e8c5 build: remove -Wunused-variable (fanquake)
5239af0 build: remove -Wswitch (fanquake)
0375906 build: use loop-analysis over range-loop-analysis (fanquake)
12712fa build: remove -Wsign-compare (fanquake)

Pull request description:

  This remove the addition of flags that are already part of other options, such as `-Wall` or `-Wextra`; see each commit message for details. All of the flags being removed here already exist as part of `-Wall` as of GCC 8, or, for Clang, all exist in `-Wmost` (included in `-Wall)`, or as part of `-Wextra` as of Clang 7. Both of which are our minimum required compilers.

  Also cherry-picks one change from bitcoin#21458.

  To give an example of how GCCs `-Wall` has changed over the last few releases:
  ### 11.x to trunk (12.x)
  Added:
  ```bash
  -Wzero-length-bounds
  -Wmismatched-dealloc
  -Wmismatched-new-delete (only for C/C++)
  ```

  ### 10.x to 11.x
  Added:
  ```bash
  -Warray-parameter=2 (C and Objective-C only)
  -Wrange-loop-construct (only for C++)
  -Wsizeof-array-div
  -Wvla-parameter (C and Objective-C only)
  ```

  Removed:
  ```bash
  -Wenum-conversion in C/ObjC;
  ```

  ### 9.x to 10.x
  Added:
  ```bash
  -Wenum-conversion in C/ObjC;
  -Wformat-overflow
  -Wformat-truncation
  -Wzero-length-bounds
  ```

  ### 8.x to 9.x
  Added:
  ```bash
  -Wpessimizing-move
  ```

  Removed:
  ```bash
  -Wstringop-truncation
  ```

  ### 7.x to 8.x
  Added:
  ```bash
  -Wcatch-value (C++ and Objective-C++ only)
  -Wmissing-attributes
  -Wmultistatement-macros
  -Wrestrict
  -Wsizeof-pointer-div
  -Wstringop-truncation
  ```

  [Clang Warning Options](https://clang.llvm.org/docs/DiagnosticsReference.html)
  [GCC Warning Options](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html)

ACKs for top commit:
  meshcollider:
    utACK aa69fd6

Tree-SHA512: 34dde6bd773c864202c151eaa35f902d03fb531c27fe5e1ef659225da03acade2efe5df56df3efb4df5bbded3d395348ce03c25b837fce83be53af3352f0f2bc
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants