<div dir="ltr">Next attempt landed in 369527.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 21, 2019 at 3:40 PM Sam McCall <<a href="mailto:sam.mccall@gmail.com">sam.mccall@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I've reverted it.<div>I didn't realize people were using this copy of gtest without also using support/adt, so I guess we'll end up somewhere similar to the existing GTEST_NO_LLVM_RAW_OSTREAM.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 21, 2019 at 3:19 PM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The buildbots (and my local build) are sad, e.g.<br>
<br>
<a href="http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/30148/steps/ninja%20check%201/logs/stdio" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/30148/steps/ninja%20check%201/logs/stdio</a><br>
<a href="http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/20170/steps/ninja%20check%201/logs/stdio" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/20170/steps/ninja%20check%201/logs/stdio</a><br>
<br>
On Wed, Aug 21, 2019 at 1:35 PM Sam McCall via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: sammccall<br>
> Date: Wed Aug 21 04:37:06 2019<br>
> New Revision: 369518<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=369518&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=369518&view=rev</a><br>
> Log:<br>
> [gtest] Fix printing of StringRef and SmallString in assert messages.<br>
><br>
> Summary:<br>
> These are detected by gtest as containers, and so previously printed as e.g.<br>
> { '.' (46, 0x2E), 's' (115, 0x73), 'e' (101, 0x65), 'c' (99, 0x63), '0' (48, 0x30) },<br>
><br>
> gtest itself overloads PrintTo for std::string and friends, we use the same mechanism.<br>
><br>
> Reviewers: labath<br>
><br>
> Subscribers: dexonsmith, llvm-commits<br>
><br>
> Tags: #llvm<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D66520" rel="noreferrer" target="_blank">https://reviews.llvm.org/D66520</a><br>
><br>
> Modified:<br>
> llvm/trunk/unittests/ADT/SmallStringTest.cpp<br>
> llvm/trunk/unittests/ADT/StringRefTest.cpp<br>
> llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h<br>
><br>
> Modified: llvm/trunk/unittests/ADT/SmallStringTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallStringTest.cpp?rev=369518&r1=369517&r2=369518&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallStringTest.cpp?rev=369518&r1=369517&r2=369518&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/ADT/SmallStringTest.cpp (original)<br>
> +++ llvm/trunk/unittests/ADT/SmallStringTest.cpp Wed Aug 21 04:37:06 2019<br>
> @@ -169,7 +169,7 @@ TEST_F(SmallStringTest, Realloc) {<br>
> EXPECT_EQ("abcdyyy", theString.slice(0, 7));<br>
> }<br>
><br>
> -TEST(StringRefTest, Comparisons) {<br>
> +TEST_F(SmallStringTest, Comparisons) {<br>
> EXPECT_EQ(-1, SmallString<10>("aab").compare("aad"));<br>
> EXPECT_EQ( 0, SmallString<10>("aab").compare("aab"));<br>
> EXPECT_EQ( 1, SmallString<10>("aab").compare("aaa"));<br>
> @@ -203,4 +203,12 @@ TEST(StringRefTest, Comparisons) {<br>
> EXPECT_EQ( 1, SmallString<10>("V8_q0").compare_numeric("V1_q0"));<br>
> }<br>
><br>
> +// Check gtest prints SmallString as a string instead of a container of chars.<br>
> +// The code is in utils/unittest/googletest/internal/custom/gtest-printers.h<br>
> +TEST_F(SmallStringTest, GTestPrinter) {<br>
> + EXPECT_EQ(R"("foo")", ::testing::PrintToString(SmallString<1>("foo")));<br>
> + const SmallVectorImpl<char> &ErasedSmallString = SmallString<1>("foo");<br>
> + EXPECT_EQ(R"("foo")", ::testing::PrintToString(ErasedSmallString));<br>
> }<br>
> +<br>
> +} // namespace<br>
><br>
> Modified: llvm/trunk/unittests/ADT/StringRefTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringRefTest.cpp?rev=369518&r1=369517&r2=369518&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringRefTest.cpp?rev=369518&r1=369517&r2=369518&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/ADT/StringRefTest.cpp (original)<br>
> +++ llvm/trunk/unittests/ADT/StringRefTest.cpp Wed Aug 21 04:37:06 2019<br>
> @@ -1055,6 +1055,12 @@ TEST(StringRefTest, StringLiteral) {<br>
> EXPECT_EQ(StringRef("Bar"), Strings[1]);<br>
> }<br>
><br>
> +// Check gtest prints StringRef as a string instead of a container of chars.<br>
> +// The code is in utils/unittest/googletest/internal/custom/gtest-printers.h<br>
> +TEST(StringRefTest, GTestPrinter) {<br>
> + EXPECT_EQ(R"("foo")", ::testing::PrintToString(StringRef("foo")));<br>
> +}<br>
> +<br>
> static_assert(is_trivially_copyable<StringRef>::value, "trivially copyable");<br>
><br>
> } // end anonymous namespace<br>
><br>
> Modified: llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h?rev=369518&r1=369517&r2=369518&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h?rev=369518&r1=369517&r2=369518&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h (original)<br>
> +++ llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h Wed Aug 21 04:37:06 2019<br>
> @@ -39,4 +39,33 @@<br>
> #ifndef GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_<br>
> #define GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_<br>
><br>
> +#include "llvm/ADT/SmallString.h"<br>
> +#include "llvm/ADT/StringRef.h"<br>
> +#include <ostream><br>
> +<br>
> +namespace llvm {<br>
> +<br>
> +// Printing of llvm String types.<br>
> +// gtest sees these as containers of char (they have nested iterator types),<br>
> +// so their operator<< is never considered unless we provide PrintTo().<br>
> +// PrintStringTo provides quotes and escaping, at the cost of a copy.<br>
> +<br>
> +inline void PrintTo(llvm::StringRef S, std::ostream *OS) {<br>
> + *OS << ::testing::PrintToString(S.str());<br>
> +}<br>
> +// We need both SmallString<N> and SmallVectorImpl<char> overloads:<br>
> +// - the SmallString<N> template is needed as overload resolution will<br>
> +// instantiate generic PrintTo<T> rather than do derived-to-base conversion<br>
> +// - but SmallVectorImpl<char> is sometimes the actual static type, in code<br>
> +// that erases the small size<br>
> +template <unsigned N><br>
> +inline void PrintTo(const SmallString<N> &S, std::ostream *OS) {<br>
> + *OS << ::testing::PrintToString(std::string(S.data(), S.size()));<br>
> +}<br>
> +inline void PrintTo(const SmallVectorImpl<char> &S, std::ostream *OS) {<br>
> + *OS << ::testing::PrintToString(std::string(S.data(), S.size()));<br>
> +}<br>
> +<br>
> +} // namespace llvm<br>
> +<br>
> #endif // GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>