[cfe-commits] [PATCH] Implements hasAncestor.

[Daniel Jasper]

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:460
@@ -451,1 +459,3 @@
 
+  // 
+  template <typename T>
----------------
Empty comment?

Maybe add

// FIXME: Implement support for BindKind in matchesAncestorOf.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:391
@@ -390,3 +390,3 @@
 ///
-/// This provides two entry methods for each base node type in the AST:
-/// - matchesChildOf:
+/// This provides three entry methods for each base node type in the AST:
+/// - \c matchesChildOf:
----------------
Should we have matchesParentOf?

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:33
@@ +32,3 @@
+/// \brief A \c RecursiveASTVisitor that builds a map from nodes to their
+/// parents.
+///
----------------
Should we add something like ".. to their parents as defined by the RecursiveASTVisitor"? I think there might not be another explicit definition of the node hierarchy and the behavior differs e.g. from Decl::getDeclContext().

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:2798
@@ +2797,3 @@
+      "struct X {}; struct A { A() {} X x; };",
+      constructorDecl(
+          hasAnyConstructorInitializer(withInitializer(expr(
----------------
I had to think about this for a bit because actually the hasAncestor does not match implicit code. The important point is that the ParentMap contains implicit code. Maybe rename to "TEST(HasAncestor, ParentMapIncludesImplicitCode)"?


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