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