[PATCH] D80703: [FileCheck][unittest] Use parameterized unittests

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Honestly, my impression is that the code on the left reads much easier. And it is 100 lines shorter.
I am not familar with things like `::testing::TestWithParam` though, may be there is a benefit of using it
that is not obvious to me atm.

I wonder what do other people think.



================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:296
 
-template <class T1, class T2>
-static Expected<ExpressionValue> doValueOperation(binop_eval_t Operation,
-                                                  T1 LeftValue, T2 RightValue) {
-  ExpressionValue LeftOperand(LeftValue);
-  ExpressionValue RightOperand(RightValue);
-  return Operation(LeftOperand, RightOperand);
-}
+const int64_t MinInt64 = std::numeric_limits<int64_t>::min();
 
----------------
You can use `INT64_MAX` instead.
(`INT32_MAX`/`UINT32_MAX` are used in many places in LLVM).


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:358
+
 const int64_t MaxInt64 = std::numeric_limits<int64_t>::max();
 
----------------
`UINT32_MAX`?


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:418
+      EXPECT_EQ(*SignedExpectedValue, *SignedActualValue);
+    } else {
+      Expected<uint64_t> UnsignedExpectedValue =
----------------
You can `return` here to avoid `else`:

```
      ASSERT_THAT_EXPECTED(SignedActualValue, Succeeded());
      EXPECT_EQ(*SignedExpectedValue, *SignedActualValue);
      return;
    } 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80703