<div dir="ltr">On Mon, Mar 11, 2013 at 7:19 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
================<br>
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:425<br>
@@ -423,3 +424,3 @@<br>
<div class="im"> private:<br>
-  bool matchesAncestorOfRecursively(<br>
+  bool memoizedMatchesAncestorOfRecursively(<br>
       const ast_type_traits::DynTypedNode &Node, const DynTypedMatcher &Matcher,<br>
</div>----------------<br>
I think the combination of these two methods is highly confusing. The name suggests that both do the same thing, just one with memoization and one without memoization, which they don't. Furthermore, the responsibilities are not clear at all. E.g. what happens if I call matchesAncestorOfRecursively() with the TUDecl? It will probably assert, as the check for that is only in memoizedMatchesAncestorOfRecursively()..<br>
</blockquote><div><br></div><div style>Any constructive ideas on how to change the layout?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, it seems a bit strange, to first match all direct parents (kind of BFS) and then recursively call matchesAncestorOfRecursively() on each one (kind of DFS). Is this intended?<br></blockquote><div><br></div><div style>
Yes. That doesn't mean it was a good idea though :)</div></div></div></div>