[PATCH] Add check misc-braces-around-statements.
Alexander Kornienko
alexfh at google.com
Fri Sep 19 01:47:06 PDT 2014
Thanks for working on this check!
Please try the suggestions in my comments and see whether it helps to resolve some of your issues.
================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:71
@@ +70,3 @@
+ //SourceLocation EndLoc = S->getLocEnd().getLocWithOffset(1 + Offset); // passes unittests
+ SourceLocation EndLoc = S->getLocEnd().getLocWithOffset(2); // passes tests
+ auto Diag = diag(S->getLocStart(), "statement should be inside braces");
----------------
Both options are wrong. You don't need to specify Offset explicitly. getLocEnd() returns the location of the first character of the last token, so you need to use Lexer::getLocForEndOfToken(S->getLocEnd()), which will get you a location right after the last token of the statement.
Also, if there's an else/while();, I'd better you put the closing brace right before them in case there are comments between the statement and else/while();.
Addressing this and the next comment will make it possible to maintain the LLVM-like style of arranging braces even without reformatting the changes.
================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:74
@@ +73,3 @@
+ // add braces
+ Diag << FixItHint::CreateInsertion(S->getLocStart(), "{")
+ << FixItHint::CreateInsertion(EndLoc, "}");
----------------
If this check aims to work well for styles similar to the one used in LLVM, it needs to insert the opening brace right after the if()/else/for()/while()/do, in case there's a comment between if()/... and the statement, as clang-format doesn't rearrange tokens.
================
Comment at: test/clang-tidy/misc-braces-around-statements.cpp:11
@@ +10,3 @@
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: statement should be inside braces
+ // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+ // CHECK-FIXES: {do_something("if");}
----------------
We don't usually check these notes, but I don't object, if you leave these checks.
================
Comment at: test/clang-tidy/misc-braces-around-statements.cpp:26
@@ +25,3 @@
+
+#if __cplusplus >= 201100L
+ int arr[4] = {1, 2, 3, 4};
----------------
Any reason for this #if? The check_clang_tidy_fix.sh script always runs clang-tidy with -std=c++11.
http://reviews.llvm.org/D5395
More information about the cfe-commits
mailing list