From d33130402939b8fec404b8b34ad3377594e26352 Mon Sep 17 00:00:00 2001
From: Daniel Jasper <djasper@google.com>
Date: Wed, 23 Jul 2014 13:17:47 +0000
Subject: [PATCH] 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
---
.../clang/ASTMatchers/ASTMatchersInternal.h | 20 +++++++++++++++++++
lib/ASTMatchers/ASTMatchFinder.cpp | 2 +-
unittests/ASTMatchers/ASTMatchersTest.cpp | 6 ++++++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h
index 94435fd274e..68b18164c23 100644
--- 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 23708e2ff0c..b8ac68aad72 100644
--- 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 d4f867114e9..e769e887bd1 100644
--- 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.
--
GitLab