[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