build: Enable some commonly enabled compiler diagnostics #19015

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:compiler-diagnostics changing 1 files +4 −0
  1. practicalswift commented at 12:39 PM on May 19, 2020: contributor

    Enable some commonly enabled compiler diagnostics as discussed in #17344.

    Compiler diagnostic no# of emitted unique GCC warnings in master no# of emitted unique Clang warnings in master
    -Wduplicated-branches: Warn if if/else branches have duplicated code 0 Not supported
    -Wduplicated-cond: Warn if if/else chain has duplicated conditions 0 Not supported
    -Wlogical-op: Warn about logical operations being used where bitwise were probably wanted 0 Not supported
    -Woverloaded-virtual: Warn if you overload (not override) a virtual function 0 0
    -Wunused-member-function: Warn on unused member function Not supported 2
    -Wunused-template: Warn on unused template Not supported 1

    There is a large overlap between this list and Jason Turner's list of recommended compiler diagnostics in the Collaborative Collection of C++ Best Practices (cppbestpractices) project. There is also an overlap with the recommendations given in the C++ Core Guidelines (with editors Bjarne Stroustrup and Herb Sutter).

    Closes #17344.

  2. fanquake added the label Build system on May 19, 2020
  3. MarcoFalke commented at 1:10 PM on May 19, 2020: member

    Concept ACK, but before merge we should check that there are no warnings. This includes warnings in headers of third-party libraries. Boost and Qt have a good track record of producing warnings.

  4. hebasto commented at 4:18 PM on May 19, 2020: member

    Concept ACK.

  5. MarcoFalke added the label Needs Guix build on May 21, 2020
  6. DrahtBot commented at 1:34 AM on May 22, 2020: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18857 (build: avoid repetitions when enabling warnings in configure.ac by vasild)

    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.

  7. DrahtBot removed the label Needs Guix build on May 23, 2020
  8. MarcoFalke added the label Needs Guix build on May 23, 2020
  9. DrahtBot removed the label Needs Guix build on May 25, 2020
  10. MarcoFalke added the label Needs gitian build on May 26, 2020
  11. MarcoFalke deleted a comment on May 26, 2020
  12. DrahtBot removed the label Needs gitian build on May 28, 2020
  13. MarcoFalke deleted a comment on May 28, 2020
  14. MarcoFalke added the label Needs gitian build on May 28, 2020
  15. DrahtBot removed the label Needs gitian build on May 29, 2020
  16. jonathanschoeller referenced this in commit eea8114657 on Jun 1, 2020
  17. laanwj referenced this in commit b46fb5cb10 on Jun 4, 2020
  18. hebasto commented at 4:03 PM on June 4, 2020: member

    @practicalswift Why this PR does not include changes to get rid of new warnings?

    master (39afe5b1c68c5979b2ef2f03b60ec6a57901328f) + this PR:

    $ make -j 9 > /dev/null 
script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
    int GetType() const { return m_type; }
        ^
1 warning generated.
index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
    DBHeightKey() : height(0) {}
    ^
