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