[PATCH] Add check misc-braces-around-statements.

Alexander Kornienko alexfh at google.com
Mon Sep 22 02:57:34 PDT 2014


That's better, but still a few comments.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:18
@@ +17,3 @@
+namespace clang {
+
+namespace tidy {
----------------
Please remove the empty line.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:30
@@ +29,3 @@
+void BracesAroundStatementsCheck::check(
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+  // to put opening brace, get location after closing parenthesis or 'do'
----------------
You can remove the "ast_matchers::" part as there's a "using namespace clang::ast_matchers;" above.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:32
@@ +31,3 @@
+  // to put opening brace, get location after closing parenthesis or 'do'
+  if (const ForStmt *FS = Result.Nodes.getNodeAs<ForStmt>("for")) {
+    checkStmt(Result, FS->getBody(), FS->getRParenLoc().getLocWithOffset(1));
----------------
This is a perfect use case for "auto". The type is spelled on the RHS of the initialization, so you can safely skip it in the LHS.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:33
@@ +32,3 @@
+  if (const ForStmt *FS = Result.Nodes.getNodeAs<ForStmt>("for")) {
+    checkStmt(Result, FS->getBody(), FS->getRParenLoc().getLocWithOffset(1));
+  } else if (const CXXForRangeStmt *FRS =
----------------
I think, you can pass the starting location of the last token to checkStmt and call Lexer::getLocForEndOfToken inside the function. Then all these invocations will look more uniform and the total amount of code is going to be less.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:55
@@ +54,3 @@
+    const Stmt* Else = IS->getElse();
+    if (Else && !IfStmt::classof(Else)) {
+      // omit 'else if' statements here, they will be handled directly
----------------
Any reasons not to use isa<IfStmt>(Else)? This seems to be clearer to me.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:82
@@ +81,3 @@
+  // add braces
+  Diag << FixItHint::CreateInsertion(StartLoc, "{")
+       << FixItHint::CreateInsertion(EndLoc, "}");
----------------
If you insert " {", the result will be one step closer to correct formatting.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.h:1
@@ +1,2 @@
+//===--- BracesAroundStatementsCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
----------------
Please respect the 80 columns limit.

http://reviews.llvm.org/D5395






More information about the cfe-commits mailing list