r174172 - Re-design the convenience interfaces on MatchFinder.

Manuel Klimek klimek at google.com
Fri Feb 1 05:41:35 PST 2013


Author: klimek
Date: Fri Feb  1 07:41:35 2013
New Revision: 174172

URL: http://llvm.org/viewvc/llvm-project?rev=174172&view=rev
Log:
Re-design the convenience interfaces on MatchFinder.

First, this implements a match() method on MatchFinder; this allows us
to get rid of the findAll implementation, as findAll is really a special
case of recursive matchers on match.

Instead of findAll, provide a convenience function match() that lets
users iterate easily over the results instead of needing to implement
callbacks.

Modified:
    cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
    cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
    cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h?rev=174172&r1=174171&r2=174172&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h Fri Feb  1 07:41:35 2013
@@ -134,11 +134,17 @@ public:
   /// \brief Creates a clang ASTConsumer that finds all matches.
   clang::ASTConsumer *newASTConsumer();
 
-  /// \brief Finds all matches on the given \c Node.
+  /// \brief Calls the registered callbacks on all matches on the given \p Node.
+  ///
+  /// Note that there can be multiple matches on a single node, for
+  /// example when using decl(forEachDescendant(stmt())).
   ///
   /// @{
-  void findAll(const Decl &Node, ASTContext &Context);
-  void findAll(const Stmt &Node, ASTContext &Context);
+  template <typename T> void match(const T &Node, ASTContext &Context) {
+    match(clang::ast_type_traits::DynTypedNode::create(Node), Context);
+  }
+  void match(const clang::ast_type_traits::DynTypedNode &Node,
+             ASTContext &Context);
   /// @}
 
   /// \brief Registers a callback to notify the end of parsing.
@@ -158,6 +164,52 @@ private:
   ParsingDoneTestCallback *ParsingDone;
 };
 
+/// \brief Returns the results of matching \p Matcher on \p Node.
+///
+/// Collects the \c BoundNodes of all callback invocations when matching
+/// \p Matcher on \p Node and returns the collected results.
+///
+/// Multiple results occur when using matchers like \c forEachDescendant,
+/// which generate a result for each sub-match.
+///
+/// @{
+template <typename MatcherT, typename NodeT>
+SmallVector<BoundNodes, 1>
+match(MatcherT Matcher, const NodeT &Node, ASTContext &Context);
+
+template <typename MatcherT>
+SmallVector<BoundNodes, 1>
+match(MatcherT Matcher, const ast_type_traits::DynTypedNode &Node,
+      ASTContext &Context);
+/// @}
+
+namespace internal {
+class CollectMatchesCallback : public MatchFinder::MatchCallback {
+public:
+  virtual void run(const MatchFinder::MatchResult &Result) {
+    Nodes.push_back(Result.Nodes);
+  }
+  SmallVector<BoundNodes, 1> Nodes;
+};
+}
+
+template <typename MatcherT>
+SmallVector<BoundNodes, 1>
+match(MatcherT Matcher, const ast_type_traits::DynTypedNode &Node,
+      ASTContext &Context) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addMatcher(Matcher, &Callback);
+  Finder.match(Node, Context);
+  return Callback.Nodes;
+}
+
+template <typename MatcherT, typename NodeT>
+SmallVector<BoundNodes, 1>
+match(MatcherT Matcher, const NodeT &Node, ASTContext &Context) {
+  return match(Matcher, ast_type_traits::DynTypedNode::create(Node), Context);
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
 

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=174172&r1=174171&r2=174172&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Fri Feb  1 07:41:35 2013
@@ -467,6 +467,26 @@ public:
     return matchesAncestorOfRecursively(Node, Matcher, Builder, MatchMode);
   }
 
+  // Matches all registered matchers on the given node and calls the
+  // result callback for every node that matches.
+  void match(const ast_type_traits::DynTypedNode& Node) {
+    for (std::vector<std::pair<const internal::DynTypedMatcher*,
+                               MatchCallback*> >::const_iterator
+             I = MatcherCallbackPairs->begin(), E = MatcherCallbackPairs->end();
+         I != E; ++I) {
+      BoundNodesTreeBuilder Builder;
+      if (I->first->matches(Node, this, &Builder)) {
+        BoundNodesTree BoundNodes = Builder.build();
+        MatchVisitor Visitor(ActiveASTContext, I->second);
+        BoundNodes.visitMatches(&Visitor);
+      }
+    }
+  }
+
+  template <typename T> void match(const T &Node) {
+    match(ast_type_traits::DynTypedNode::create(Node));
+  }
+
   // Implements ASTMatchFinder::getASTContext.
   virtual ASTContext &getASTContext() const { return *ActiveASTContext; }
 
@@ -544,24 +564,6 @@ private:
     return false;
   }
 
-  // Matches all registered matchers on the given node and calls the
-  // result callback for every node that matches.
-  template <typename T>
-  void match(const T &node) {
-    for (std::vector<std::pair<const internal::DynTypedMatcher*,
-                               MatchCallback*> >::const_iterator
-             I = MatcherCallbackPairs->begin(), E = MatcherCallbackPairs->end();
-         I != E; ++I) {
-      BoundNodesTreeBuilder Builder;
-      if (I->first->matches(ast_type_traits::DynTypedNode::create(node),
-                            this, &Builder)) {
-        BoundNodesTree BoundNodes = Builder.build();
-        MatchVisitor Visitor(ActiveASTContext, I->second);
-        BoundNodes.visitMatches(&Visitor);
-      }
-    }
-  }
-
   std::vector<std::pair<const internal::DynTypedMatcher*,
                         MatchCallback*> > *const MatcherCallbackPairs;
   ASTContext *ActiveASTContext;
