[PATCH] D79936: [FileCheck] Add function call support to numerical expressions.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:287
+  // Try to parse a function call.
+  if (Expr.startswith(CallPrefix)) {
+    if (AO != AllowedOperand::Any)
----------------
thopre wrote:
> jhenderson wrote:
> > I'm not strongly opposed to the use of `!` to indicate a function call, but is it actually necessary? It seems like a function call could just be identified by `<sequence of identifier chars>(<some chars>)`. The code would look something like the following semi-pseudo code:
> > 
> > ```
> > size_t Parenthesis = Expr.find_first_of('(');
> > if (Parenthesis != npos) {
> >   if (all_of(Expr.take_front(Parenthesis), [](char C) { 
> >       return isValidIdentifierChar(C);
> >     }) {
> >     if (AO != AllowedOperand::Any)
> >       return ErrorDiagnostic::get(SM, Expr, "unexpected function call");
> > 
> >     return parseCallExpr(Expr, LineNumber, Context, SM);
> >   }
> > }
> > ```
> > 
> > Assuming I've not missed something, that would allow us to simplify all the usages of function calls.
> Regardless of the ease of implementation, I like the ! prefix since these are builtin functions/operators, not something the user can define. YMMV of course
I'm not sure why it matters that they are builtin? Even if we do provide the ability for users to define their own functions, surely their behaviour should be identical to built-in functions from the majority of the code's point-of-view? I'd actually think that including the `!` would make it harder to parse, since we'd have to support function calls both with and without the `!`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79936/new/

https://reviews.llvm.org/D79936