[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