[llvm-branch-commits] [clang] 075e262 - [ASTMatchers] Avoid pathological traversal over nested lambdas

[Tom Stellard via llvm-branch-commits]

Author: Stephen Kelly
Date: 2021-02-15T18:58:35-08:00
New Revision: 075e2629b02d194313a069ce1ee9a8d40c7bd66f

URL: https://github.com/llvm/llvm-project/commit/075e2629b02d194313a069ce1ee9a8d40c7bd66f
DIFF: https://github.com/llvm/llvm-project/commit/075e2629b02d194313a069ce1ee9a8d40c7bd66f.diff

LOG: [ASTMatchers] Avoid pathological traversal over nested lambdas

Differential Revision: https://reviews.llvm.org/D95573

(cherry picked from commit 6f0df3cddb3e3f38df1baa7aa4d743a74bb46688)

Added: 
    

Modified: 
    clang/include/clang/AST/RecursiveASTVisitor.h
    clang/lib/ASTMatchers/ASTMatchFinder.cpp
    clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 505ea700fd0e..db2ef21f4364 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -186,6 +186,9 @@ template <typename Derived> class RecursiveASTVisitor {
   /// code, e.g., implicit constructors and destructors.
   bool shouldVisitImplicitCode() const { return false; }
 
+  /// Return whether this visitor should recurse into lambda body
+  bool shouldVisitLambdaBody() const { return true; }
+
   /// Return whether this visitor should traverse post-order.
   bool shouldTraversePostOrder() const { return false; }
 
@@ -2057,6 +2060,14 @@ bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
       // by clang.
       (!D->isDefaulted() || getDerived().shouldVisitImplicitCode());
 
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) {
+    if (const CXXRecordDecl *RD = MD->getParent()) {
+      if (RD->isLambda()) {
+        VisitBody = VisitBody && getDerived().shouldVisitLambdaBody();
+      }
+    }
+  }
+
   if (VisitBody) {
     TRY_TO(TraverseStmt(D->getBody())); // Function body.
   }

diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 8ddd3c87e09d..5034203840fc 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -556,9 +556,9 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
         if (LE->hasExplicitResultType())
           TraverseTypeLoc(Proto.getReturnLoc());
         TraverseStmt(LE->getTrailingRequiresClause());
-
-        TraverseStmt(LE->getBody());
       }
+
+      TraverseStmt(LE->getBody());
       return true;
     }
     return RecursiveASTVisitor<MatchASTVisitor>::dataTraverseNode(S, Queue);
@@ -697,6 +697,10 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
   bool shouldVisitTemplateInstantiations() const { return true; }
   bool shouldVisitImplicitCode() const { return true; }
 
+  // We visit the lambda body explicitly, so instruct the RAV
+  // to not visit it on our behalf too.
+  bool shouldVisitLambdaBody() const { return false; }
+
   bool IsMatchingInASTNodeNotSpelledInSource() const override {
     return TraversingASTNodeNotSpelledInSource;
   }

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 92bf244b0e4a..8004599e01a2 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3853,6 +3853,78 @@ void binop()
   }
 }
 
+TEST(IgnoringImpCasts, PathologicalLambda) {
+
+  // Test that deeply nested lambdas are not a performance penalty
+  StringRef Code = R"cpp(
+void f() {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+  [] {
+    int i = 42;
+    (void)i;
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+  }();
+}
+  )cpp";
+
+  EXPECT_TRUE(matches(Code, integerLiteral(equals(42))));
+  EXPECT_TRUE(matches(Code, functionDecl(hasDescendant(integerLiteral(equals(42))))));
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.