[PATCH] D150880: [RFC, FileCheck] Allow AP value for numeric expressions

[Alexander Richardson via Phabricator via llvm-commits]

arichardson accepted this revision.
arichardson added a comment.
This revision is now accepted and ready to land.

Overall LGTM but as this is a rather large diff I may have missed something.



================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:153
+  APInt ResultValue;
+  bool ParseFailure = StrVal.getAsInteger(Hex ? 16 : 10, ResultValue);
+  (void)ParseFailure;
----------------
[[maybe_unused]] for the variable instead of the void cast?


================
Comment at: llvm/lib/FileCheck/FileCheckImpl.h:121
-/// Class representing a numeric value.
-class ExpressionValue {
-private:
----------------
To reduce the size of the diff we could have this class wrap a single apint but since you've made the changes to remove it already I think we may as well remove it.


================
Comment at: llvm/lib/FileCheck/FileCheckImpl.h:155
   /// AST or an error if evaluation fails.
-  virtual Expected<ExpressionValue> eval() const = 0;
+  virtual Expected<APInt> eval() const = 0;
 
----------------
Hmm maybe we should add `using ExpressionValue = APInt` to avoid all these changes.


================
Comment at: llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt:4
-;
-; At one time, FileCheck's exit status for the following example was zero even
-; though the excluded pattern does match (it's just capturing that fails).
----------------
It seems like some of this test is still useful, we just have to create a different error?


================
Comment at: llvm/unittests/FileCheck/FileCheckTest.cpp:384
+  if (std::is_unsigned_v<T>)
+    return APInt(128, Value);
+  return APInt(128, Value, /*IsSigned=*/true);
----------------
The test previously used 64 bit values, do we need 128 bit here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150880