[PATCH] D60390: FileCheck [10/12]: Add support for signed numeric values

[Alexander Richardson via Phabricator via llvm-commits]

arichardson accepted this revision.
arichardson added a comment.

Looks good to me once the tests have been added and other reviewers are also happy.



================
Comment at: llvm/lib/Support/FileCheck.cpp:125
+  // Operands must have same sign.
+  if (Op1.Signed != Op2.Signed)
+    return make_error<ValueError>(ValPromotion);
----------------
This is fine for the initial version of the patch, but I think in the future it will be useful to permit adding an unsigned variable to a signed one, etc. as long as the result is still within the representable range.


================
Comment at: llvm/lib/Support/FileCheck.cpp:201
+  ExpressionValue RightValue = *RightOp;
+  if (!LeftValue.isSigned() || !RightValue.isSigned()) {
+    Error ConvError = LeftValue.convertUnsigned();
----------------
I'm not sure if this is necessary but I feel like it may be useful to permit adding a signed (negative value) to an unsigned value as long as the result is >= 0. Maybe these checks should move into operator+/-?


================
Comment at: llvm/test/FileCheck/numeric-expression.txt:67
 USE DEF FMT IMPL MATCH
 11
 12
----------------
Since there are quite a lot of CHECK lines now, it might be helpful to add a comment after every line that checks a new variable. For example:

```
11 // VAR1
12
10
-30 // VAR2
-29
-31
c // VAR3 (hex)
...
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60390