[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:233
 
+Expected<ExpressionValue> llvm::max(const ExpressionValue &LeftOperand,
+                                    const ExpressionValue &RightOperand) {
----------------
paulwalker-arm wrote:
> jhenderson wrote:
> > Should this be an `Expected` if it can't fail? Same for `min`.
> This is to match the binop_eval_t typedef required by BinaryOperation.
Thanks, makes sense.


================
Comment at: llvm/lib/Support/FileCheck.cpp:374-380
+  for (unsigned I = 1, E = Str.size(); I != E; ++I) {
+    // Have we reached the end of the function name of a call expression?
+    if (Str[I] == '(')
+      return true;
+    if (!isValidVarNameChar(Str[I]))
+      break;
+  }
----------------
paulwalker-arm wrote:
> jhenderson wrote:
> > We should probably allow for optional whitespace between the end of the function name and the `(`.
> In LLVM's c/c++ world clang-format will remove such whitespace (presumably to aid readability) so do we really want to allow it in FileCheck code?
We've allowed arbitrary whitespace everywhere else in the expressions, so I think we should. Not all environments will necessarily follow LLVM's coding standards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79936