<div dir="ltr">Hi Galina,<div><br></div><div>I dug into this a bunch more.</div><div>There is an existing workaround in that test: #pragma GCC diagnostic ignored "-Wunused-function".</div><div>This works around the GCC bug, which is <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916">https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916</a></div><div><br></div><div>Unfortunately, GCC's warning suppression pragmas are fragile, and don't work for this case.</div><div>This is GCC bug <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431">https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431</a></div><div><br></div><div>I can't find a reasonable workaround for this combination of errors, so I'd suggest adding -Wno-unused-function to the buildbot. Thoughts?</div></div><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Feb 12, 2018 at 11:06 PM Sam McCall <<a href="mailto:sam.mccall@gmail.com">sam.mccall@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto">Hi Galina,<div dir="auto"><br></div><div dir="auto">Thanks for the pointer.<br><div dir="auto"><br><div dir="auto">I had a quick look already:</div><div dir="auto">- this is definitely a GCC bug, and not one I recognize</div><div dir="auto">- the trigger is unmodified third-party code (my patch just tickled it)</div><div dir="auto"><br></div><div dir="auto">I'll get a GCC 7.1 set up in the CET morning and hunt for a workaround. If I can't find one I'll revert.</div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Feb 12, 2018, 22:53 Galina Kistanova <<a href="mailto:gkistanova@gmail.com" target="_blank">gkistanova@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hello Sam,<br><br>This commit  broke the one of our builders:<br><a href="http://lab.llvm.org:8011/builders/ubuntu-gcc7.1-werror/builds/5290" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/ubuntu-gcc7.1-werror/builds/5290</a><br><br>Please have a look?<br><br>Thanks<br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 12, 2018 at 2:20 AM, Sam McCall via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" rel="noreferrer" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: sammccall<br>
Date: Mon Feb 12 02:20:09 2018<br>
New Revision: 324876<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=324876&view=rev" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=324876&view=rev</a><br>
Log:<br>
[gtest] Support raw_ostream printing functions more comprehensively.<br>
<br>
Summary:<br>
These are functions like operator<<(raw_ostream&, Foo).<br>
<br>
Previously these were only supported for messages. In the assertion<br>
  EXPECT_EQ(A, B) << C;<br>
the local modifications would explicitly try to use raw_ostream printing for C.<br>
However A and B would look for a std::ostream printing function, and often fall<br>
back to gtest's default "168 byte object <00 01 FE 42 ...>".<br>
<br>
This patch pulls out the raw_ostream support into a new header under `custom/`.<br>
<br>
I changed the mechanism: instead of a convertible stream, we wrap the printed<br>
value in a proxy object to allow it to be sent to a std::ostream.<br>
I think the new way is clearer.<br>
<br>
I also changed the policy: we prefer raw_ostream printers over std::ostream<br>
ones. This is because the fallback printers are defined using std::ostream,<br>
while all the raw_ostream printers should be "good".<br>
<br>
Reviewers: ilya-biryukov, chandlerc<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D43091" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D43091</a><br>
<br>
Added:<br>
    llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h<br>
Modified:<br>
    llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h<br>
    llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h<br>
<br>
Modified: llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h?rev=324876&r1=324875&r2=324876&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h?rev=324876&r1=324875&r2=324876&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h (original)<br>
+++ llvm/trunk/utils/unittest/googletest/include/gtest/gtest-message.h Mon Feb 12 02:20:09 2018<br>
@@ -49,36 +49,7 @@<br>
 #include <limits><br>
<br>
 #include "gtest/internal/gtest-port.h"<br>
-<br>
-#if !GTEST_NO_LLVM_RAW_OSTREAM<br>
-#include "llvm/Support/raw_os_ostream.h"<br>
-<br>
-// LLVM INTERNAL CHANGE: To allow operator<< to work with both<br>
-// std::ostreams and LLVM's raw_ostreams, we define a special<br>
-// std::ostream with an implicit conversion to raw_ostream& and stream<br>
-// to that.  This causes the compiler to prefer std::ostream overloads<br>
-// but still find raw_ostream& overloads.<br>
-namespace llvm {<br>
-class convertible_fwd_ostream : public std::ostream {<br>
-  raw_os_ostream ros_;<br>
-<br>
-public:<br>
-  convertible_fwd_ostream(std::ostream& os)<br>
-    : std::ostream(os.rdbuf()), ros_(*this) {}<br>
-  operator raw_ostream&() { return ros_; }<br>
-};<br>
-}<br>
-template <typename T><br>
-inline void GTestStreamToHelper(std::ostream& os, const T& val) {<br>
-  llvm::convertible_fwd_ostream cos(os);<br>
-  cos << val;<br>
-}<br>
-#else<br>
-template <typename T><br>
-inline void GTestStreamToHelper(std::ostream& os, const T& val) {<br>
-  os << val;<br>
-}<br>
-#endif<br>
+#include "gtest/internal/custom/raw-ostream.h"<br>
<br>
 // Ensures that there is at least one operator<< in the global namespace.<br>
 // See Message& operator<<(...) below for why.<br>
@@ -157,12 +128,8 @@ class GTEST_API_ Message {<br>
     // from the global namespace.  With this using declaration,<br>
     // overloads of << defined in the global namespace and those<br>
     // visible via Koenig lookup are both exposed in this function.<br>
-#if GTEST_NO_LLVM_RAW_OSTREAM<br>
     using ::operator <<;<br>
-    *ss_ << val;<br>
-#else<br>
-    ::GTestStreamToHelper(*ss_, val);<br>
-#endif<br>
+    *ss_ << llvm_gtest::printable(val);<br>
     return *this;<br>
   }<br>
