[cfe-commits] [PATCH] Matchers for Types, QualTypes and TypeLocs

[Manuel Klimek]

  Apart from the nits, I think this is fine to go in.


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:112
@@ -112,1 +111,3 @@
 typedef internal::Matcher<Stmt> StatementMatcher;
+typedef internal::Matcher<QualType> TypeMatcher;
+typedef internal::Matcher<TypeLoc> TypeLocMatcher;
----------------
Daniel Jasper wrote:
> Manuel Klimek wrote:
> > Do we want to keep that backwards compatible, or do we want to rename this to QualTypeMatcher?
> I don't see a reason to change this now, but I am happy to, if you think it is better.
Nah, fine in a follow-up patch.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2462
@@ +2461,3 @@
+/// Given
+///   struct A {};
+///   A a;
----------------
Missing the \code tags...

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1095
@@ +1094,3 @@
+    NestedNameSpecifierLoc NextNode = Node.getPrefix();
+    if (!NextNode)
+      return false;
----------------
Daniel Jasper wrote:
> Manuel Klimek wrote:
> > I think we should be consistent with == NULL vs. implicit in this file.
> I don't think this comment makes sense. NextNode is not a pointer so I don't think we should be comparing it against NULL.
Heh, right, sorry :)

================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:233
@@ -227,8 +232,3 @@
 ///
-/// The traversal is done using the given \c FunctionName.
-#define LOC_TRAVERSE_MATCHER(                                                  \
-      MatcherName, NodeType, FunctionName)                                     \
-  inline internal::Matcher<NodeType> hasPrefix(                                \
-      const internal::Matcher<NodeType> &InnerMatcher) {                       \
-    return internal::makeMatcher(new internal::TraverseMatcher<NodeType>(      \
-      InnerMatcher, &NodeType::getPrefix));                                    \
+/// The traversal is done using \c FunctionName.
+#define AST_TYPE_TRAVERSE_MATCHER(MatcherName, FunctionName)                   \
----------------
Daniel Jasper wrote:
> Manuel Klimek wrote:
> > Perhaps: The traversal is done using \c NodeTypel::FunctionName.?
> Like this?
Are you saying NodeType is not the one that FunctionName is called on?

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:560
@@ +559,3 @@
+  // RecursiveASTVisitor by itself would only visit the TypeLocs, not the
+  // contained types.
+  match(TypeLocNode);
----------------
The question is: why do we want to traverse the Types here?


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