[PATCH] D60385: FileCheck [5/12]: Introduce regular numeric variables

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

I am pretty happy with this now, maybe give the other people who have been participating a day or so to chime in again.



================
Comment at: llvm/lib/Support/FileCheck.cpp:27-51
+/// Return value of this numeric variable.
+llvm::Optional<uint64_t> FileCheckNumExprVar::getValue() const {
+  if (!Defined)
+    return llvm::None;
+  return Value;
+}
+
----------------
arsenm wrote:
> thopre wrote:
> > thopre wrote:
> > > arsenm wrote:
> > > > This can go away if you just use Optional as the member
> > > Sorry I don't follow. An Optional variable as the member of the binop? How does that help? I still have to check whether that variable has a value or not.
> > Ping?
> I'm confused because the code here doesn't match what I see in the Phabricator email anymore.
> 
> If you are separately tracking Defined and returning None or Value, this is the same as just returning an Optional member
I think the comment was originally on FileCheckNumericVariable, which used to have a separate bool, but @thopre changed this to an Optional a while ago. So this is done.


================
Comment at: llvm/lib/Support/FileCheck.cpp:356
+
+        // Detect collision between pattern and numeric variable when the former
+        // is created later than the latter.
----------------
collisions ... variables


================
Comment at: llvm/lib/Support/FileCheck.cpp:1588
+
+      // Detect collision between pattern and numeric variable when the latter
+      // is created later than the former.
----------------
collisions ... variables


================
Comment at: llvm/lib/Support/FileCheck.cpp:1628
+
+      // Detect collision between pattern and numeric variable when the former
+      // is created later than the latter.
----------------
collisions ... variables


================
Comment at: llvm/test/FileCheck/numeric-defines-diagnostics.txt:30
+RUN:   | FileCheck %s --strict-whitespace --check-prefix NUMERRCLIEXPR
+
+NUMERRCLIEXPR: Global defines:2:27: error: invalid value in numeric variable definition 'VALUE1 + 2'
----------------
Maybe also `-D#VALUE3=VALUE1+2` (i.e. without quotes or whitespace).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60385