<br>
@@ -184,11 +151,7 @@ class GTEST_API_ Message {<br>
     if (pointer == NULL) {<br>
       *ss_ << "(null)";<br>
     } else {<br>
-#if GTEST_NO_LLVM_RAW_OSTREAM<br>
-      *ss_ << pointer;<br>
-#else<br>
-      ::GTestStreamToHelper(*ss_, pointer);<br>
-#endif<br>
+      *ss_ << llvm_gtest::printable(pointer);<br>
     }<br>
     return *this;<br>
   }<br>
<br>
Modified: llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h?rev=324876&r1=324875&r2=324876&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h?rev=324876&r1=324875&r2=324876&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h (original)<br>
+++ llvm/trunk/utils/unittest/googletest/include/gtest/gtest-printers.h Mon Feb 12 02:20:09 2018<br>
@@ -102,6 +102,7 @@<br>
 #include <vector><br>
 #include "gtest/internal/gtest-port.h"<br>
 #include "gtest/internal/gtest-internal.h"<br>
+#include "gtest/internal/custom/raw-ostream.h"<br>
<br>
 #if GTEST_HAS_STD_TUPLE_<br>
 # include <tuple><br>
@@ -246,7 +247,7 @@ void DefaultPrintNonContainerTo(const T&<br>
   // impossible to define #1 (e.g. when foo is ::std, defining<br>
   // anything in it is undefined behavior unless you are a compiler<br>
   // vendor.).<br>
-  *os << value;<br>
+  *os << ::llvm_gtest::printable(value);<br>
 }<br>
<br>
 }  // namespace testing_internal<br>
<br>
Added: llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h?rev=324876&view=auto" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h?rev=324876&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h (added)<br>
+++ llvm/trunk/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h Mon Feb 12 02:20:09 2018<br>
@@ -0,0 +1,74 @@<br>
+//===-- raw-ostream.h - Support for printing using raw_ostream --*- C++ -*-===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+// This file is not part of gtest, but extends it to support LLVM libraries.<br>
+// This is not a public API for testing - it's a detail of LLVM's gtest.<br>
+//<br>
+// gtest allows providing printers for custom types by defining operator<<.<br>
+// In LLVM, operator<< usually takes llvm:raw_ostream& instead of std::ostream&.<br>
+//<br>
+// This file defines a template printable(V), which returns a version of V that<br>
+// can be streamed into a std::ostream.<br>
+//<br>
+// This interface is chosen so that in the default case (printable(V) is V),<br>
+// the main gtest code calls operator<<(OS, V) itself. gtest-printers carefully<br>
+// controls the lookup to enable fallback printing (see testing::internal2).<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#ifndef GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_RAW_OSTREAM_H_<br>
+#define GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_RAW_OSTREAM_H_<br>
+<br>
+namespace llvm_gtest {<br>
+// StreamSwitch is a trait that tells us how to stream a T into a std::ostream.<br>
+// By default, we just stream the T directly. We'll specialize this later.<br>
+template <typename T, typename Enable = void> struct StreamSwitch {<br>
+  static const T& printable(const T& V) { return V; }<br>
+};<br>
+<br>
+// printable() returns a version of its argument that can be streamed into a<br>
+// std::ostream. This may be the argument itself, or some other representation.<br>
+template <typename T><br>
+auto printable(const T &V) -> decltype(StreamSwitch<T>::printable(V)) {<br>
+  // We delegate to the trait, to allow partial specialization.<br>
+  return StreamSwitch<T>::printable(V);<br>
+}<br>
+} // namespace llvm_gtest<br>
+<br>
+// If raw_ostream support is enabled, we specialize for types with operator<<<br>
+// that takes a raw_ostream.<br>
+#if !GTEST_NO_LLVM_RAW_OSTREAM<br>
+#include "llvm/Support/raw_ostream.h"<br>
+#include "llvm/Support/raw_os_ostream.h"<br>
+#include <ostream><br>
+namespace llvm_gtest {<br>
+<br>
+// The printable() of a raw_ostream-enabled type T is a RawStreamProxy<T>.<br>
+// It uses raw_os_ostream to write the wrapped value to a std::ostream.<br>
+template <typename T><br>
+struct RawStreamProxy {<br>
+  const T& V;<br>
+  friend std::ostream &operator<<(std::ostream &S, const RawStreamProxy<T> &V) {<br>
+    llvm::raw_os_ostream OS(S);<br>
+    OS << V.V;<br>
+    return S;<br>
+  }<br>
+};<br>
+<br>
+// We enable raw_ostream treatment if `(raw_ostream&) << (const T&)` is valid.<br>
+// We don't want implicit conversions on the RHS (e.g. to bool!), so "consume"<br>
+// the possible conversion by passing something convertible to const T& instead.<br>
+template <typename T> struct ConvertibleTo { operator T(); };<br>
+template <typename T><br>
+struct StreamSwitch<T, decltype((void)(std::declval<llvm::raw_ostream &>()<br>
+                                       << ConvertibleTo<const T &>()))> {<br>
+  static const RawStreamProxy<T> printable(const T &V) { return {V}; }<br>
+};<br>
+} // namespace llvm_gtest<br>
+#endif  // !GTEST_NO_LLVM_RAW_OSTREAM<br>
+<br>
+#endif // GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_RAW_OSTREAM_H_<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" rel="noreferrer" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</blockquote></div></blockquote></div>