================ Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:137 @@ -136,2 +136,3 @@ - /// \brief Finds all matches on the given \c Node. + /// \brief Triggers on all matches on the given \c Node. + /// ---------------- Manuel Klimek wrote: > Daniel Jasper wrote: > > Triggers what? I presume it is the registered callbacks? > Done. ?
================ Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:146 @@ +145,3 @@ + } + void match(const clang::ast_type_traits::DynTypedNode &Node, + ASTContext &Context); ---------------- Manuel Klimek wrote: > Daniel Jasper wrote: > > Should this be in the public interface? > Yes, as with the parent map in ASTContext having a DynTypedNode is actually > something that happens... Ack. ================ Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:167 @@ -160,1 +166,3 @@ +/// \brief Returns the results of matching \c Matcher on \c Node. +/// ---------------- Manuel Klimek wrote: > Daniel Jasper wrote: > > How about: "Returns the set of bound nodes for each match of \c Matcher > > under \c Node." > > > > The "under" is also used below, but I don't know whether it makes perfectly > > clear that the matcher is matched on Node and all of its descendants. > It is not. 'match' only matches the node. This is a strict superset of the > findAll use case, but an important one - without it, it's impossible to run a > matcher against a single node (for example inside a match callback), which is > very useful (and the reason I started on this patch). Ack. Might be easier to understand with a comment, though. I presume you need to return a SmallVector to support matchers such as forEachDescendant? How about: "Returns the \c BoundNodes of all callback invocations when matching \p Matcher on \p Node in \p Context."? How about choosing a name that reflects that this returns something? E.g. "matchResults". http://llvm-reviews.chandlerc.com/D359 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
