[PATCH] D99614: [flang] Update list-input test to use GTest
Asher Mancinelli via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 12 08:28:00 PDT 2021
ashermancinelli added a comment.
@awarzynski Thanks again for the review! I have another patch ready to address this.
I typically use `static` to reduce the number of initializations, but you're right to point out that in these tests readability is probably more important than saving a few bytes (and most variables are PODs anyways).
================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:65
+ ASSERT_EQ(std::strcmp(outputBuffers[j], expectedOutput), 0)
+ << "wanted outputBuffers[" << j << "]=" << expectedOutput << ", got '"
+ << outputBuffers[j] << "'\n";
----------------
awarzynski wrote:
> Probably `expectedOutput`?
Could you elaborate? I'm not sure I understand.
================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:92
+ static std::int64_t have[listInputLength]{-1, -2, -3, -4, 5, -6, 7, -8, 9};
+ static const std::int64_t want[listInputLength]{1, 2, 3, 3, 5, 6, 7, 8, 9};
+ for (j = 0; j < listInputLength; ++j) {
----------------
awarzynski wrote:
> [nit] Perhaps `expectedOutput`? `want` is a bit problematic for me (you may `want` something, but does imply that you _can_ `expect` it?). I'm probably overthinking this!
Fair enough! If it's confusing for you, it'll certainly be confusing to future contributors.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99614/new/
https://reviews.llvm.org/D99614
More information about the llvm-commits
mailing list