index/blockfilterindex.cpp:80:5: warning: unused function template 'Unserialize' [-Wunused-template]
    SERIALIZE_METHODS(DBHashKey, obj) {
    ^
./serialize.h:215:10: note: expanded from macro 'SERIALIZE_METHODS'
    void Unserialize(Stream& s)                                                     \
         ^
index/blockfilterindex.cpp:80:5: warning: unused function template 'Unser' [-Wunused-template]
./serialize.h:220:5: note: expanded from macro 'SERIALIZE_METHODS'
    FORMATTER_METHODS(cls, obj)
    ^
./serialize.h:196:17: note: expanded from macro 'FORMATTER_METHODS'
    static void Unser(Stream& s, cls& obj) { SerializationOps(obj, s, CSerActionUnserialize()); } \
                ^
3 warnings generated.
script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
    int GetType() const { return m_type; }
        ^
1 warning generated.
test/util_tests.cpp:1972:14: warning: member function 'operator=' is not needed and will not be emitted [-Wunneeded-member-function]
    Tracker& operator=(Tracker&& t) noexcept
             ^
1 warning generated.
  19. sidhujag referenced this in commit 2ab90a2fe4 on Jun 4, 2020
  20. practicalswift commented at 7:35 AM on June 5, 2020: contributor

    @hebasto Thanks for testing! I wanted to keep this PR build-system only to keep review simple. Feel free to tackle the warnings in a separate PR if you want :)

  21. MarcoFalke deleted a comment on Jun 5, 2020
  22. MarcoFalke deleted a comment on Jun 5, 2020
  23. MarcoFalke added the label Waiting for author on Jun 5, 2020
  24. practicalswift commented at 10:49 AM on June 7, 2020: contributor

    @MarcoFalke I don't think this one needs rebase - anything else that you had in mind with the "waiting for author" label? :)

  25. MarcoFalke commented at 10:55 AM on June 7, 2020: member

    -Wunreachable-code-loop-increment has been merged (silent merge conflict), so it needs to be removed (with or without a rebase)

  26. practicalswift force-pushed on Jun 7, 2020
  27. practicalswift commented at 7:40 PM on June 7, 2020: contributor

    @MarcoFalke Oh, thanks for clarifying! Now fixed :)

  28. MarcoFalke removed the label Waiting for author on Jun 7, 2020
  29. practicalswift commented at 2:44 PM on June 8, 2020: contributor

    The Travis TSan job failure is unrelated but I'm unable to re-run the job :)

  30. MarcoFalke commented at 2:53 PM on June 8, 2020: member

    Another rebase would make it rerun

  31. practicalswift force-pushed on Jun 8, 2020
  32. stackman27 referenced this in commit 2d9b7d8233 on Jun 26, 2020
  33. MarcoFalke added the label Needs gitian build on Jul 2, 2020
  34. DrahtBot removed the label Needs gitian build on Jul 3, 2020
  35. fanquake commented at 11:17 AM on August 10, 2020: member

    Concept ACK

    Thanks for testing! I wanted to keep this PR build-system only to keep review simple. Feel free to tackle the warnings in a separate PR if you want :)

    There's not really a rush to do this, and it's unlikely this will be merged while it introduces any new (presumably easy to fix?) warnings. So if we're going to turn new diagnostics on, let's just take care of any warnings now, or, if you'd rather not deal with code changes, please reduce the scope of this PR.

    I checked out this branch and started building. Spamming the log with the following is not going to be useful, and in this case it's coming from the brew installed Boost:

    Options used to compile and link:
  multiprocess  = no
  with wallet   = yes
  with gui / qt = yes
    with qr     = yes
  with zmq      = yes
  with test     = yes
    with fuzz   = no
  with bench    = yes
  with upnp     = yes
  use asm       = yes
  sanitizers    = 
  debug enabled = no
  gprof enabled = no
  werror        = no

  target os     = darwin
  build os      = darwin19.6.0

  CC            = /usr/local/bin/ccache gcc
  CFLAGS        = -g -O2
  CPPFLAGS      =   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -I/usr/local/opt/berkeley-db@4/include -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0
  CXX           = /usr/local/bin/ccache g++ -std=c++11
  CXXFLAGS      =   -Wstack-protector -fstack-protector-all  -Wall -Wextra -Wgnu -Wformat -Wvla -Wshadow-field -Wswitch -Wformat-security -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunused-variable -Wdate-time -Wconditional-uninitialized -Wsign-compare -Woverloaded-virtual -Wunused-member-function -Wunused-template -Wunreachable-code-loop-increment  -Wno-unused-parameter -Wno-self-assign -Wno-unused-local-typedef -Wno-deprecated-register -Wno-implicit-fallthrough   -g -O2
  LDFLAGS       = -pthread  -Wl,-bind_at_load   -Wl,-headerpad_max_install_names -Wl,-dead_strip -Wl,-dead_strip_dylibs
  ARFLAGS       = cr

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C src bitcoind
  CXX      bitcoind-bitcoind.o
  CXX      libbitcoin_server_a-addrdb.o
  CXX      libbitcoin_server_a-addrman.o
  CXX      libbitcoin_server_a-banman.o
  CXX      libbitcoin_server_a-blockencodings.o
  CXX      libbitcoin_server_a-blockfilter.o
  CXX      libbitcoin_server_a-chain.o
  CXX      libbitcoin_server_a-flatfile.o
  CXX      libbitcoin_server_a-httprpc.o
  CXX      libbitcoin_server_a-httpserver.o
  CXX      libbitcoin_server_a-init.o
  CXX      libbitcoin_server_a-dbwrapper.o
  CXX      libbitcoin_server_a-miner.o
  CXX      libbitcoin_server_a-net.o
  CXX      libbitcoin_server_a-net_processing.o
