[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