[PATCH] D60389: FileCheck [9/12]: Add support for matching formats

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

thopre added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:94-105
+  if (AllowHex) {
+    if (AllowUpperHex) {
+      EXPECT_EQ(*TenMatchingString, "A");
+      EXPECT_EQ(*FifteenMatchingString, "F");
+    } else {
+      EXPECT_EQ(*TenMatchingString, "a");
+      EXPECT_EQ(*FifteenMatchingString, "f");
----------------
jhenderson wrote:
> jhenderson wrote:
> > I'd recommend the following, to reduce duplication of checks.
> > ```
> > StringRef Ten;
> > StringRef Fifteen;
> > if (AllowHex) {
> >   if (AllowUpperHex) {
> >     Ten = "A";
> >     Fifteen = "F";
> >   } else {
> >     Ten = "a";
> >     Fifteen = "f";
> >   }
> > } else {
> >   Ten = "10";
> >   Fifteen = "15;
> > }
> > EXPECT_EQ(*TenMatchingString, Ten);
> > EXPECT_EQ(*FifteenMatchingString, Fifteen);
> > ```
> This doesn't appear to have been done here, only in the bufferized bit below.
My bad, I don't know how I missed that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60389