================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
