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

[sammccall]

Summary

These are detected by gtest as containers, and so previously printed as e.g.

{ '.' (46, 0x2E), 's' (115, 0x73), 'e' (101, 0x65), 'c' (99, 0x63), '0' (48, 0x30) },

gtest itself overloads PrintTo for std::string and friends, we use the same mechanism.

[sammccall]

revert extraneous include

[jhenderson]

llvm/unittests/ADT/SmallStringTest.cpp

206 ↗

(On Diff #216361)

as a string

209 ↗

(On Diff #216361)

Do you really need the R"" syntax for the string here? It seems a bit unnecessary when "foo" should suffice.

llvm/unittests/ADT/StringRefTest.cpp

1058 ↗

(On Diff #216361)

as a string

[labath]

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.

llvm/unittests/ADT/SmallStringTest.cpp

209 ↗

(On Diff #216361)

I believe the string will come out as "foo", which would be "\"foo\"" in non-raw form.

llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h

54 ↗

(On Diff #216361)

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)?

[sammccall]

review comments

[sammccall]

llvm/unittests/ADT/SmallStringTest.cpp

209 ↗

(On Diff #216361)

Right: EXPECT_EQ("\"foo\"", ...) doesn't seem any more readable to me.

llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h

54 ↗

(On Diff #216361)

Done via PrintToString. It's an extra copy, but I don't think it matters here.

We can't call the std::string overload of PrintTo via ADL as it's not in namespace std, gtest relies on regular lookup from a callsite inside ::testing::internal for that one.

(I think ::testing::internal::PrintTo might actually be a badly-named *public* API: it's documented as such in gtest-printers.h alongside ::testing::PrintToString. But no reason to worry about the ambiguity)

llvm/trunk/unittests/ADT/SmallStringTest.cpp

TEST_F(SmallStringTest, Realloc) {
   EXPECT_EQ("abcd", theString);
   unsigned const N = 100000;
   theString.reserve(N);
   for (unsigned i = 0; i < N - 4; ++i)
     theString.push_back('y');
   EXPECT_EQ("abcdyyy", theString.slice(0, 7));
 }
 
-TEST(StringRefTest, Comparisons) {
+TEST_F(SmallStringTest, Comparisons) {
   EXPECT_EQ(-1, SmallString<10>("aab").compare("aad"));
   EXPECT_EQ( 0, SmallString<10>("aab").compare("aab"));
   EXPECT_EQ( 1, SmallString<10>("aab").compare("aaa"));
   EXPECT_EQ(-1, SmallString<10>("aab").compare("aabb"));
   EXPECT_EQ( 1, SmallString<10>("aab").compare("aa"));
   EXPECT_EQ( 1, SmallString<10>("\xFF").compare("\1"));
 
   EXPECT_EQ(-1, SmallString<10>("AaB").compare_lower("aAd"));
TEST_F(SmallStringTest, Comparisons) {
   EXPECT_EQ( 1, SmallString<10>("V16").compare_numeric("V1_q0"));
   EXPECT_EQ(-1, SmallString<10>("V1_q0").compare_numeric("V16"));
   EXPECT_EQ(-1, SmallString<10>("V8_q0").compare_numeric("V16"));
   EXPECT_EQ( 1, SmallString<10>("V16").compare_numeric("V8_q0"));
   EXPECT_EQ(-1, SmallString<10>("V1_q0").compare_numeric("V8_q0"));
   EXPECT_EQ( 1, SmallString<10>("V8_q0").compare_numeric("V1_q0"));
 }
 
+// Check gtest prints SmallString as a string instead of a container of chars.
+// The code is in utils/unittest/googletest/internal/custom/gtest-printers.h
+TEST_F(SmallStringTest, GTestPrinter) {
+  EXPECT_EQ(R"("foo")", ::testing::PrintToString(SmallString<1>("foo")));
+  const SmallVectorImpl<char> &ErasedSmallString = SmallString<1>("foo");
+  EXPECT_EQ(R"("foo")", ::testing::PrintToString(ErasedSmallString));
 }
+
+} // namespace

llvm/trunk/unittests/ADT/StringRefTest.cpp

 }
 
 TEST(StringRefTest, StringLiteral) {
   constexpr StringLiteral Strings[] = {"Foo", "Bar"};
   EXPECT_EQ(StringRef("Foo"), Strings[0]);
   EXPECT_EQ(StringRef("Bar"), Strings[1]);
 }
 
+// Check gtest prints StringRef as a string instead of a container of chars.
+// The code is in utils/unittest/googletest/internal/custom/gtest-printers.h
+TEST(StringRefTest, GTestPrinter) {
+  EXPECT_EQ(R"("foo")", ::testing::PrintToString(StringRef("foo")));
+}
+
 static_assert(is_trivially_copyable<StringRef>::value, "trivially copyable");
 
 } // end anonymous namespace

llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h

 // See documentation at gtest/gtest-printers.h for details on how to define a
 // custom printer.
 //
 // ** Custom implementation starts here **
 
 #ifndef GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
 #define GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
 
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include <ostream>
+
+namespace llvm {
+
+// Printing of llvm String types.
+// gtest sees these as containers of char (they have nested iterator types),
+// so their operator<< is never considered unless we provide PrintTo().
+// PrintStringTo provides quotes and escaping, at the cost of a copy.
+
+inline void PrintTo(llvm::StringRef S, std::ostream *OS) {
+  *OS << ::testing::PrintToString(S.str());
+}
+// We need both SmallString<N> and SmallVectorImpl<char> overloads:
+//  - the SmallString<N> template is needed as overload resolution will
+//    instantiate generic PrintTo<T> rather than do derived-to-base conversion
+//  - but SmallVectorImpl<char> is sometimes the actual static type, in code
+//    that erases the small size
+template <unsigned N>
+inline void PrintTo(const SmallString<N> &S, std::ostream *OS) {
+  *OS << ::testing::PrintToString(std::string(S.data(), S.size()));
+}
+inline void PrintTo(const SmallVectorImpl<char> &S, std::ostream *OS) {
+  *OS << ::testing::PrintToString(std::string(S.data(), S.size()));
+}
+
+} // namespace llvm
+
 #endif  // GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_