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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm marked 2 inline comments as done.
paulwalker-arm added inline comments.


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:693
+  * a function call, or
+  * an expression followed by an operator and an expression.
 
----------------
thopre wrote:
> paulwalker-arm wrote:
> > thopre wrote:
> > > Why change this sentence? Recursion only happens on one of the operand only.
> > I don't understand the distinction. I changed the sentence as I didn't what to say:
> > ```
> > an expression followed by an operator and either a numerical operand or a function call.
> > ```
> > 
> > Are you saying the above is more correct?
> > 
> > Also with functions you can have 
> > ```
> > !(mul(VAR+(umin(VAR2,4)) +  !(udivl(VAR3+(umax(VAR3,4))
> > ```
> Oh right, I forgot about function altogether. That said, I think an expression is a kind of numeric operand so we should just expand the definition below saying it can also be a function call.
So something like:
```
A numeric operand is a previously defined numeric variable, an integer literal or the result of a function call.
```


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:708-709
+
+  ``!(<expr>)`` is a precedence operator, which can be used to force the order
+  of operations when symbolic operators are in use.
+
----------------
arichardson wrote:
> thopre wrote:
> > paulwalker-arm wrote:
> > > thopre wrote:
> > > > I don't think the exclamation mark should be required here. A parenthesis pair should be enough to force precedence. Note that there was a patch to adds support for that and you might want to rebase your patch on top of it.
> > > Sure, it's more that this implementation comes for free, whereas supporting arbitrary parenthesis pairs along side function calls requires more complex parsing.  Is it worth the extra effort?  If so I'll happy prevent this usage and error out for unnamed functions.
> > > 
> > > I'd rather not wait for the parenthesis work because I've got other work at review whose tests are much improved when I can make use of function calls.
> > I think the exclamation mark should be reserved for function call. I find it a bit confusing otherwise but let's wait to see what other reviewers might think.
> > 
> > Do the tests you need this feature for require some way to force precedence or can this be dealt later?
> Apologies for the delay, I've now rebased the parentheses revision (D77383).
I currently have no use for the precedence operator, the function calls are why I started down this path, so am happy either way.

Part of me just assumed that in the future the parser might be reworked to remove the need for the ! prefix, but I suppose providing the power of precedence early might prevent that work from happening :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79936