Enable unused member function diagnostic by default

practicalswift commented at 9:25 am on August 12, 2020: contributor

Enable unused member function diagnostic by default.

Clang has a nice compiler diagnostic -Wunused-member-function which is good at catching accidentally dead/unreachable code.

While dead/unreachable code is most often harmless from a security/robustness perspective there are instances where dead/unreachable code may be an indication of more serious issues. Enabling this diagnostic allows us to find such cases and reason about which is which.

See concept ACKs for enabling this diagnostic in #19015.

Current warnings in master that would need to be addressed when enabling this diagnostic:

0index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
1script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]

Useful skills:

Basic understanding of C++. See #19015 for an example on how to enable a compiler diagnostic by editing configure.ac.

Want to work on this issue?

For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.

practicalswift added the label good first issue on Aug 12, 2020

skmcontrib commented at 11:26 am on August 12, 2020: none

@practicalswift can i work on this?

fanquake added the label Build system on Aug 12, 2020

practicalswift commented at 11:57 am on August 12, 2020: contributor

@skmcontrib Yes you can! :) This is a permissionless project: everyone is invited to submit PR:s for review. Welcome!

Looking forward to reviewing your PR!

Zero-1729 commented at 1:33 pm on August 12, 2020: contributor

@practicalswift would there also be a need to enable the -Wunused-template compiler diagnostic flag in ./configure.ac to warn on unused templates by default?

If yes, I can open up a PR.

practicalswift commented at 2:16 pm on August 12, 2020: contributor

@Zero-1729 That would be great, but unfortunately -Wunused-template seems to warn in boost and Qt code as noted by fanquake in #19015 (comment). Perhaps -isystem could be used to exclude those warnings? I’m afraid that introducing -isystem would be quite a bit more work, but please don’t hesitate to give it a try if you want! :) The repo search result for “isystem” might be helpful. Hope you find a good solution! :)

Zero-1729 commented at 4:09 pm on August 12, 2020: contributor

@practicalswift yeah, it would require some work. Thanks for the heads up, if I find a solution I’ll open a PR.

skmcontrib commented at 4:12 am on August 13, 2020: none

@practicalswift Correct me if I’m wrong on what is expected in this issue:

Enable compiler diagnostic -Wunused-member-function and compile
Fix all errors which have the unused member function warning (2 of which you have mentioned)
Undo the change for compiler diagnostic (compiler diagnostic -Wunused-member-function). Based on the reasons you mentioned earlier related to boost and Qt code.
commit and raise PR

practicalswift commented at 10:03 am on August 13, 2020: contributor

@skmcontrib If you skip step 3 everything should be correct. AFAICT we have any boost/Qt issue with Wunused-member-function, so we want to enable it :)

skmcontrib commented at 4:37 pm on August 16, 2020: none

@practicalswift I’m trying to cross compile with macOs target on Ubuntu 20.04 (with clang 10.0 in system path) and getting this error. Not sure if its related to my system or a bug (do let me know if its a bug so I’ll raise an issue). Any thoughts?

make HOST=x86_64-apple-darwin16 FORCE_USE_SYSTEM_CLANG=1 NO_QT=1 NO_WALLET=1 …patience… …patience… …found 1779 targets… …updating 55 targets… clang-linux.compile.c++.without-pch bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/codecvt_error_category.o

“clang++” “-target” “x86_64-apple-darwin16” “-mmacosx-version-min=10.12” “–sysroot” “/home/skm/work/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers” “-stdlib=libc++” “-mlinker-version=530” “-B/home/skm/work/bitcoin/depends/x86_64-apple-darwin16/native/bin” “-nostdinc++” “-isystem” “/home/skm/work/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1” -c -x c++ -fvisibility-inlines-hidden -std=c++11 -fvisibility=hidden -I/home/skm/work/bitcoin/depends/x86_64-apple-darwin16/include -m64 -O3 -Wall -fvisibility=hidden -Wno-inline -DBOOST_ALL_NO_LIB=1 -DBOOST_FILESYSTEM_STATIC_LINK=1 -DNDEBUG -I"." -o “bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/codecvt_error_category.o” “libs/filesystem/src/codecvt_error_category.cpp”

In file included from libs/filesystem/src/codecvt_error_category.cpp:22: In file included from ./boost/filesystem/config.hpp:28: In file included from ./boost/config.hpp:44: ./boost/config/detail/select_stdlib_config.hpp:18:12: fatal error: ‘cstddef’ file not found include ^~~~~~~~~ 1 error generated. …failed clang-linux.compile.c++.without-pch bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/codecvt_error_category.o… clang-linux.compile.c++.without-pch bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/operations.o

“clang++” “-target” “x86_64-apple-darwin16” “-mmacosx-version-min=10.12” “–sysroot” “/home/skm/work/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers” “-stdlib=libc++” “-mlinker-version=530” “-B/home/skm/work/bitcoin/depends/x86_64-apple-darwin16/native/bin” “-nostdinc++” “-isystem” “/home/skm/work/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1” -c -x c++ -fvisibility-inlines-hidden -std=c++11 -fvisibility=hidden -I/home/skm/work/bitcoin/depends/x86_64-apple-darwin16/include -m64 -O3 -Wall -fvisibility=hidden -Wno-inline -DBOOST_ALL_NO_LIB=1 -DBOOST_FILESYSTEM_STATIC_LINK=1 -DNDEBUG -I"." -o “bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/operations.o” “libs/filesystem/src/operations.cpp”

In file included from libs/filesystem/src/operations.cpp:68: In file included from ./boost/filesystem/operations.hpp:18: In file included from ./boost/config.hpp:44: ./boost/config/detail/select_stdlib_config.hpp:18:12: fatal error: ‘cstddef’ file not found include ^~~~~~~~~ 1 error generated. …failed clang-linux.compile.c++.without-pch bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/operations.o… clang-linux.compile.c++.without-pch bin.v2/libs/filesystem/build/clang-linux-10.0.0/release/link-static/target-os-darwin/threading-multi/visibility-hidden/path.o

hebasto commented at 4:43 pm on August 16, 2020: member

@skmcontrib

Check macOS SDK 10.14. Also see #8913.

practicalswift commented at 7:43 am on August 28, 2020: contributor

@skmcontrib Are you actively working on this? If not perhaps @Zero-1729 wants to take a look. Would be really nice to have this enabled by default in master :)

skmcontrib commented at 7:28 am on August 30, 2020: none

Hi @practicalswift I worked on it (and know what to do) but having some difficulty in getting the compilation done with macOS sdk libraries since I don’t have any macs around (trying to cross compile on ubuntu for mac). I did get the libraries from https://github.com/phracker/MacOSX-SDKs to get this going but having similar issues again. If @Zero-1729 can get this fixed faster that’s fine too, whatever works best for the project, I’m all for it.

MarcoFalke closed this on Jan 5, 2021

PastaPastaPasta referenced this in commit bdbb9f9930 on Mar 5, 2022

PastaPastaPasta referenced this in commit 532a851e06 on Mar 5, 2022

PastaPastaPasta referenced this in commit 5ac7ca1296 on Mar 5, 2022

DrahtBot locked this on Aug 16, 2022

Enable unused member function diagnostic by default #19702

Useful skills:

Want to work on this issue?