@@ -777,16 +779,11 @@ ASTConsumer *MatchFinder::newASTConsumer
   return new internal::MatchASTConsumer(&MatcherCallbackPairs, ParsingDone);
 }
 
-void MatchFinder::findAll(const Decl &Node, ASTContext &Context) {
-  internal::MatchASTVisitor Visitor(&MatcherCallbackPairs);
-  Visitor.set_active_ast_context(&Context);
-  Visitor.TraverseDecl(const_cast<Decl*>(&Node));
-}
-
-void MatchFinder::findAll(const Stmt &Node, ASTContext &Context) {
+void MatchFinder::match(const clang::ast_type_traits::DynTypedNode &Node,
+                        ASTContext &Context) {
   internal::MatchASTVisitor Visitor(&MatcherCallbackPairs);
   Visitor.set_active_ast_context(&Context);
-  Visitor.TraverseStmt(const_cast<Stmt*>(&Node));
+  Visitor.match(Node);
 }
 
 void MatchFinder::registerTestCallbackAfterParsing(

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=174172&r1=174171&r2=174172&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Fri Feb  1 07:41:35 2013
@@ -3439,24 +3439,18 @@ TEST(NNSLoc, NestedNameSpecifierLocsAsDe
       new VerifyIdIsBoundTo<NestedNameSpecifierLoc>("x", 3)));
 }
 
-template <typename T>
-class VerifyRecursiveMatch : public BoundNodesCallback {
+template <typename T> class VerifyMatchOnNode : public BoundNodesCallback {
 public:
-  explicit VerifyRecursiveMatch(StringRef Id,
-                                const internal::Matcher<T> &InnerMatcher)
-      : Id(Id), InnerMatcher(InnerMatcher) {}
-
-  virtual bool run(const BoundNodes *Nodes) {
-    return false;
+  VerifyMatchOnNode(StringRef Id, const internal::Matcher<T> &InnerMatcher)
+      : Id(Id), InnerMatcher(InnerMatcher) {
   }
 
+  virtual bool run(const BoundNodes *Nodes) { return false; }
+
   virtual bool run(const BoundNodes *Nodes, ASTContext *Context) {
     const T *Node = Nodes->getNodeAs<T>(Id);
-    bool Found = false;
-    MatchFinder Finder;
-    Finder.addMatcher(InnerMatcher, new VerifyMatch(0, &Found));
-    Finder.findAll(*Node, *Context);
-    return Found;
+    SmallVector<BoundNodes, 1> Result = match(InnerMatcher, *Node, *Context);
+    return !Result.empty();
   }
 private:
   std::string Id;
@@ -3464,21 +3458,36 @@ private:
 };
 
 TEST(MatchFinder, CanMatchDeclarationsRecursively) {
-  EXPECT_TRUE(matchAndVerifyResultTrue("class X { class Y {}; };",
-    recordDecl(hasName("::X")).bind("X"),
-    new VerifyRecursiveMatch<clang::Decl>("X", recordDecl(hasName("X::Y")))));
-  EXPECT_TRUE(matchAndVerifyResultFalse("class X { class Y {}; };",
-    recordDecl(hasName("::X")).bind("X"),
-    new VerifyRecursiveMatch<clang::Decl>("X", recordDecl(hasName("X::Z")))));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class X { class Y {}; };", recordDecl(hasName("::X")).bind("X"),
+      new VerifyMatchOnNode<clang::Decl>(
+          "X", decl(hasDescendant(recordDecl(hasName("X::Y")))))));
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+      "class X { class Y {}; };", recordDecl(hasName("::X")).bind("X"),
+      new VerifyMatchOnNode<clang::Decl>(
+          "X", decl(hasDescendant(recordDecl(hasName("X::Z")))))));
 }
 
 TEST(MatchFinder, CanMatchStatementsRecursively) {
-  EXPECT_TRUE(matchAndVerifyResultTrue("void f() { if (1) { for (;;) { } } }",
-    ifStmt().bind("if"),
-    new VerifyRecursiveMatch<clang::Stmt>("if", forStmt())));
-  EXPECT_TRUE(matchAndVerifyResultFalse("void f() { if (1) { for (;;) { } } }",
-    ifStmt().bind("if"),
-    new VerifyRecursiveMatch<clang::Stmt>("if", declStmt())));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "void f() { if (1) { for (;;) { } } }", ifStmt().bind("if"),
+      new VerifyMatchOnNode<clang::Stmt>("if",
+                                         stmt(hasDescendant(forStmt())))));
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+      "void f() { if (1) { for (;;) { } } }", ifStmt().bind("if"),
+      new VerifyMatchOnNode<clang::Stmt>("if",
+                                         stmt(hasDescendant(declStmt())))));
+}
+
+TEST(MatchFinder, CanMatchSingleNodesRecursively) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class X { class Y {}; };", recordDecl(hasName("::X")).bind("X"),
+      new VerifyMatchOnNode<clang::Decl>(
+          "X", recordDecl(has(recordDecl(hasName("X::Y")))))));
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+      "class X { class Y {}; };", recordDecl(hasName("::X")).bind("X"),
+      new VerifyMatchOnNode<clang::Decl>(
+          "X", recordDecl(has(recordDecl(hasName("X::Z")))))));
 }
 
 class VerifyStartOfTranslationUnit : public MatchFinder::MatchCallback {





More information about the cfe-commits mailing list