[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)
Loïc Joly via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 15 11:25:19 PDT 2020
loic-joly-sonarsource created this revision.
loic-joly-sonarsource added a reviewer: klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
In ASTMatcher, when we have `has(...)` and `hasParent(...)` called with the same internal matcher on the same node, the memoization process will mix-up the two calls because the direction of the traversal is not part of the memoization key.
This patch adds this information.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80025
Files:
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2591,6 +2591,14 @@
compoundStmt(hasParent(ifStmt()))));
}
+TEST(MatcherMemoize, HasParentDiffersFromHas) {
+ // Test introduced after detecting a bug in memoization
+ EXPECT_TRUE(matches(
+ "void f() { throw 1; }",
+ expr(eachOf(cxxThrowExpr(hasParent(expr())),
+ cxxThrowExpr(has(expr()))))));
+}
+
TEST(HasAncestor, MatchesAllAncestors) {
EXPECT_TRUE(matches(
"template <typename T> struct C { static void f() { 42; } };"
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -43,6 +43,11 @@
// optimize this on.
static const unsigned MaxMemoizationEntries = 10000;
+enum class MatchDirection {
+ Ancestors,
+ Descendants
+};
+
// We use memoization to avoid running the same matcher on the same
// AST node twice. This struct is the key for looking up match
// result. It consists of an ID of the MatcherInterface (for
@@ -60,11 +65,12 @@
ast_type_traits::DynTypedNode Node;
BoundNodesTreeBuilder BoundNodes;
ast_type_traits::TraversalKind Traversal = ast_type_traits::TK_AsIs;
+ MatchDirection Direction;
bool operator<(const MatchKey &Other) const {
- return std::tie(MatcherID, Node, BoundNodes, Traversal) <
+ return std::tie(MatcherID, Node, BoundNodes, Traversal, Direction) <
std::tie(Other.MatcherID, Other.Node, Other.BoundNodes,
- Other.Traversal);
+ Other.Traversal, Other.Direction);
}
};
@@ -457,6 +463,7 @@
// Note that we key on the bindings *before* the match.
Key.BoundNodes = *Builder;
Key.Traversal = Ctx.getTraversalKind();
+ Key.Direction = MatchDirection::Descendants;
MemoizationMap::iterator I = ResultCache.find(Key);
if (I != ResultCache.end()) {
@@ -706,6 +713,7 @@
Key.Node = Node;
Key.BoundNodes = *Builder;
Key.Traversal = Ctx.getTraversalKind();
+ Key.Direction = MatchDirection::Ancestors;
// Note that we cannot use insert and reuse the iterator, as recursive
// calls to match might invalidate the result cache iterators.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80025.264293.patch
Type: text/x-patch
Size: 2468 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200515/cc30f083/attachment-0001.bin>
More information about the cfe-commits
mailing list