[PATCH] D64230: [FileCheck] Record numeric variable availability

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

thopre added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:52
 
-  /// Line number where this variable is defined. Used to determine whether a
-  /// variable is defined on the same line as a given use.
-  size_t DefLineNumber;
+  /// Line number where this variable's value becomes available.
+  size_t AvailLineNumber;
----------------
jhenderson wrote:
> I'm not sure it's clear to me what "available" means in this context if it doesn't mean definition.
The change in naming is mostly for pseudo variables (@LINE being the only at the moment) where the definition is implicit and thus there isn't a definition line number per se. However I think it makes sense to talk about value availability (e.g. @LINE only has a value from the first line in the CHECK file, whereas other variable have their value available from after their definition).


================
Comment at: llvm/include/llvm/Support/FileCheck.h:58
+  /// and whose value becomes available at line \p AvailLineNumber.
+  FileCheckNumericVariable(StringRef Name, size_t AvailLineNumber)
+      : Name(Name), AvailLineNumber(AvailLineNumber) {}
----------------
jhenderson wrote:
> Not that I'm bothered really, but is there a reason to change the signature order here?
1) Consistency
2) I usually put the most important parameters first and the name of a variable feels more essential (since that's how you refer to it). The line number is only to catch some incorrect uses


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64230