Thanks again for the hard work on this patch! It's now almost fine, but there's 
still one issue, which clang-format won't be able to fix. I'd slightly prefer 
that it be fixed before submitting the patch. See the comment inline.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:57
@@ +56,3 @@
+SourceLocation
+forwardSkipWhitespacesAndComments(const SourceManager &SM,
+                                  const clang::ASTContext *Context,
----------------
s/Whitespaces/Whitespace/

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:74
@@ +73,3 @@
+
+} // end anonymous namespace
+
----------------
nit: I'd prefer "// namespace" for consistency with how the 
llvm-namespace-comment check does it.

================
Comment at: clang-tidy/misc/BracesAroundStatementsCheck.cpp:96
@@ +95,3 @@
+    SourceLocation StartLoc =
+        backwardSkipWhitespacesAndComments(
+            *Result.SourceManager, Result.Context, 
WS->getBody()->getLocStart())
----------------
I think, RParenLoc should be stored inside WhileStmt and IfStmt, then this code 
will be much easier. This doesn't have to be in this patch, but please add a 
FIXME.

================
Comment at: test/clang-tidy/misc-braces-around-statements.cpp:147
@@ +146,3 @@
+  // CHECK-FIXES: if (cond("ifif2")) {
+  // CHECK-FIXES: /*comment*/ ;}} // comment
+
----------------
This looks slightly wrong to put a closing brace before a trailing comment, and 
clang-format won't fix this. I think, the check should try to keep the trailing 
comment next to the statement it belongs to. I'm not sure if there can be a 
100% fool-proof solution, but I hope, we can come up with a decent heuristic.

Assuming the coding style is similar to the one used in LLVM, we could do the 
following:

  1. if there's a corresponding "else" or "while", the check should insert "} " 
right before that token.
  2. if there's a multi-line block comment starting on the same line after the 
location we're inserting the closing brace at, or there's a non-comment token, 
the check should insert "} " right before that token.
  3. otherwise the check should find the end of line (possibly after some block 
or line comments) and insert "}\n" right after that EOL. It could also insert 
some spaces to make the indentation nice, but this could be done by 
clang-format as well.

Do you see any problems with this plan?

http://reviews.llvm.org/D5395



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to