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

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

thopre added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:23
+  EXPECT_TRUE(Value);
+  EXPECT_EQ((uint64_t)42, *Value);
+  EXPECT_TRUE(FooVar.setValue((uint64_t)43));
----------------
jhenderson wrote:
> thopre wrote:
> > jhenderson wrote:
> > > What's the purpose of the cast here and elsewhere in this test?
> > To avoid a warning about comparison between sined (42) and unsigned (*Value which is uint64_t) integers. Elsewhere I've suffixed the literal with U, I've now normalized and that approach (ie. 42U in this case)
> Okay. I feel like it's a bug if that warning comes out and the signed number is known to be a positive literal, but that's a separate issue.
Yes but not a bug in FileCheck :-)


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:24
+  EXPECT_EQ(42U, *Value);
+  EXPECT_TRUE(FooVar.setValue(43));
+
----------------
jhenderson wrote:
> Check getValue() here after setting, to show that the new value is returned for a defined variable.
I suppose you mean the old value. As mentioned in the comment, setValue is supposed to fail when the variable is already defined and the value is then unchanged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60385