[PATCH] D60390: FileCheck [10/12]: Add support for signed numeric values

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 28 03:12:59 PDT 2020


thopre added a comment.

In D60390#2056652 <https://reviews.llvm.org/D60390#2056652>, @jhenderson wrote:

> LGTM, with nit.
>
> Re. TEST_P, the best suggestion I've got is look at the examples in DWARFDebugLineTest.cpp, where I've used a few different tricks to achieve the goal of code re-use. In particular, `AdjustAddressFixtureBase` related test cases might be interesting to you. In your case, I'd probably have a set of parameters each for failing and passing combinations, and for addition and subtraction, for a total of 4 combinations, if I'm not mistaken. You'd have thin fixtures for each, but they could probably share some code via a base class.
>
> That all being said, only make those changes if you think they are worthwhile. I'm happy enough with this as-is.


So far, I'm getting:

llvm/unittests/Support/FileCheckTest.cpp | 413 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 263 insertions(+), 150 deletions(-)

It's also quite a lot of code change, so even if that is acceptable because the code is deemed more readable, it's better to look at it as a separate patch and land the current code (with the suggested spacing fix)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60390





More information about the llvm-commits mailing list