build: enable unused member function diagnostic #19846

1. 
   Zero-1729 commented at 2:10 PM on August 31, 2020: contributor

   This PR enables the -Wunused-member-function compiler diagnostic, as discussed in #19702.

   Notice: The unused-member-function diagnostic is only available on clang. Therefore, clang should be used to test this PR.

   • Include the -Wunused-member-functiondiagnostic in ./configure.ac. (ed69213c2b2a99023bdee5168614cb8b71990f5f)
   • Resolve the reported warnings. (819d03b932134ee91df3b0fe98a481a331ce57bf)

   Currently, enabling this flag no longer reports the following warnings:

   Note: output from make 2>&1 | grep "warning: unused member function" | sort | uniq -c

   1 index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
   2 script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
   1 test/util_tests.cpp:1975:14: warning: unused member function 'operator=' [-Wunused-member-function]
   

   All tests have passed locally (from make check & src/test/test_bitcoin).

   This PR closes #19702.

2. build: enable unused member function diagnostic ed69213c2b
3. DrahtBot added the label Build system on Aug 31, 2020
4. 
   practicalswift commented at 2:47 PM on August 31, 2020: contributor

   Concept ACK

   Thanks for doing this @Zero-1729! :)

5. Zero-1729 marked this as ready for review on Sep 8, 2020
6. 
   pox commented at 3:51 PM on September 8, 2020: contributor

   Tested ACK 0791ffd

   Tested by building commit ed69213 (where warning is enabled) with clang 10 and seeing the following warnings:

     CXX      index/libbitcoin_server_a-blockfilterindex.o
   index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
       DBHeightKey() : height(0) {}
       ^
   1 warning 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:1975:14: warning: member function 'operator=' is not needed and will not be emitted [-Wunneeded-member-function]
       Tracker& operator=(Tracker&& t) noexcept
                ^
   
   

   Then rebuilt commit 0791ffd to see warnings are gone. Also searched manually for the removed functions in the codebase to make sure they're not referenced by any uncompiled code.

   Thanks for not squashing the commits and making it easier to recreate the warnings.

7. 
   practicalswift commented at 8:12 PM on September 8, 2020: contributor

   ACK 0791ffda1772930cf3db440333b5bbc559699172 -- patch looks correct

   Thanks for yet another nice contribution @Zero-1729 :)

8. 
   tryphe commented at 7:45 AM on September 16, 2020: contributor

   ACK 0791ffda1772930cf3db440333b5bbc559699172. Neutral on the idea of refactoring and changing the build in one PR, but the changes look good and are simple. Good work!

9. refactor: took out unused member functions

   Took out the following unused member functions:
   
   - 'DBHeightKey()'
   - 'GetType()'
   - 'operator='

   819d03b932
10. Zero-1729 force-pushed on Sep 22, 2020
11. 
    Zero-1729 commented at 11:15 PM on September 23, 2020: contributor

    Not sure why the CI Build for 0791ffda1772930cf3db440333b5bbc559699172 failed, so I squashed the last three commits (0791ffda1772930cf3db440333b5bbc559699172, 819df4ca714a1fc18426a69cd0352857c78a5201, and 9885ced065da082c10f49927d6c4416de3fd359a) into 819d03b932134ee91df3b0fe98a481a331ce57bf and everything now checks out.

    Thanks @practicalswift and @tryphe for reviewing, and @pox for testing :+1:.

12. 
    practicalswift commented at 6:03 AM on September 24, 2020: contributor

    ACK 819d03b932134ee91df3b0fe98a481a331ce57bf - patch still looks correct :)

13. 
    MarcoFalke commented at 6:21 PM on September 28, 2020: member

    ACK 819d03b932134ee91df3b0fe98a481a331ce57bf

14. theStack approved
15. 
    theStack commented at 6:30 PM on October 11, 2020: member

    tested ACK 819d03b932134ee91df3b0fe98a481a331ce57bf Verified that re-running autogen.sh after the first commit adds -Wunused-member-function to the CXXFLAGS and building detects all unused member functions that are removed in the second commit (using clang 3.8.0 here).

16. 
    practicalswift commented at 6:53 PM on October 11, 2020: contributor

    @pox @tryphe Would you mind re-reviewing @Zero-1729's updated version of this PR to refresh your stale ACK:s? :)

17. 
    pox commented at 8:06 PM on October 11, 2020: contributor

    Tested ACK 819d03b932134ee91df3b0fe98a481a331ce57bf with clang after make clean. No unused member function warnings.

18. 
    Zero-1729 commented at 10:27 AM on October 24, 2020: contributor

    @tryphe have you had time to re-review?

19. 
    kristapsk commented at 6:07 PM on October 25, 2020: contributor

    Checked out commit ed69213c2b2a99023bdee5168614cb8b71990f5f, can't reproduce warnings, GCC 8.4.0.

20. 
    theStack commented at 6:21 PM on October 25, 2020: member

    Checked out commit ed69213, can't reproduce warnings, GCC 8.4.0.

    I remember I also tried to reproduce this with GCC first, but then I found out that this diagnostic is only available on clang, not on GCC. (Couldn't find out though which exact clang version introduced it.) Maybe a fact that's worth mentioning in the PR description.

21. 
    Zero-1729 commented at 6:46 PM on October 25, 2020: contributor

    @theStack thanks for pointing that out. I just updated the PR description with a notice to let others know about this.

22. MarcoFalke added this to the milestone 22.0 on Oct 26, 2020
23. 
    Zero-1729 commented at 3:10 PM on October 30, 2020: contributor

    @kristapsk have you tried testing ed69213 with clang?

24. 
    kristapsk commented at 5:02 PM on October 30, 2020: contributor

    @Zero-1729 , no, at least not yet.

25. 
    Zero-1729 commented at 6:03 PM on December 31, 2020: contributor

    @kristapsk did you eventually get round to testing ed69213?

26. MarcoFalke assigned fanquake on Jan 5, 2021
27. MarcoFalke merged this on Jan 5, 2021
28. MarcoFalke closed this on Jan 5, 2021

    ---

29. sidhujag referenced this in commit 18c7f8ab36 on Jan 5, 2021
30. PastaPastaPasta referenced this in commit bdbb9f9930 on Mar 5, 2022
31. PastaPastaPasta referenced this in commit 532a851e06 on Mar 5, 2022
32. PastaPastaPasta referenced this in commit 5ac7ca1296 on Mar 5, 2022
33. DrahtBot locked this on Aug 16, 2022

Milestone
22.0