================
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:132-133
@@ -127,7 +131,4 @@
private:
- /// \brief The MatchCallback*'s will be called every time the
- /// UntypedBaseMatcher matches on the AST.
- std::vector< std::pair<
- const internal::UntypedBaseMatcher*,
- MatchCallback*> > Triggers;
+ /// \brief The triggers executed against the AST.
+ std::vector<Trigger> Triggers;
----------------
Sean Silva wrote:
> Manuel Klimek wrote:
> > Sean Silva wrote:
> > > Since "trigger" is just a plain pair, it's not really clear at all what
> > > "executed against the AST means" for a trigger without some supporting
> > > documentation. Also as I mentioned above, putting the definition of
> > > `Trigger` nearby would help too.
> > >
> > > (in general, a more self-documenting name than "trigger" would be good
> > > ("MatcherWithAction"?)).
> > Yea, I didn't really like the name either, and the more I think about it,
> > the worse "Trigger" sounds.
> >
> > How about MatchAction?
> To me, MatchAction sounds like "the action corresponding to a match", which
> is what MatchCallback is.
Ok, couldn't come up with a better name and reverted the pull-out of the
abstraction for now. Instead did a tyepdef of MatchCallback in the .cpp file
for brevity of the type.
================
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?
================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:257-258
@@ -257,1 +256,4 @@
+ const UntypedMatchInput input(Matcher.getID(), Node.getMemoizationData());
+ assert(input.second &&
+ "Fix getMemoizationData once more types allow recursive matching.");
std::pair<MemoizationMap::iterator, bool> InsertResult
----------------
Sean Silva wrote:
> Maybe put a `// FIXME` near this to keep things grep'able.
There is actually no FIXME here - this code will not need to be adapted. The
assert() is just there for the case where getMemoizationData() is out of sync
with the rest of the DynTypedNode (which hopefully will not happen ;)
http://llvm-reviews.chandlerc.com/D33
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
