[PATCH] D60388: FileCheck [8/12]: Define numeric var from expr

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:37
+  if (ExpressionAST != nullptr) {
+    Expected<uint64_t> EvaluatedValue = ExpressionAST->eval();
+    assert(EvaluatedValue &&
----------------
thopre wrote:
> jhenderson wrote:
> > This is unchecked in builds without assertions.
> That's fine. setValue is called after substitution has happened and match was successful. Therefore ExpressionAST ought to evaluate fine. This is just in case code is reworked in the future and this invariant is broken.
It isn't. LLVM Errors and Expecteds HAVE to be checked. If they aren't it will lead to an abort. From the Programmer's Manual:

> All Error instances, whether success or failure, must be either checked or moved from (via std::move or a return) before they are destructed. Accidentally discarding an unchecked error will cause a program abort at the point where the unchecked value’s destructor is run, making it easy to identify and fix violations of this rule.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60388