Google Test fails to compile if a standard library implements LWG 2221

[BillyONeal]

Hi folks!

I implemented LWG 2221 in MSVC++'s STL last week and now all projects that use Google Test to test things fail to compile, due to an ambiguity, see the operator<< here: https://github.com/google/googletest/blob/master/googletest/include/gtest/gtest-printers.h#L230

D:\PDFium\pdfium\third_party\googletest\src\googletest\include\gtest/gtest-printers.h(285): error C2593: 'operator <<' is ambiguous

C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.12.25827\include\ostream(501): note: could be 'std::basic_ostream<char,std::char_traits<char>> &std::basic_ostream<char,std::char_traits<char>>::operator <<<void>(std::nullptr_t)'

D:\PDFium\pdfium\third_party\googletest\src\googletest\include\gtest/gtest-printers.h(230): note: or       'std::basic_ostream<char,std::char_traits<char>> &testing::internal2::operator <<<char,std::char_traits<char>,T>(std::basic_ostream<char,std::char_traits<char>> &,const T &)'

Looking at what Google Test is doing, it doesn't seem like this attempt at adding overloads to std::'s operator<< is portable, as it's making assumptions about how the normal operator<<s are declared, and those are members. Standard libraries have broad latitude to change member function signatures, as long as an ordinary call would have the same behavior as the original signature; see http://eel.is/c++draft/member.functions#2 -- which makes attempting to add to an overload set the standard library declares perilous. Previously, it looks like this was working because the other operator<<s in our standard library implementation are not templates, so the last overloading ambiguity resolution rule (prefer the non-template) was preferring the standard ones; but there is no requirement that the standard library not implement those member functions as templates, and our nullptr_t bit needs to be a template to avoid getting exported.

Unfortunately as far as I can tell there isn't a portable way to do what Google Test wants to do in C++98/03. What you really want to do is something like:

#include <utility>
#include <type_traits>

template<class...>
struct gtest_void_ish {
    using type = void;
};

template<class T, class = void>
struct can_be_streamed_out : ::std::false_type {};

template<class T>
struct can_be_streamed_out<T, typename gtest_void_ish<decltype(
    ::std::declval<::std::ostream&>() << ::std::declval<const T&>()
    )>::type> : ::std::true_type {};


template<typename T>
::std::ostream& gtest_print_impl(::std::ostream& os, const T& x, ::std::true_type) {
    return os << x;
}

template<typename T>
::std::ostream& gtest_print_impl(::std::ostream& os, const T& x, ::std::false_type) {
  TypeWithoutFormatter<T, (internal::IsAProtocolMessage<T>::value
                               ? kProtobuf
                               : internal::ImplicitlyConvertible<
                                     const T&, internal::BiggestInt>::value
                                     ? kConvertibleToInteger
                                     :
#if GTEST_HAS_ABSL
                                     internal::ImplicitlyConvertible<
                                         const T&, absl::string_view>::value
                                         ? kConvertibleToStringView
                                         :
#endif
                                         kOtherType)>::PrintValue(x, &os);
  return os;
}

template <typename T>
::std::ostream& gtest_print(::std::ostream& os, const T& x) {
  return gtest_print_impl(os, x, can_be_streamed_out<T>());
}

Would Google Test consider using a strategy like this, at least on C++11 supporting compilers?

[gennadiycivil]

How is this? #1616

[gennadiycivil]

#1620

[Stannieman]

@BillyONeal do you know who's maintaining this package: https://www.nuget.org/packages/Microsoft.googletest.v140.windesktop.msvcstl.static.rt-dyn/?
It's installed by default when creating a Google Test project in Visual Studio, but it's stuck on GTest 1.8.0 and thus does not include the latest fixes including this one.

There's an alternative package that's more up to date but it would be nice if the official MS package also could follow the GTest release more closely.

[BillyONeal]

@Stannieman I can poke some folks. What release do they need to update to to get this fixed?

Thanks!

[Stannieman]

@BillyONeal not sure actually, 1.8.1 probably if that includes the fix for this issue but I'm not sure it does.

[Stannieman]

@BillyONeal I had time to check this package issue again and can confirm that GTest 1.8.1 resolves the issue.

What's a bit frustrating though is that this package I linked to seems to be some quick and dirty package to get GTest support in VS working.
If you look at it:
First VS gets GTest support and that GTest packages is made to support that.
Then LWG 2221 gets implemented and it becomes apparent that GTest does non-standard C++ things.
Then GTest is fixed and gets a new 1.8.1 release with that fix.
Then the package that's automatically referenced by the official GTest project template never gets updated with this fix.
Result: The built in official GTest support of VS does not work with newer MSVC versions.

So what are we supposed to do? Will this package be maintained in the future and get updates when GTest gets new releases? Should we make our own GTest builds + package and use that instead of the default one (this is easy)?
And if the package will be maintained by MS, then will there be a guarantee that it gets proper updates?

[BillyONeal]

I just pinged folks that own this again.

[Stannieman]

Thank you sir!

[tsarevichdm]

@Stannieman We just copied essential part of #1620 till MS package maintainers update gtest to 1.8.1+, like this:

#include "gtest/gtest.h"

// TODO(me): Workaround for https://github.com/google/googletest/issues/1616 till
// MS GTest package maintainers update to 1.8.1+.
namespace testing::internal {
#if GTEST_LANG_CXX11
inline void PrintTo(std::nullptr_t, ::std::ostream* os) { *os << "(nullptr)"; }
#endif  // GTEST_LANG_CXX11
}  // namespace testing::internal

Quite ugly, but still works.

[Stannieman]

@tsarevichdm we went the "build own package" route.