boinc
boinc copied to clipboard
Remove unused functions
Description of the Change Browsing through the code, I have found several functions defined in header files that Visual Studio found did not have an associated function nor inline code. Since these functions do not exist, this PR removes them.
I have broken up the PR into three commits; each commit is for a subfolder (client, manager, and lib).
This PR does not clean up ALL unused functions. There were some functions that were empty constructors that I didn't feel comfortable removing. Another thing I discovered were some functions were noted by Visual Studio as not defined, but the function was written in one or more preprocessor definitions. With that said, I tried to carefully remove only functions that were obvious to me that weren't defined. Thank you in advance for reviewing!
Oops, I don't see how I missed that Start function, but it makes sense now. I'll fix that when I'm able to.
Please do not remove the following line from MainDocument.h:
// void TestAsyncRPC(); // For testing Async RPCs
The comment clearly states that this line has been left in the code but disabled with // to allow easily re-enabling it for additional testing when needed. The corresponding function is in AsyncRPC.cpp and disabled by #if 0 and #endif, allowing for easy re-enabling when needed.
Visual Studio found did not have an associated function nor inline code.
Since this line is disabled with //, I don't see how the compiler would mark it as a declaration with no corresponding function.
Also, please use caution when removing declarations, because some functions are used only by builds for certain operations systems (Windows, MacOS, Linux, Android, etc.) Ideally both the header declaration and function definition will be guarded by something like #ifdef _WIN32 or #ifdef __WXMAC__, but there might be instances where that guard is missing from the header declaration. That situation would also make the compiler think there is a header declaration with no associated function. Hopefully, the CI build will fail if you remove a declaration needed when building for a different operating system.
Please do not remove the following line from MainDocument.h:
// void TestAsyncRPC(); // For testing Async RPCsThe comment clearly states that this line has been left in the code but disabled with//to allow easily re-enabling it for additional testing when needed. The corresponding function is in AsyncRPC.cpp and disabled by#if 0and#endif, allowing for easy re-enabling when needed.
Understood, apologies that I missed that the first time around.
Since this line is disabled with
//, I don't see how the compiler would mark it as a declaration with no corresponding function.
It didn't find it that way, I just thought it was commented out for not being used, but I was too quick with the delete key for that one. This was an exception to how I was targeting unused functions. Again, apologies.
Also, please use caution when removing declarations, because some functions are used only by builds for certain operations systems (Windows, MacOS, Linux, Android, etc.) Ideally both the header declaration and function definition will be guarded by something like
#ifdef _WIN32or#ifdef __WXMAC__, but there might be instances where that guard is missing from the header declaration. That situation would also make the compiler think there is a header declaration with no associated function. Hopefully, the CI build will fail if you remove a declaration needed when building for a different operating system.
Yes, I did see some of those, and for the most part I left them alone because I didn't want to break something I didn't fully understand. For all other functions I did painstakingly search through the entire code to see if the function was used. If I didn't find it anywhere else, I removed it. There are some instances where a function was marked as not used, but something else was happening (different input variables, or preprocessor declarations for example), and I left those alone since it wasn't just removing a declaration.
The only declaration I see right now I have removed is part of a preprocessor is here:
https://github.com/BOINC/boinc/blob/be884908dc15de5c3d75739f9174548a18c7fc5f/client/project.h#L390
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 10.78%. Comparing base (
dca4153) to head (aa35989). Report is 18 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #5850 +/- ##
============================================
+ Coverage 10.51% 10.78% +0.26%
Complexity 1068 1068
============================================
Files 280 280
Lines 36021 36448 +427
Branches 8441 8441
============================================
+ Hits 3789 3930 +141
- Misses 31843 32129 +286
Partials 389 389
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/app_ipc.h | 0.00% <ø> (ø) |
|
| lib/common_defs.h | 0.00% <ø> (ø) |
|
| lib/gui_rpc_client.h | 0.00% <ø> (ø) |
|
| lib/prefs.h | 0.00% <ø> (ø) |
|
| lib/str_util.h | 0.00% <ø> (ø) |
Okay, take two. I think this is ready for review now.
Ok, it was not easy. Looks good to me.
Thank you for reviewing, @AenBleidd. I'm sure it was harder than it seemed.