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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
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.
+
----------------
paulwalker-arm wrote:
> thopre wrote:
> > paulwalker-arm wrote:
> > > 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 :)
> > I wouldn't hold my breath for a parser rewrite. While I'd love to make the code nicer and more flexible I have little free time to work on it myself. Anyway, if you don't require operator precedence just error on empty function name for now and we can extend it to be used for operator precedence later if needed.
> Done. Perhaps it's a good idea to mandate that all symbolic operators have a named function counterpart.  That way in the short term if somebody does want to force precedence they just need to write a slightly more verbose check line.
No objection to that, should be a 2 line changes, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79936