[PATCH] D66520: [gtest] Fix printing of StringRef and SmallString in assert messages.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 04:02:02 PDT 2019


labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks for the quick turnaround. This looks good to me. If there's a way to use the regular `std::string` printer without going through the internal API, then I think it would be better to do that, but what you have here doesn't look too bad either.



================
Comment at: llvm/unittests/ADT/SmallStringTest.cpp:209
+TEST_F(SmallStringTest, GTestPrinter) {
+  EXPECT_EQ(R"("foo")", ::testing::PrintToString(SmallString<1>("foo")));
+  const SmallVectorImpl<char> &ErasedSmallString = SmallString<1>("foo");
----------------
jhenderson wrote:
> Do you really need the R"" syntax for the string here? It seems a bit unnecessary when "foo" should suffice.
I believe the string will come out as `"foo"`, which would be `"\"foo\""` in non-raw form.


================
Comment at: llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h:54
+inline void PrintTo(llvm::StringRef S, std::ostream *OS) {
+  ::testing::internal::PrintStringTo(S.str(), OS);
+}
----------------
Could these delegate to the "public" `StringTo` function instead of going for the internal API (`PrintTo(S.str(), OS)`, or whatever is the right ADL magic for calling that)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66520





More information about the llvm-commits mailing list