[PATCH] Introduce clang-query tool.

Peter Collingbourne peter at pcc.me.uk
Wed Nov 6 16:36:28 PST 2013



================
Comment at: clang-query/Query.cpp:59
@@ +58,3 @@
+        M = decl(M).bind("root");
+      Finder.addMatcher(decl(forEachDescendant(M)), &Collect);
+    } else if (Matcher.canConvertTo<Stmt>()) {
----------------
Samuel Benzaquen wrote:
> What is the purpose of forEachDescendant() here?
> Finder will recursively try to match the provided matchers already.
> 
> Doing it this way will match the topmost node if something in it matches, instead of returning the actual node that matched.
MatchFinder::match will not match recursively, so we need to do it ourselves with forEachDescendant.

However, upon further reflection, I've decided that it would be better to have a MatchFinder::matchAST that does the same thing for ASTs as the ASTConsumer returned by MatchFinder::getConsumer. See D2115.

================
Comment at: clang-query/Query.cpp:55-65
@@ +54,13 @@
+    CollectBoundNodes Collect(Matches);
+    if (Matcher.canConvertTo<Decl>()) {
+      DeclarationMatcher M = Matcher.convertTo<Decl>();
+      if (QS.BindRoot)
+        M = decl(M).bind("root");
+      Finder.addMatcher(decl(forEachDescendant(M)), &Collect);
+    } else if (Matcher.canConvertTo<Stmt>()) {
+      StatementMatcher M = Matcher.convertTo<Stmt>();
+      if (QS.BindRoot)
+        M = stmt(M).bind("root");
+      Finder.addMatcher(decl(forEachDescendant(M)), &Collect);
+    }
+    Finder.match(*AST->getASTContext().getTranslationUnitDecl(),
----------------
Samuel Benzaquen wrote:
> Manuel Klimek wrote:
> > Yea, we really need addDynamicMatcher ...
> If I remember correctly, I added one, then we decide to remove it again.
> This tool would be the first use case that would require it.
I've resurrected it in D2114.

================
Comment at: clang-query/QueryParser.h:22
@@ +21,3 @@
+/// \c InvalidQuery if a parse error occurs.
+QueryRef ParseQuery(const char *Line);
+
----------------
Manuel Klimek wrote:
> Here also StringRef...
OK, done.

================
Comment at: clang-query/Query.cpp:58
@@ +57,3 @@
+      if (QS.BindRoot)
+        M = decl(M).bind("root");
+      Finder.addMatcher(decl(forEachDescendant(M)), &Collect);
----------------
Samuel Benzaquen wrote:
> You can bind at the DynTypedMatcher level using DynTypedMatcher::tryBind(). No need to do it for every type.
Done.

================
Comment at: clang-query/Query.h:60-61
@@ +59,4 @@
+
+  static bool classof(const Query *Q) { return Q->Kind == QK_Invalid; }
+  static bool classof(const InvalidQuery *Q) { return true; }
+};
----------------
Sean Silva wrote:
> These `return true` classof's should be here. See <http://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html>. In particular, see the second rule of thumb <http://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#rules-of-thumb>.
I assume you meant "should not be here".  Done.

================
Comment at: clang-query/QueryParser.h:20
@@ +19,3 @@
+///
+/// \return A reference to the parsed query object, which may be an
+/// \c InvalidQuery if a parse error occurs.
----------------
Samuel Benzaquen wrote:
> There should be a comment somewhere specifying the valid commands and their syntax.
> It could in the parser, or somewhere in ClangQuery.cpp (possibly printing it to the user with a --help option).
Done.


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

BRANCH
  clangquery

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list