Prevent assert in ASTMatchFinder.

If nodes without memoization data (e.g. TypeLocs) are bound to specific
names, that effectively prevents memoization as those elements cannot be
compared effectively. If it is tried anyway, this can lead to an assert
as demonstrated in the new test.

In the long term, the better solution will be to enable DynTypedNodes
without memoization data. For now, simply skip memoization instead.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@213751 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h
index 94435fd274eb7442486bec63ac52a240dd942180..68b18164c236a70e1da82054a1e28c5c8ebe051a 100644 (file)
--- a/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -103,6 +103,16 @@ public:
     return NodeMap;
   }
 
+  /// \brief Returns \c true if this \c BoundNodesMap can be compared, i.e. all
+  /// stored nodes have memoization data.
+  bool isComparable() const {
+    for (const auto &IDAndNode : NodeMap) {
+      if (!IDAndNode.second.getMemoizationData())
+        return false;
+    }
+    return true;
+  }
+
 private:
   IDToNodeMap NodeMap;
 };
@@ -153,6 +163,16 @@ public:
     return Bindings < Other.Bindings;
   }
 
+  /// \brief Returns \c true if this \c BoundNodesTreeBuilder can be compared,
+  /// i.e. all stored node maps have memoization data.
+  bool isComparable() const {
+    for (const BoundNodesMap &NodesMap : Bindings) {
+      if (!NodesMap.isComparable())
+        return false;
+    }
+    return true;
+  }
+
 private:
   SmallVector<BoundNodesMap, 16> Bindings;
 };

diff --git a/lib/ASTMatchers/ASTMatchFinder.cpp b/lib/ASTMatchers/ASTMatchFinder.cpp
index 23708e2ff0c77acbdf24034fe97d09e3ec9e377e..b8ac68aad72118454eeace1dc0fbfe3bb5bb567e 100644 (file)
--- a/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -372,7 +372,7 @@ public:
                                   BoundNodesTreeBuilder *Builder, int MaxDepth,
                                   TraversalKind Traversal, BindKind Bind) {
     // For AST-nodes that don't have an identity, we can't memoize.
-    if (!Node.getMemoizationData())
+    if (!Node.getMemoizationData() || !Builder->isComparable())
       return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
                                 Bind);
 

diff --git a/unittests/ASTMatchers/ASTMatchersTest.cpp b/unittests/ASTMatchers/ASTMatchersTest.cpp
index d4f867114e9b3e7508789ce446b11a608b59352f..e769e887bd1b993f8ff61f20acccb91e73e704b0 100644 (file)
--- a/unittests/ASTMatchers/ASTMatchersTest.cpp
+++ b/unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -643,6 +643,12 @@ TEST(DeclarationMatcher, HasDescendant) {
       "};", ZDescendantClassXDescendantClassY));
 }
 
+TEST(DeclarationMatcher, HasDescendantMemoization) {
+  DeclarationMatcher CannotMemoize =
+      decl(hasDescendant(typeLoc().bind("x")), has(decl()));
+  EXPECT_TRUE(matches("void f() { int i; }", CannotMemoize));
+}
+
 // Implements a run method that returns whether BoundNodes contains a
 // Decl bound to Id that can be dynamically cast to T.
 // Optionally checks that the check succeeded a specific number of times.