[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