Enable clang-tidy readability-braces-around-statements check #160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All tests still pass. Signed-off-by: Sean Robinson <[email protected]>
|
✅Static analysis result - no issues found! ✅ |
Signed-off-by: Sean Robinson <[email protected]>
operator& should return combined values of the same type, not a new type (i.e. bool). This is much more verbose, but easier to reason about without implied conversion. Signed-off-by: Sean Robinson <[email protected]>
Enabling this check caught the implicit bool conversion with default_arguments::operator& fixed in the previous commit. Signed-off-by: Sean Robinson <[email protected]>
Adds names for integer bases used with details::parse_number. Signed-off-by: Sean Robinson <[email protected]>
skrobinson
force-pushed
the
wip-tidy
branch
3 times, most recently
from
55293d0 to
9eb1fe5
Compare
Adds names recommended by clang-tidy (e.g. "unused"). Note that clang-tidy v12 appears to detect unnamed parameters in lambdas, while clang-tidy v13 does not. Signed-off-by: Sean Robinson <[email protected]>
Following the clang-tidy suggested fix in consume_digits causes compile failures with MSVC 19.29 in our CI. Signed-off-by: Sean Robinson <[email protected]>
Signed-off-by: Sean Robinson <[email protected]>
Also adds a default destructor, as recommended by the check. Signed-off-by: Sean Robinson <[email protected]>
Surprisingly, no fixes are required in argparse when turning on this suite of checks. Signed-off-by: Sean Robinson <[email protected]>
A common pattern in the previous code was goto/return/throw if a condition is true, else goto/return/throw something different. The new pattern uses the fact that the second goto/return/throw is only reachable when the first goto/return/throw is not called. Signed-off-by: Sean Robinson <[email protected]>
The two functions previously flagged by this check, Argument::is_decimal_literal and Argument::validate, fell below the default Cognitive Complexity threshold as a result of the branch simplification for the readability-else-after-return check. Signed-off-by: Sean Robinson <[email protected]>
Also converts most C-style arrays to a std::array onjects. The check is disabled for ArgumentParser::parse_args(int, const char *const[]) as this is a helper function to convert away from a common input format. Signed-off-by: Sean Robinson <[email protected]>
The "Cpp11" alias is deprecated for Latest (a rolling standard). For now, stick to C++17. Includes end-of-line whitespace removal. Signed-off-by: Sean Robinson <[email protected]>
Most changes are to better fit within "ColumnLimit: 80". The change from "T &&... var" to "T &&...var" is caused by "PointerAlignment: Right". Member functions chained from add_argument() use ContinuationIndentWidth, which is set to 4. Setting ContinuationIndentWidth to 2 causes many other continuation lines to change. So, this commit uses the original value (i.e. 4) as the preferred size. Signed-off-by: Sean Robinson <[email protected]>
This trait is unused and removing it does not cause any tests to fail. Signed-off-by: Sean Robinson <[email protected]>
The new naming pattern is CamelCase for structs, except parse_number as a primarily callable interface. Trait structs are named Has*Traits and constexpr variables to the struct template are named Is*. Signed-off-by: Sean Robinson <[email protected]>
There are no functional changes in this commit. Signed-off-by: Sean Robinson <[email protected]>
|
亲，来信已经收到！祝你快乐！
|
|
Thanks @skrobinson I get three warnings. Did you get these as well?
Looks like they are coming from:
|
|
Interesting... What version of
|
|
I see! The foo:bar$ clang-tidy --version
LLVM (http://llvm.org/):
LLVM version 10.0.0
Optimized build.
Default target: x86_64-pc-linux-gnu
Host CPU: icelake-clientI have no other concerns. I'll merge this. Thanks!! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All tests still pass.
Signed-off-by: Sean Robinson [email protected]