[PATCH] Add completion to the query parser, and hook it up to clang-query.

[Peter Collingbourne]

================
Comment at: clang-query/QueryParser.h:53
@@ +52,3 @@
+
+  const char *CompletionPos;
+  std::vector<llvm::LineEditor::Completion> Completions;
----------------
Manuel Klimek wrote:
> I think some of the code later might be simpler if this was an offset from Begin. I might be missing something, though.
The main reason I decided to make this a pointer was that Begin moves along the string as we parse, and if this were an offset we would need to keep it updated.

================
Comment at: clang-query/QueryParser.cpp:64-71
@@ +63,10 @@
+  LexOrCompleteWord &Case(const char (&S)[N], const T &Value) {
+    if (WordCompletionPos == ~0u)
+      SS.Case(S, Value);
+    else if (N != 1 && SeenCases.insert(Value).second &&
+             std::memcmp(S, Str.data(), WordCompletionPos) == 0)
+      P->Completions.push_back(LineEditor::Completion(
+          std::string(S + WordCompletionPos, N - 1 - WordCompletionPos) + " ",
+          std::string(S, N - 1)));
+    return *this;
+  }
----------------
Manuel Klimek wrote:
> I find this somewhat hard to read.
> Things that might make it easier:
> - pull out a constant for ~0u, or use one of the already existing constants
> - s/SS/Switch/, or find an even more descriptive name what we're switching on here
> - is there a particular reason you're using memcmp instead of using StringRefs and substr and operator==?
> pull out a constant for ~0u, or use one of the already existing constants

Done.

> s/SS/Switch/, or find an even more descriptive name what we're switching on here

Done.

> is there a particular reason you're using memcmp instead of using StringRefs and substr and operator==?

No particularly good reason. In fact I think the call to memcmp was a memory error. I've changed this as you suggested.

I also got rid of the SeenCases set; I replaced it with a new IsCompletion argument to Case which should make that logic more readable.

================
Comment at: clang-query/QueryParser.cpp:51
@@ -48,3 +50,3 @@
 
-static QueryRef ParseSetBool(bool QuerySession::*Var, StringRef ValStr) {
-  unsigned Value = StringSwitch<unsigned>(ValStr)
+template <typename T> struct QueryParser::LexOrCompleteWord {
+  StringSwitch<T> SS;
----------------
Manuel Klimek wrote:
> When reading this without the context of how it's used it's hard to know why this is templated, and what the template parameters are supposed to be.
Added some comments (here and for lexOrCompleteWord).

================
Comment at: clang-query/QueryParser.h:23
@@ +22,3 @@
+
+class QueryParser {
+ public:
----------------
Manuel Klimek wrote:
> While mostly self-explanatory, I still think the public interface might benefit from some comments...
Added.


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