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

2020-02-29 Thread Alexander Lanin via Phabricator via cfe-commits
AlexanderLanin added a comment.

Here is the r_brace use case:

  if (true) while ("ok") {}

Unfortunately it looks quite similar to:

  if (true) s = {};


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D16267/new/

https://reviews.llvm.org/D16267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-01-19 Thread Alexander Kornienko via cfe-commits
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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits