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

[Daniel Jasper]

================
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:131
@@ -133,1 +130,3 @@
+  std::vector<std::pair<const internal::DynTypedMatcher*, MatchCallback*> >
+    Triggers;
 
----------------
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?

================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:86
@@ -71,1 +85,3 @@
+  /// pointers and thus need to be stored by value.
+  llvm::AlignedCharArrayUnion<Decl*, Stmt*, QualType> Storage;
 };
----------------
Michael Diamond wrote:
> I think it would be better to use boost::any here. Then you don't have to explicitly specify all the types. You will still be able to store the QualType by value, but you won't have to worry about it in the declaration of Storage.
LLVM and Boost don't mix ;-).... Boost is quite massive and would create a significant burden on clang. The specialized abstractions provided by LLVM are quite sufficient for most use cases.

The best data structure would probably by a union, however until we can use C++11, unions don't support members with non-trivial constructors. That is why we use this helper structure for the time being.

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


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