[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]

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:81-85
@@ -81,3 +80,7 @@
     reset();
-    traverse(Node);
+    if (const Decl *D = DynNode.get<Decl>())
+      traverse(*D);
+    else if (const Stmt *S = DynNode.get<Stmt>())
+      traverse(*S);
+    // FIXME: Add other base types after adding tests.
     return Matches;
----------------
Manuel Klimek wrote:
> Sean Silva wrote:
> > A switch on the kind, with an "unreachable" default would allow clang to warn on incomplete coverage of the enum during future maintenance.
> I actually don't want to expose the kind enum. Perhaps I'm a little too cautious, but I like my interfaces as small as possible.
> 
> I'm more thinking about having something built into RAV that would make sure all types are handled, but that would make RAV depend on the DynTypedNodes. I guess we'll see how it goes...
Ok, no biggie.

My impression is that DynTypedNode will eventually graduate to be a public interface for general AST manipulation, and that the NodeTypeTag enum will naturally be part of the interface to allow users to switch on it. Only time will tell though. It's not a big deal at the moment though.

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


http://llvm-reviews.chandlerc.com/D33