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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:44
+/// Class representing a numeric expression.
+class FileCheckNumExpr {
+private:
----------------
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?


================
Comment at: llvm/lib/Support/FileCheck.cpp:241
+    // substitution mode. Numeric variable names have the same restriction as
+    // their pattern variable counterpart.
     if (PatternStr.startswith("[[")) {
----------------
probinson wrote:
> I thought numeric variables didn't exist until a later patch?
Regular numeric variables yes but this patch rebrand @LINE as being a pseudo numeric variable and expressions being based on it as being a numeric expression (if of the form [[#<something>]]) or legacy numeric expression.


================
Comment at: llvm/test/FileCheck/line-count.txt:12
 ; RUN: not FileCheck -check-prefix BAD10 -input-file %s %s 2>&1 | FileCheck -check-prefix ERR10 %s
 ; RUN: not FileCheck -check-prefix BAD10 -input-file %s %s 2>&1 | FileCheck -check-prefix ERR10 %s
 13
----------------
jhenderson wrote:
> Missed this previously. This should be BAD11 and ERR11, right?
Oops good catch. Had to fix the test as well so I've put it as a separate patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60384