[PATCH] D16267: Handle C++11 brace initializers in readability-braces-around-statements

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 19 07:41:41 PST 2016


alexfh added inline comments.

================
Comment at: clang-tidy/readability/BracesAroundStatementsCheck.cpp:63-72
@@ -62,2 +62,12 @@
   tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+  // If we are at "}", but the following token is ";", then we could be
+  // reading a statement like "x = A{};" and "}" does not mean we reached
+  // the end of a statement. Hence we look ahead.
+  SourceLocation LocOneAhead = forwardSkipWhitespaceAndComments(Loc.getLocWithOffset(1), SM, Context);
+  tok::TokenKind OneAheadTokKind;
+  if (LocOneAhead.isValid()) {
+    OneAheadTokKind = getTokenKind(LocOneAhead, SM, Context);
+  } else {
+    OneAheadTokKind = tok::NUM_TOKENS;
+  }
   if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
----------------
adek05 wrote:
> alexfh wrote:
> > adek05 wrote:
> > > This might not be the best approach. Maybe it will be better to try to find the matching opening brace and see whether it is the opening brace for 'then' block. Feel free to throw more suggestions.
> > Maybe just skip correct bracket sequences (a statement can also contain lambdas, subscript expressions and normal parentheses in various combinations)?
> Well, we could just remove `TokKind == tok::r_brace` in line 64. I don't really understand why this check is there. The test suite passes if I remove that check, including the new cases I have added. Maybe that's the way to go?
> 
> I tracked the initial commit which introduced `TokKind == tok::r_brace`: D5395 and I don't see a clear reason why it needs to be there. Do you know why we need it?
> Well, we could just remove TokKind == tok::r_brace in line 64.

You mean line 74, I guess. It should be easy to check if removing it helps by running tests and applying fixes to a large enough codebase, e.g. LLVM+Clang, and running its tests to ensure the correctness of changes + manual inspection of a few replacements.


http://reviews.llvm.org/D16267





More information about the cfe-commits mailing list