In file included from blockencodings.cpp:13:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.
  CXX      libbitcoin_server_a-noui.o
  CXX      libbitcoin_server_a-pow.o
  CXX      libbitcoin_server_a-rest.o
  CXX      libbitcoin_server_a-shutdown.o
  CXX      libbitcoin_server_a-timedata.o
  CXX      libbitcoin_server_a-torcontrol.o
  CXX      libbitcoin_server_a-txdb.o
In file included from miner.cpp:6:
In file included from ./miner.h:11:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.
  CXX      libbitcoin_server_a-txmempool.o
  CXX      libbitcoin_server_a-ui_interface.o
  CXX      libbitcoin_server_a-validation.o
In file included from rest.cpp:19:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.
  CXX      libbitcoin_server_a-validationinterface.o
  CXX      libbitcoin_server_a-versionbits.o
  CXX      interfaces/libbitcoin_wallet_a-wallet.o
  CXX      wallet/libbitcoin_wallet_a-coincontrol.o
In file included from txmempool.cpp:6:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.
  CXX      wallet/libbitcoin_wallet_a-context.o
  CXX      wallet/libbitcoin_wallet_a-crypter.o
  CXX      wallet/libbitcoin_wallet_a-db.o
In file included from init.cpp:28:
In file included from ./miner.h:11:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.
  CXX      wallet/libbitcoin_wallet_a-feebumper.o
  CXX      wallet/libbitcoin_wallet_a-fees.o
  CXX      wallet/libbitcoin_wallet_a-load.o
  CXX      wallet/libbitcoin_wallet_a-rpcdump.o
In file included from net_processing.cpp:16:
In file included from ./validation.h:22:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.

> Killed build.
  36. MarcoFalke deleted a comment on Aug 10, 2020
  37. MarcoFalke added the label Waiting for author on Aug 10, 2020
  38. practicalswift commented at 9:05 AM on August 11, 2020: contributor

    @fanquake Thanks for testing, and your feedback makes sense: I'll adjust accordingly. I don't have any macOS machine to test on -- would you mind sharing the results of make 2>&1 | grep "warning: " | sort | uniq -c? :)

  39. fanquake commented at 11:52 AM on August 11, 2020: member

    I don't have any macOS machine to test on

    You should just be able to just check the Travis output:

    Making install in src
< snip >
  CXX      libbitcoin_server_a-init.o
In file included from blockencodings.cpp:13:
In file included from ./txmempool.h:26:
In file included from /usr/local/include/boost/multi_index_container.hpp:32:
In file included from /usr/local/include/boost/multi_index_container_fwd.hpp:19:
In file included from /usr/local/include/boost/multi_index/ordered_index_fwd.hpp:16:
In file included from /usr/local/include/boost/multi_index/detail/ord_index_args.hpp:21:
In file included from /usr/local/include/boost/multi_index/tag.hpp:17:
In file included from /usr/local/include/boost/multi_index/detail/no_duplicate_tags.hpp:18:
In file included from /usr/local/include/boost/mpl/set/set0.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/at_impl.hpp:18:
In file included from /usr/local/include/boost/mpl/set/aux_/has_key_impl.hpp:21:
In file included from /usr/local/include/boost/mpl/aux_/overload_names.hpp:17:
/usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
template< typename T > static T const& ptr_to_ref(T*);
                                       ^
