[PATCH] Introduce clang-query tool.

Manuel Klimek klimek at google.com
Mon Nov 4 00:46:33 PST 2013



================
Comment at: clang-query/QueryEngine.h:28
@@ +27,3 @@
+
+class QueryEngine {
+  llvm::raw_ostream &Out;
----------------
I'm a very strong proponent of "public-first". The reason is that a reader of the class usually starts at the top, and a user of a class should not care about the privates, and thus be able to skip them - this is considerably easier if they're at the bottom of the class definition.

================
Comment at: clang-query/QueryEngine.h:37
@@ +36,3 @@
+      : Out(Out), ASTs(ASTs), OutKind(OK_Diag), BindRoot(true) {}
+  void ActOnQuery(const Query &Q);
+};
----------------
Some doxygen comments would be highly appreciated ;)

================
Comment at: clang-query/QueryEngine.cpp:34
@@ +33,3 @@
+void QueryEngine::ActOnQuery(const Query &Q) {
+  switch (Q.Kind) {
+  case QK_Invalid:
----------------
This seems like a strong indication that we want virtual dispatch here :) (I thought that when reading the header, but this kinda drives it home)

================
Comment at: clang-query/tool/ClangQuery.cpp:50-57
@@ +49,10 @@
+// Set up the command line options
+cl::opt<std::string> BuildPath(
+  cl::Positional,
+  cl::desc("<build-path>"));
+
+cl::list<std::string> SourcePaths(
+  cl::Positional,
+  cl::desc("<source0> [... <sourceN>]"),
+  cl::OneOrMore);
+
----------------
Can we add a -c "..." or -f <command-file> batch mode? This would also allow us to add a FileCheck integration test...


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



More information about the cfe-commits mailing list