Page MenuHomePhabricator

[FileCheck] Add multiplication support to numerical expressions.
AbandonedPublic

Authored by paulwalker-arm on May 13 2020, 9:58 AM.

Details

Reviewers
thopre

Diff Detail

Event Timeline

paulwalker-arm created this revision.May 13 2020, 9:58 AM

Thanks for this change, multiplication seems like a much needed addition to the numeric expression support indeed.

This changes gives the same priority to the multiplication operator as the addition and subtraction operators. That's counter-intuitive and is sure to bite people later on. The change will also need to update the documentation (llvm/docs/CommandGuide/FileCheck.rst) since it mentions valid operators.

We also added multiplication and division locally in the CHERI fork of LLVM. However, I didn't try upstreaming it since I feel like operator precedence needs to be solved first.
D77383 adds support for parenthesized expressions that allows working around the lack of operator precendence, but I haven't had time to update it yet.

I understand the operator precedence might surprise some (although personally I just assumed a left->right evaluation order) but when taken with named operators (https://reviews.llvm.org/D79882) is it worth enforcing and documenting this simple precedence order rather than complicating things? With the bracket feature providing a route to a custom evaluation order.

Updated FileCheck documentation.

I understand the operator precedence might surprise some (although personally I just assumed a left->right evaluation order) but when taken with named operators (https://reviews.llvm.org/D79882) is it worth enforcing and documenting this simple precedence order rather than complicating things? With the bracket feature providing a route to a custom evaluation order.

Using the functional notation as I suggested would allow to clarify how is the evaluation to be performed since it forces parenthesis around the arguments.

I've created D79936 as a potential replacement for this patch.

paulwalker-arm abandoned this revision.May 15 2020, 10:44 AM