1 warning generated.

    make 2>&1 | grep "warning: " | sort | uniq -c

      12 /usr/local/Cellar/qt/5.15.0/lib/QtCore.framework/Headers/qtestsupport_core.h:55:31: warning: unused function template 'qWaitFor' [-Wunused-template]
 137 /usr/local/include/boost/mpl/aux_/ptr_to_ref.hpp:42:40: warning: unused function template 'ptr_to_ref' [-Wunused-template]
   1 index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
   1 index/blockfilterindex.cpp:80:5: warning: unused function template 'Unser' [-Wunused-template]
   1 index/blockfilterindex.cpp:80:5: warning: unused function template 'Unserialize' [-Wunused-template]
   1 qt/bitcoin.cpp:259:53: warning: 'QFlags' is deprecated: Use default constructor instead [-Wdeprecated-declarations]
   1 qt/bitcoingui.cpp:1304:29: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/optionsmodel.cpp:222:59: warning: 'split' is deprecated: Use Qt::SplitBehavior variant instead [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:100:12: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:105:45: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:121:9: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:132:9: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:139:9: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/qrimagewidget.cpp:98:9: warning: 'pixmap' is deprecated: Use the other overload which returns QPixmap by-value [-Wdeprecated-declarations]
   1 qt/sendcoinsdialog.cpp:174:72: warning: 'buttonClicked' is deprecated: Use QButtonGroup::idClicked(int) instead [-Wdeprecated-declarations]
   1 qt/sendcoinsdialog.cpp:175:72: warning: 'buttonClicked' is deprecated: Use QButtonGroup::idClicked(int) instead [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:240:85: warning: 'split' is deprecated: Use Qt::SplitBehavior variant instead [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:278:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:285:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:291:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:296:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:297:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:301:17: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:586:13: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 qt/transactionview.cpp:587:13: warning: 'QDateTime' is deprecated: Use QDate::startOfDay() [-Wdeprecated-declarations]
   1 random.cpp:257:13: warning: unused function 'GetDevURandom' [-Wunused-function]
   2 script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
   1 test/util_tests.cpp:1972:14: warning: unused member function 'operator=' [-Wunused-member-function]
  40. build: Enable some commonly enabled compiler diagnostics 2f8a4c9a06
  41. practicalswift force-pushed on Aug 11, 2020
  42. practicalswift commented at 12:10 PM on August 11, 2020: contributor

    I don't have any macOS machine to test on

    You should just be able to just check the Travis output:

    Oh, of course! Thanks! :)

    I've now updated the PR to only enable -Wduplicated-branches, -Wduplicated-cond, -Wlogical-op and -Woverloaded-virtual. None of these should warn on current master.

    Should be ready for final review :)

  43. fanquake removed the label Waiting for author on Aug 14, 2020
  44. hebasto approved
  45. hebasto commented at 1:41 PM on August 15, 2020: member

    ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909, no new warnings in Travis jobs.

    Are there any reasons for not adding corresponding -Werror?

  46. jonatack commented at 3:31 PM on August 15, 2020: member

    ACK 2f8a4c9a06ace6 no warnings for me with these locally on debian 5.7.10-1 (2020-07-26) x86_64 with gcc 10 and clang 12

  47. fanquake approved
  48. fanquake commented at 4:59 AM on August 18, 2020: member

    ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909 - no-longer seeing any obvious issues with doing this.

  49. fanquake merged this on Aug 18, 2020
  50. fanquake closed this on Aug 18, 2020
  51. sidhujag referenced this in commit 64f37492b7 on Aug 18, 2020
  52. practicalswift commented at 4:57 AM on September 8, 2020: contributor

    A post-merge success story: only 21 days after merge -Wlogical-op caught the first bug in the form of #19912 :)

    wallet/scriptpubkeyman.cpp:455:55: warning: logical ‘and’ applied to non-boolean constant [-Wlogical-op]
  455 |     if (m_storage.CanSupportFeature(FEATURE_HD_SPLIT) && CHDChain::VERSION_HD_CHAIN_SPLIT) {
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    Thanks a lot for reporting @kristapsk! ❤️

    Perhaps we should do -Werror=logical-op.

  53. practicalswift deleted the branch on Apr 10, 2021
  54. Fabcien referenced this in commit 5c2868f3d5 on Sep 15, 2021
  55. Fabcien referenced this in commit 6fc14008fb on Sep 15, 2021
  56. DrahtBot locked this on Aug 18, 2022
Contributors
practicalswiftMarcoFalkehebastoDrahtBotfanquakejonatack
Labels
Build system
Linked (view graph)
#17344 RFC: Enabling some commonly enabled compiler diagnostics#19017 Fix -Wunreachable-code-loop-increment warning#19131 refactor: Fix unreachable code in init arg checks#19702 Enable unused member function diagnostic by default#19912 -Wlogical-op warning in wallet/scriptpubkeyman.cpp when building current master
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-27 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me