[PATCH] Introduce clang-query tool.

[Peter Collingbourne]

================
Comment at: clang-query/QueryParser.h:18
@@ +17,3 @@
+
+Query ParseQuery(const char *Line);
+
----------------
Samuel Benzaquen wrote:
> StringRef instead of const char*
I'd prefer not to.  The implementation of the parser is simpler when the string is known to be null terminated.

================
Comment at: clang-query/QueryParser.cpp:22
@@ +21,3 @@
+
+static bool IsWhitespace(char C) {
+  return C == ' ' || C == '\t' || C == '\n' || C == '\r';
----------------
Samuel Benzaquen wrote:
> isWhitespace() from clang/Basic/CharInfo.h
Done.

================
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);
+
----------------
Manuel Klimek wrote:
> Can we add a -c "..." or -f <command-file> batch mode? This would also allow us to add a FileCheck integration test...
I think both would be useful; done.

================
Comment at: clang-query/QueryEngine.h:28
@@ +27,3 @@
+
+class QueryEngine {
+  llvm::raw_ostream &Out;
----------------
Manuel Klimek wrote:
> 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.
I believe this comment is moot following the design change.

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

================
Comment at: clang-query/QueryEngine.cpp:34
@@ +33,3 @@
+void QueryEngine::ActOnQuery(const Query &Q) {
+  switch (Q.Kind) {
+  case QK_Invalid:
----------------
Manuel Klimek wrote:
> 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)
I wanted to keep Query a pure data structure in order to simplify the parser tests. But of course there are other considerations. Done.


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