[PATCH] D60386: FileCheck [6/12]: Introduce numeric variable definition

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:606
 
 The above example would match the line:
 
----------------
"lines" (or maybe just "text" as I suspect you'll want to go back to the one-line example in patch 12 of this series).


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:613
 
 but would not match the line:
 
----------------
"lines" (or "text")


================
Comment at: llvm/lib/Support/FileCheck.cpp:472
           return true;
         IsNumExpr = true;
+        if (DefinedNumericVariable) {
----------------
`IsNumExpr` is guaranteed true here, because of the simplified 'if' expression above.


================
Comment at: llvm/test/FileCheck/numeric-expression.txt:78
 ; Numeric expression with unsupported operator
-RUN: not FileCheck -D#VAR1=11 --check-prefixes CHECK,INVAL-OP --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix INVAL-OP-MSG %s
+RUN: not FileCheck -check-prefixes CHECK,INVAL-OP -input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace -check-prefix INVAL-OP-MSG %s
----------------
It appears that the only reason to include the CHECK prefix here is so it will define VAR1 for you? Maybe use a different variable instead, so the sections of the test are less intertwined.


================
Comment at: llvm/test/FileCheck/numeric-expression.txt:90
 ; Name conflict between Numeric variable definition and pattern variable
-; definition
-RUN: not FileCheck -D#VAR1=11 -D#NUMVAR=42 --check-prefixes CONFLICT,CONFLICT1 --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-INPUT-PAT-CONFLICT %s
-RUN: not FileCheck -D#VAR1=11 -D#NUMVAR=42 -DNUMVAR=foobar --check-prefix CONFLICT --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-CLI-PAT-CONFLICT %s
-RUN: not FileCheck -D#VAR1=11 -DPATVAR=foobar -D#PATVAR=42 --check-prefix CONFLICT --input-file %s %s 2>&1 \
-RUN:   | FileCheck --strict-whitespace --check-prefix CLI-CLI-NUM-CONFLICT %s
+; definition whether from the command-line or input text
+RUN: not FileCheck -check-prefixes CONFLICT,CONFLICT1,CONFLICT2 -input-file %s %s 2>&1 \
----------------
Needs a period/full-stop at the end of the sentence.


================
Comment at: llvm/test/FileCheck/var-scope.txt:4
 
-RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' --check-prefixes CHECK,LOCAL3,GLOBAL --input-file %s %s
-RUN: FileCheck -D#LOCNUM=1 -D'#$GLOBNUM=1' --check-prefixes CHECK,GLOBAL --enable-var-scope --input-file %s %s
-RUN: not FileCheck -D#LOCNUM=1 -D#'$GLOBNUM=1' --check-prefixes CHECK,LOCAL1 --enable-var-scope --input-file %s %s 2>&1 \
+RUN: FileCheck --check-prefixes CHECK,LOCAL3,GLOBAL --input-file %s %s
+RUN: FileCheck --check-prefixes CHECK,GLOBAL --enable-var-scope --input-file %s %s
----------------
Please add a comment noting that this first RUN doesn't enable scoping.
(I've been caught by that twice now!  Clearly I need help reading tests.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60386