Fix a number of low-effort warnings #6641
Conversation
Some of the sub-warnings are still disabled.
Our only change is to rename a var from `requires` (which is a C++20 keyword) to `dependencies`.
Most re-enabled diagnostics have been removed. The remaining ones are tied to diagnostics that require deeper changes. This makes the parent diagnostics explicit so it’s easier to identified when the remaining re-enabled flags should be removed from the list.
Applied early, because editing files fixes this issue, so it’s fixed explicitly before it’s fixed implicitly.
These are a C++20 feature that Clang makes available in C++17. We can keep them, IMO, depending on how strict we want to be with our C++17 compliance.
`-Wcast-qual` warns when `const` or `volatile` is discarded in a cast. This necessitated some more significant changes - added `UNFLATDATA` to allow `FLATDATA` to work on `const` structures and - added non-const `begin` operations to `CKey` and `CPubKey` to support populating those structures.
No longer need a `default` case if a `switch` covers the full `enum`.
The one part of this I am unhappy with is that torcontrol.cpp is included as a header, and so I down-scoped the `using namespace` clauses.
According to [the Clang documentation](https://releases.llvm.org/15.0.0/tools/clang/docs/DiagnosticsReference.html#wsuggest-destructor-override) this flag doesn’t control any other flags, but it appears to actually control `-Winconsistent-missing-override`, so this also disables that flag for now.
This should be tied to (or at least merged no later than) enabling `-Wcovered-switch-default`, because if that gets in then we’ll get warnings when we use an unnecessary `default`, but not when we’re missing a necessary `default`, which can bite us when we add cases to an `enum`. I took a bit of liberty here to improve the readability of `PrivacyPolicyMeet`.
|
This PR is likely to bitrot if left (because it touches a lot of files), so perhaps we should get it in soon to avoid repeating work. |
Agreed. It depends on #6636, which I just updated now that #6673 is merged. ZenHub isn’t working for me at the moment for some reason, so I don’t know if the blocking edge is already there. This PR is also pretty malleable. It can be split between any commits, we can take an arbitrary subset of commits into a new PR and postpone/skip the rest, etc. I figure people can either go commit by commit and decide which ones need more thorough discussion, or go through it holistically and then we can decide which things to postpone based on complaints about individual changes. Although, untli #6636 is merged, it’s hard to go through this holistically. |
This enables another pile of disabled warnings, leaving us with a much shorter list that mostly require either more thinking or more involved changes that should be in their own PRs.