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

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:632
+variables and checking a numeric expression is thus
+``[[#<NUMVAR>: <constraint> <expr>]]`` with each element as described
+previously.
----------------
Where is <constraint> described?


================
Comment at: llvm/lib/Support/FileCheck.cpp:40
+    // call this method if the expression associated to this variable can be
+    // evaluated.
+    assert(EvaluatedValue &&
----------------
You should check the Expected return value with something other than an assert.  It is not sufficient to claim that the caller should call this only when the Expected result must be success. The contract for using Expected is that you will check the result. You could do something like
```
if (!EvaluatedValue || *EvaluatedValue != NewValue)
  llvm_unreachable("New value does not match expression for this variable");
```



================
Comment at: llvm/lib/Support/FileCheck.cpp:471
       StringRef SubstStr;
-      StringRef MatchRegexp;
+      StringRef MatchRegexp = StringRef();
       size_t SubstInsertIdx = RegExStr.size();
----------------
? this should be the default, you should not have to say `= StringRef()` explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60388