[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

Florian Gross via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 11 08:16:34 PST 2017


fgross updated this revision to Diff 91473.
fgross added a comment.

Replaced std::map with llvm::DenseMap, added comment about traversal.

I just assumed it would traverse in the "right" way, is there any documentation about AST / matcher traversal?


https://reviews.llvm.org/D30841

Files:
  clang-tidy/readability/MisleadingIndentationCheck.cpp
  clang-tidy/readability/MisleadingIndentationCheck.h
  test/clang-tidy/readability-misleading-indentation.cpp


Index: test/clang-tidy/readability-misleading-indentation.cpp
===================================================================
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
     {
     }
 
+  if(cond1) {
+  }
+  else if (cond2) {
+  }
+  else {
+  }
+
+  if(cond1) {
+  }
+  else if (cond2) {
+  }
+       else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+  
+  if (cond1) {
+    if (cond1) {
+    }
+    else if (cond2) {
+    }
+    else {
+    }
+  }
+  else if (cond2) {
+  }
+
   BLOCK
 }
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===================================================================
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
 
 #include "../ClangTidy.h"
+#include "llvm/ADT/DenseMap.h"
 
 namespace clang {
 namespace tidy {
@@ -32,6 +33,10 @@
 private:
   void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
   void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+
+  /// Key: Chained If
+  /// Value: Preceding If
+  llvm::DenseMap<const IfStmt *, const IfStmt *> ChainedIfs;
 };
 
 } // namespace readability
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===================================================================
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -25,10 +25,22 @@
   if (IfLoc.isMacroID() || ElseLoc.isMacroID())
     return;
 
+  if (const auto *ChainedIf = dyn_cast<IfStmt>(If->getElse())) {
+    if (SM.getExpansionLineNumber(ElseLoc) ==
+        SM.getExpansionLineNumber(ChainedIf->getIfLoc()))
+		ChainedIfs.insert({ ChainedIf, If });
+  }
+
   if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
       SM.getExpansionLineNumber(ElseLoc))
     return;
 
+  // Find location of first 'if' in a 'if else if' chain.
+  // Works because parent nodes will be matched before child nodes.
+  for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end();
+       Iter = ChainedIfs.find(Iter->second))
+    IfLoc = Iter->second->getIfLoc();
+
   if (SM.getExpansionColumnNumber(IfLoc) !=
       SM.getExpansionColumnNumber(ElseLoc))
     diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30841.91473.patch
Type: text/x-patch
Size: 2577 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170311/e8e2b7ac/attachment.bin>


More information about the cfe-commits mailing list