[PATCH] D60384: FileCheck [4/12]: Introduce @LINE numeric expressions

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:44
+/// Class representing a numeric expression.
+class FileCheckNumExpr {
+private:
----------------
thopre wrote:
> jhenderson wrote:
> > I don't think this is a strict requirement, but I think it would be worth putting everything in a namespace, to allow these classes to all be called NumericExpression, PatternContext etc without any abbreviations (or with fewer). That would make this code a little more descriptive and less repetitive everywhere.
> > 
> > If you did that, you should probably do it in a different change.
> You mean put all FileCheck classes in a FileCheck namespace?
Yes, that was what I was referring to (or similar to that anyway). Quite possibly this was discussed before.


================
Comment at: llvm/lib/Support/FileCheck.cpp:241
+    // Both pattern variables and numeric variables must satisfy the regular
+    // expression "[a-zA-Z_][0-9a-zA-Z_]*" to be valid, as this help catch some
+    // common errors.
----------------
help -> helps


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:149
 
+TEST_F(FileCheckTest, Substitute) {
+  SourceMgr SM;
----------------
Substitute -> Substitution


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60384