[PATCH] D60382: FileCheck [2/12]: Stricter parsing of -D option

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

thopre marked an inline comment as done.
thopre added inline comments.


================
Comment at: llvm/test/FileCheck/defines.txt:38
+
+; ERRCLIFMT: Global defines:1:1: error: Invalid name for variable definition '10VALUE'
+; ERRCLIFMT-NEXT: 10VALUE=10
----------------
jhenderson wrote:
> The line and column numbers like a bit weird to me, but isn't a big deal. If there's a way to not have them though, it would be better. Alternatively, if they have some actual meaning, please have a test case that produces different values for them.
Not have them in the output or in this expected text?

If the former yes currently it's not meaningful because the current code create a separate buffer with same name (Global defines) for each definition, so all definition would be a line 1, and the name would be a column 1 (I think it's good to test that bit since it shows we point to the variable name rather than the value).
I could change the code to have one line per definition which could be useful if a variable is defined twice from the command-line (i.e. -DFOO=10 -DFOO=12). Alternatively I could make the definition one after the other on the same line, perhaps separated by space (i.e. output would be FOO=10 FOO=12 with the caret pointing to the right FOO). Both of these approaches will require to loop over definition twice (once to construct the buffer, the second to parse it).

If the latter, I cannot introduce variations because the name will always be first but I think it's worth testing that the location is accurate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60382