[PATCH] D74214: [clang-tidy] Fix PR#34798 'readability-braces-around-statements breaks statements containing braces.'

[Karasev Nikita via Phabricator via cfe-commits]

f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

In method 'BracesAroundStatementsCheck::checkStmt' the call 'Lexer::makeFileCharRange(line 253)' return different EndSourceLocation for different statements.

  CharSourceRange FileRange = Lexer::makeFileCharRange(
        CharSourceRange::getTokenRange(S->getSourceRange()), SM,
        Context->getLangOpts());

For example

  class c {};
  c test()
  {
  	if (true)
  		return {};
  	if (true)
  		bool b[2] = { true, true };
  }

In case

  return {};

FileRange.getEnd() will be ';'
In case

  bool b[2] = { true, true };

FileRange.getEnd() will be '\r'

Before call 'findEndLocation' we do this

  const auto FREnd = FileRange.getEnd().getLocWithOffset(-1);

so 'findEndLocation' received '}' in the first case and ';' in the second what breaks the logic of 'findEndLocation'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74214

Files:
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
@@ -204,3 +204,28 @@
   // CHECK-FIXES-NEXT: {{^      ;$}}
   // CHECK-FIXES-NEXT: {{^}$}}
 }
+
+class X {};
+X test_initListExpr()
+{
+  if (cond("x")) return {};
+  else return {};
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("x")) { return {};
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: }
+
+  if (cond("y1"))
+    if (cond("y2") && cond("y3"))
+      return {};
+    else
+      return {};
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("y1")) {
+  // CHECK-FIXES: if (cond("y2") && cond("y3")) {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: }
+}
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -18,19 +18,32 @@
 namespace readability {
 namespace {
 
-tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
-                            const ASTContext *Context) {
-  Token Tok;
+bool tryGetTokenKind(SourceLocation Loc, const SourceManager &SM,
+                     const ASTContext *Context, tok::TokenKind &Kind) {
+  Kind = tok::NUM_TOKENS;
   SourceLocation Beginning =
       Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts());
+
+  Token Tok;
   const bool Invalid =
       Lexer::getRawToken(Beginning, Tok, SM, Context->getLangOpts());
-  assert(!Invalid && "Expected a valid token.");
+  if (!Invalid) {
+    Kind = Tok.getKind();
+  }
+
+  return !Invalid;
+}
+
+tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
+                            const ASTContext *Context) {
+  tok::TokenKind Kind;
+  if (tryGetTokenKind(Loc, SM, Context, Kind)) {
+    return Kind;
+  }
 
-  if (Invalid)
-    return tok::NUM_TOKENS;
+  assert(false && "Expected a valid token.");
 
-  return Tok.getKind();
+  return tok::NUM_TOKENS;
 }
 
 SourceLocation forwardSkipWhitespaceAndComments(SourceLocation Loc,
@@ -58,8 +71,21 @@
   // Loc points to the beginning of the last (non-comment non-ws) token
   // before end or ';'.
   assert(Loc.isValid());
-  bool SkipEndWhitespaceAndComments = true;
   tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+
+  // We need to check that it is not 'InitListExpr' which ends with 
+  // the tokens '};' because it will break the following analysis
+  tok::TokenKind NextTokKind;
+  SourceLocation NextLoc =
+      Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+  if (tryGetTokenKind(NextLoc, SM, Context, NextTokKind)) {
+    if (TokKind == tok::r_brace && NextTokKind == tok::semi) {
+      Loc = NextLoc;
+      TokKind = NextTokKind;
+    }
+  }
+
+  bool SkipEndWhitespaceAndComments = true;
   if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
       TokKind == tok::r_brace) {
     // If we are at ";" or "}", we found the last token. We could use as well


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74214.243131.patch
Type: text/x-patch
Size: 3500 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200207/44a41a29/attachment.bin>