[cfe-commits] [PATCH] Introduces DynTypedMatcher as a new concept that replaces the UntypedBaseMatcher and TypedMatcher. Due to DynTypedNode the basic dynamically typed matcher interface can now be simplified.
Sean Silva
reviews at llvm-reviews.chandlerc.com
Mon Sep 3 07:27:24 PDT 2012
> ================
> Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:353-355
> @@ -367,4 +352,5 @@
> + this, &Builder)) {
> BoundNodesTree BoundNodes = Builder.build();
> MatchVisitor Visitor(ActiveASTContext, It->second);
> BoundNodes.visitMatches(&Visitor);
> }
> ----------------
> Sean Silva wrote:
>> I know this isn't directly related to this patch, but this is really the nerve-center of the matching process, and deserves to be better explained (or cross-referenced to where it is explained).
> Hm, which parts need more explanations? Anything specific?
It's not that it's that hard to understand, but that it's hard to
find. So, for example, I think it would be good to mention it in the
comment at the top of the file. Also, I think a comment on match()
should definitely say "this is the core of the matching process", or
something like that. In other words, in order to understand the code,
this is the best place to start at (AFAIK), and the reader should be
aware of that so that they don't waste their time.
If there is another place which is a better place to start, then that
should be the thing called out in the comment at the top of the file
of course.
--Sean Silva
http://llvm-reviews.chandlerc.com/D33
More information about the cfe-commits
mailing list