[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