Generally looks good. A few comments:
================
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;
};
----------------
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.
================
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:131
@@ -133,1 +130,3 @@
+ std::vector<std::pair<const internal::DynTypedMatcher*, MatchCallback*> >
+ Triggers;
----------------
I would prefer a clearer name, even if it is on the wordy side. Trigger doesn't
really fit. Some possibilities:
MatcherMatchActionPair
MatcherMatchCallbackPair
MatcherAndActionPair
================
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>
----------------
This comment is stale; might as well fix it while we're nearby. The comment
doesn't include CXXCtorInitializer.
================
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;
----------------
Why the change to reinterpret_cast? It seems a fair bit messier. This would
also be cleaner using boost::any.
================
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
----------------
typo: "guaranteed be" -> "guaranteed to be"
http://llvm-reviews.chandlerc.com/D33
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
