[PATCH] Implements memoization for ancestor matching.

[Daniel Jasper]

  A few nits, otherwise looks good.


================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:428
@@ +427,3 @@
+  //
+  // The oder of matching (which is visible through which node is bound in case
+  // there are multiple matches) is breadth first search.
----------------
nit: s/oder/order/

nit: maybe "(which can lead to different nodes being bound)"

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:436-439
@@ +435,6 @@
+  //
+  // Once there are multiple parents, the result of a hasAncestor(...) matcher
+  // in terms of which node was bound is not necessarily the same as the result
+  // of breadth first search on a child node, thus we do not memoize ancestors
+  // below split points.
+  bool memoizedMatchesAncestorOfRecursively(
----------------
Wow, this is hard to understand ;-).

How about:
For nodes with multiple parents, memoization becomes more complex as (to give the same result as a BFS) we would need to memoize the match distance. As this is a rare case and multiple parents usually occur close to the root of the AST, the performance gain does not justify the increased complexity.

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:488
@@ +487,3 @@
+              // are splits on the way down.
+              if (!Visited.count(I->getMemoizationData())) {
+                Visited.insert(I->getMemoizationData());
----------------
Use the "if (Visited.insert(..).second) {}".


http://llvm-reviews.chandlerc.com/D510

BRANCH
  clang-memoize

ARCANIST PROJECT
  clang