r213751 - Prevent assert in ASTMatchFinder.

[Daniel Jasper]

Author: djasper
Date: Wed Jul 23 08:17:47 2014
New Revision: 213751

URL: http://llvm.org/viewvc/llvm-project?rev=213751&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
    cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
    cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=213751&r1=213750&r2=213751&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Wed Jul 23 08:17:47 2014
@@ -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;
 };

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=213751&r1=213750&r2=213751&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Wed Jul 23 08:17:47 2014
@@ -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);
 

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=213751&r1=213750&r2=213751&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Wed Jul 23 08:17:47 2014
@@ -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.