[PATCH] Implements memoization for ancestor matching.

Manuel Klimek klimek at google.com
Mon Mar 11 07:57:54 PDT 2013


On Mon, Mar 11, 2013 at 7:19 AM, Daniel Jasper <djasper at google.com> wrote:

>
>
> ================
> Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:425
> @@ -423,3 +424,3 @@
>  private:
> -  bool matchesAncestorOfRecursively(
> +  bool memoizedMatchesAncestorOfRecursively(
>        const ast_type_traits::DynTypedNode &Node, const DynTypedMatcher
> &Matcher,
> ----------------
> 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()..
>

Any constructive ideas on how to change the layout?


> 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?
>

Yes. That doesn't mean it was a good idea though :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130311/17085d77/attachment.html>


More information about the cfe-commits mailing list