[PATCH] D60381: FileCheck [1/12]: Move variable table in new object

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

thopre marked 2 inline comments as done.
thopre added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:44
+  EXPECT_FALSE(LocalVar);
+  EXPECT_TRUE(GlobalVar);
+}
----------------
thopre wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > thopre wrote:
> > > > jhenderson wrote:
> > > > > I would add one last check to this test: re-add a local variable and show that you can fetch it again.
> > > > The API does not provide a function to add a variable so I'm not sure how I would do that. FileCheckPattern is friend of FileCheckPatternContext so can add it directly so as to avoid creating a any user of the class to be able to add variables.
> > > Okay, just waiting for this point to be addressed before I LGTM this.
> > Oh, right, yes of course. I forgot you decided to go the assert route in defineCmdlineVariables. Perhaps in another test you could run defineCmdlineVariables with just locals, clear them, then run it again?
> See my answer above. Do you want me to create a function to set a variable and make it public?
Oops, just saw your comment about local vars after I wrote the above message. Will fix that immediately


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60381