================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
