[PATCH] Add new 'let' command to bind arbitrary values into constants.

Peter Collingbourne peter at pcc.me.uk
Thu Apr 17 15:09:50 PDT 2014



================
Comment at: clang-query/QueryParser.cpp:71
@@ -70,3 +70,3 @@
       Switch.Case(S, Value);
     else if (N != 1 && IsCompletion && WordCompletionPos <= CaseStr.size() &&
              CaseStr.substr(0, WordCompletionPos) ==
----------------
Samuel Benzaquen wrote:
> Peter Collingbourne wrote:
> > You could factor this code out into a completeWord function.
> How would it help with this change?
I meant that it could help with implementing completion, but if you want to do that separately, that's fine.

================
Comment at: clang-query/QueryParser.cpp:264
@@ -216,1 +263,3 @@
 
+  case PQK_Unlet: {
+    StringRef Name = lexWord();
----------------
Samuel Benzaquen wrote:
> Peter Collingbourne wrote:
> > It would be nice to support completion for value names here.
> Yes.
> When I added the named values in dynamic::Parser I added a TODO for completion.
> The Sema should provide completion for named values, which will allow for completion here and when building matchers.
> My next change will add that to dynamic::Parser.
OK, sounds good.

================
Comment at: clang-query/Query.cpp:131
@@ +130,3 @@
+  } else {
+    QS.NamedValues.erase(Name);
+  }
----------------
Sorry, one thing I forgot to mention before: is this the correct behavior if the value does not exist in the map? Should we be printing an error message in that case?

================
Comment at: unittests/clang-query/QueryParserTest.cpp:123
@@ +122,3 @@
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("expected variable name", cast<InvalidQuery>(Q)->ErrStr);
+}
----------------
Please also test that extra stuff after the unlet query causes an error.


http://reviews.llvm.org/D3383






More information about the cfe-commits mailing list