[PATCH] Implements memoization for ancestor matching.

[Manuel Klimek]

================
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.
----------------
Daniel Jasper wrote:
> nit: s/oder/order/
> 
> nit: maybe "(which can lead to different nodes being bound)"
Done.

================
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(
----------------
Daniel Jasper wrote:
> 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.
"... would need to memoize the match distance". I'm actually not sure :) I'd need to go and figure out what exactly we'd need.

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


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

BRANCH
  clang-memoize

ARCANIST PROJECT
  clang