Some const correctness, upgrades to c++17, and lint suggestions #3143
Conversation
| { | ||
| std::cerr << boost::str (boost::format ("Starting generation profiling. Difficulty: %1$#x (%2%x from base difficulty %3$#x)\n") % difficulty % nano::to_string (nano::difficulty::to_multiplier (difficulty, network_constants.publish_full.base), 4) % network_constants.publish_full.base); | ||
| while (!result) |
There was a problem hiding this comment.
This while(!result) and the previous if (!result) didn't make much sense because the code guaranteed result would be untouched and thus equal to 0. So I just ignored the if and added a while (true) instead.
|
I've also removed the |
| @@ -1633,17 +1633,17 @@ void nano::wallets::reload () | |||
| } | |||
| // Delete non existing wallets from memory | |||
| std::vector<nano::wallet_id> deleted_items; | |||
| for (auto i : items) | |||
There was a problem hiding this comment.
I changed the name of i here because it was overshadowing the previously declared i, but perhaps that's not necessary
|
OK I've reviewed my own PR and the changes seem to be correct. I won't be making any more modifications other than possible suggestions from reviews. The only hard to check parts of this PR are the early returns, as most others would not compile if invalid, and because the git diff here on GitHub is a bit messed up. Specially |
marioortizmanero
changed the title
|
Thank you very much for doing this cleanup, the changes look good on brief inspection. I'll figure out when is a good time to include this as it churns a significant amount of files |
|
@theohax We are interested in having you look at the easier of these various changes to remove the conflicts on and clean up for inclusion. |
|
Superseded by #3510 . Thanks again @marioortizmanero for your contribution! |
|
Glad I could be of help! Thanks for cleaning up the PR and fixing the build as well. |
I've been manually reviewing the code for some improvements regarding #3099, #3139 and #297. Here are the main changes:
explicitto constructors with a single parameter. Reference. All of them suggested byclang-tidy.rg 'if \(!error\)'andrg 'if \(!result\)'.Added[[nodiscard]]to getters and similar functions. Reference. All of them suggested byclang-tidy.constto a good chunk of the codebase. I haven't found a way to do this automatically so not all of them are included yet. I've enforced that all of them are in theT constorder, as it was the most commonly used one. Unfortunately, this can't be checked automatically withclang-formatfor now.const Tparameters in headers, as they have no effect. Reference. All of them suggested byclang-tidy.I'm not adding more changes related to early returns because there are so many lines modified already and I wanted to keep the PR smaller. If it's too much I can split it up or something. Sorry about that.