[PATCH] Re-design the convenience interfaces on MatchFinder.

[Daniel Jasper]

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