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

[Manuel Klimek]

================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:92
@@ -77,1 +91,3 @@
+    if (Tag == NT_Decl)
+      return dyn_cast<T>(*reinterpret_cast<Decl*const*>(Storage));
     return NULL;
----------------
Daniel Jasper wrote:
> Michael Diamond wrote:
> > Why the change to reinterpret_cast? It seems a fair bit messier. This would also be cleaner using boost::any.
> As said above, no boost in LLVM. However, I agree that we could probably continue passing in a void* here, right? Even for the QualType we are passing a pointer to the storage inside DynTypedNode, which should work.
The change from void* is because now we have an actual storage (the character array), and I think funneling this through a void* will make it less explicit that we're doing something fishy here.

================
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:131
@@ -133,1 +130,3 @@
+  std::vector<std::pair<const internal::DynTypedMatcher*, MatchCallback*> >
+    Triggers;
 
----------------
Daniel Jasper wrote:
> Michael Diamond wrote:
> > I would prefer a clearer name, even if it is on the wordy side. Trigger doesn't really fit. Some possibilities:
> > MatcherMatchActionPair
> > MatcherMatchCallbackPair
> > MatcherAndActionPair
> Maybe: CallbackForMatcher?
Renamed to MatcherCallbackPairs. I dislike pretty much all of the names, but that seems to be most straight forward.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:375
@@ -346,3 +374,3 @@
 /// \brief IsBaseType<T>::value is true if T is a "base" type in the AST
 /// node class hierarchies (i.e. if T is Decl, Stmt, or QualType).
 template <typename T>
----------------
Michael Diamond wrote:
> This comment is stale; might as well fix it while we're nearby. The comment doesn't include CXXCtorInitializer.
Done.

================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:83
@@ +82,3 @@
+  /// Note that we can store \c Decls and \c Stmts by pointer as they are
+  /// guaranteed be unique pointers pointing to dedicated storage in the
+  /// AST. \c QualTypes on the other hand do not have storage or unique
----------------
Michael Diamond wrote:
> typo: "guaranteed be" -> "guaranteed to be"
Done.


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