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

Manuel Klimek klimek at google.com
Mon Jan 27 11:12:19 PST 2014



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

================
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;
+  }
----------------
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==?

================
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;
----------------
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.

================
Comment at: clang-query/QueryParser.h:53
@@ +52,3 @@
+
+  const char *CompletionPos;
+  std::vector<llvm::LineEditor::Completion> Completions;
----------------
I think some of the code later might be simpler if this was an offset from Begin. I might be missing something, though.


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



More information about the cfe-commits mailing list