[PATCH] Add support for named values in the parser.

[Peter Collingbourne]

  Can you please revert your commit and create a new revision? We can continue the review there.


================
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:431
@@ +430,3 @@
+        S->getNamedValue(Tokenizer->peekNextToken().Text);
+    if (!NamedValue.isNothing()) {
+      Tokenizer->consumeNextToken();  // Actually consume it.
----------------
Samuel Benzaquen wrote:
> Peter Collingbourne wrote:
> > What if the user defines a named value with the same name as a matcher? Yes, probably a bad idea, but it's easy to disambiguate between a matcher expression and a named value by checking whether the next token is a '('. That would also let us give better diagnostics (i.e. "no such named value" instead of "no such matcher").
> > 
> > It would also be nice (but not required for this change) if it were possible to bind against named values.
> I thought about it.
> What happens if you have: "expr". What error should that give?
> Right now it gives "missing (". If I change it like you say it will give "no such value", which might be confusing for anyone that doesn't know about named values.
> 
> Another alternative would be to accept "expr" as a matcher with no arguments. That way I try a named value, and then a matcher with no args. But what happens if it can't find any? which error should it give then?
> 
> wrt binding named values, the user can always wrap it in a bindable thing, like expr(my_conditions).bind("foo").
> What happens if you have: "expr". What error should that give?

My intuition is that because tab completion inserts the '(' for you, people wouldn't normally be making that kind of mistake.

That said, one other possibility is to disambiguate first based on whether the identifier names only a named value or a matcher, and if that fails (either because we have both or neither) we look at the following token. So if the identifier names a known matcher and we do not have a named value with that name, we would diagnose "missing '('".

> Another alternative would be to accept "expr" as a matcher with no arguments.

I don't think we should do this, mainly because the syntax wouldn't be compatible with that for static matchers.

> wrt binding named values, the user can always wrap it in a bindable thing, like expr(my_conditions).bind("foo").

Sure, but it would be nice if the user doesn't have to do that.


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

COMMIT
  http://llvm-reviews.chandlerc.com/rL205419