[clang-tools-extra] r247163 - [clang-tidy] Fix PR22785.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep  9 12:06:09 2015
New Revision: 247163

URL: http://llvm.org/viewvc/llvm-project?rev=247163&view=rev
Log:
[clang-tidy] Fix PR22785.

Fix http://llvm.org/PR22785. Bug 22785 - readability-braces-around-statements
doesn't work well with macros.

http://reviews.llvm.org/D12729

Patch by Marek Kurdej!

Modified:
    clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements.cpp
    clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp?rev=247163&r1=247162&r2=247163&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp Wed Sep  9 12:06:09 2015
@@ -172,7 +172,7 @@ SourceLocation
 BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
                                            const SourceManager &SM,
                                            const ASTContext *Context) {
-  // Skip macros
+  // Skip macros.
   if (S->getLocStart().isMacroID())
     return SourceLocation();
 
@@ -219,13 +219,17 @@ bool BracesAroundStatementsCheck::checkS
     // Already inside braces.
     return false;
   }
-  // Skip macros.
-  if (S->getLocStart().isMacroID())
-    return false;
 
   const SourceManager &SM = *Result.SourceManager;
   const ASTContext *Context = Result.Context;
 
+  // Treat macros.
+  CharSourceRange FileRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(S->getSourceRange()), SM,
+      Context->getLangOpts());
+  if (FileRange.isInvalid())
+    return false;
+
   // InitialLoc points at the last token before opening brace to be inserted.
   assert(InitialLoc.isValid());
   SourceLocation StartLoc =
@@ -237,7 +241,8 @@ bool BracesAroundStatementsCheck::checkS
     EndLoc = EndLocHint;
     ClosingInsertion = "} ";
   } else {
-    EndLoc = findEndLocation(S->getLocEnd(), SM, Context);
+    const auto FREnd = FileRange.getEnd().getLocWithOffset(-1);
+    EndLoc = findEndLocation(FREnd, SM, Context);
     ClosingInsertion = "\n}";
   }
 

Modified: clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements.cpp?rev=247163&r1=247162&r2=247163&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements.cpp Wed Sep  9 12:06:09 2015
@@ -171,3 +171,16 @@ void test() {
   // CHECK-FIXES-NEXT: }
   // CHECK-FIXES-NEXT: }
 }
+
+#define M(x) x
+
+int test_macros(bool b) {
+  if (b) {
+    return 1;
+  } else
+    M(return 2);
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should be inside braces
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES-NEXT:   M(return 2);
+  // CHECK-FIXES-NEXT: }
+}

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp?rev=247163&r1=247162&r2=247163&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp Wed Sep  9 12:06:09 2015
@@ -457,6 +457,27 @@ TEST(BracesAroundStatementsCheck, Macros
                     "int main() {\n"
                     "  FOR(;;)\n"
                     "}");
+  EXPECT_EQ("#define DO_IT ++i\n"
+            "int i = 0;\n"
+            "int main() {\n"
+            "  if (false) {\n"
+            "    DO_IT;\n"
+            "  } else if (1 == 2) {\n"
+            "    DO_IT;\n"
+            "  } else {\n"
+            "    DO_IT;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("#define DO_IT ++i\n"
+                                                        "int i = 0;\n"
+                                                        "int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    DO_IT;\n"
+                                                        "  else if (1 == 2)\n"
+                                                        "    DO_IT;\n"
+                                                        "  else\n"
+                                                        "    DO_IT;\n"
+                                                        "}"));
 }
 
 } // namespace test