[libc++] Define ostream nullptr inserter for >= C++17 only

[jloser]

Summary

The ostream nullptr inserter implemented in 3c125fe is missing a C++ version
guard. Normally, libc++ takes the stance of backporting LWG issues to older
standards modes as was done in 3c125fe. However, backporting to older standards
modes breaks existing code in popular libraries such as Boost.Test and
Google Test who define their own overload for nullptr_t.

Instead, only apply this operator<< overload in C++17 or later.

Fixes https://github.com/llvm/llvm-project/issues/55861.

[Mordante]

I see both MSVC STL and libstdc++ have taken the approach to only accept this overload in C++17 and later.
However our general policy is to retro-actively apply LWG issues to older versions of the Standard.
@ldionne what's your opinion?

[ldionne]

Normally, we do apply LWG issues retroactively, so I wouldn't change this. @jwakely @STL_MSFT Any insight as to why your respective standard libraries guarded this with C++17?

[jwakely]

In D127033#3563484, @ldionne wrote:

Normally, we do apply LWG issues retroactively, so I wouldn't change this. @jwakely @STL_MSFT Any insight as to why your respective standard libraries guarded this with C++17?

I don't remember, sorry. I'm not sure we actually ever discussed that aspect, we spent far more time discussing the content of the NTBS it prints.

Personally, I don't think this should really have been handled as a library issue. It's more of a new feature, like "Printing volatile Pointers" https://wg21.link/p1147r1. And since I'm not aware of anybody actually caring about this feature, I haven't spent much time thinking/caring about it.

If users were requesting it in C++11 and C++14 modes I'd consider enabling it, but I don't think anybody has asked for that.

[jwakely]

I've done some digging. It looks like ostream support for nullptr_t was discussed in August 2011 (thread beginning at c++std-lib-30987) and the outcome was "not a defect, wait for a new standard". That supports my feeling that it never should have been handled as a library issue.

Also, Billy O'Neal reported that the new overload caused ambiguities for some morally questionable code in Google Test, which was trying to define its own overload for this type. Breaking existing code (even if it's bad code) is another reason to not enable it unconditionally. I guess that's no longer an issue, or somebody would have noticed it when using gtest and libc++!

It seems like the only known uses for this operator<< are in Boost.Test, Google Test, and Mozilla's assertion macros ... so I super don't care about it.

[lichray]

In D127033#3563576, @jwakely wrote:

Also, Billy O'Neal reported that the new overload caused ambiguities for some morally questionable code in Google Test, which was trying to define it's own overload for this type. Breaking existing code (even if it's bad code) is another reason to not enable it unconditionally. I guess that's no longer an issue, or somebody would have noticed it when using gtest and libc++!

It seems like the only known uses for this operator<< are in Boost.Test, Google Test, and Mozilla's assertion macros ... so I super don't care about it.

I filed https://github.com/llvm/llvm-project/issues/55861 because of a breakage found in Symantec's test code (a header we own added a conflicting overload). I'm requesting the new overload *not* be added to old versions of C++, for portability's sake.

[STL_MSFT]

Yep, the Google Test impact was indeed why MSVC's STL had to guard this for C++17 mode (as our default and oldest mode is C++14).

[Mordante]

Still pending @ldionne's approval, but based on the feedback I think we should do this. But please add the requested comments. (That will avoid us trying to backport this again in the future.)

libcxx/include/ostream

228

Please add comment this is LWG 2221 and add a rationale why this isn't backported, a link to this review suffices.

libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters/streambuf.pass.cpp

70

Please add comment that this LWG-issue isn't backported.

[ldionne]

LGTM w/ comments applied.

Thanks a lot @jwakely for the additional information.

libcxx/include/ostream

228

+1

[jloser]

Add comment about LWG 2221 not being backported.
Update commit message to provide brief rationale.

If CI passes, I'll ship this.

[Mordante]

In D127033#3663262, @jloser wrote:

Add comment about LWG 2221 not being backported.
Update commit message to provide brief rationale.

Thanks, LGTM!

libcxx/include/ostream

#if _LIBCPP_STD_VER > 20
     _LIBCPP_HIDE_FROM_ABI
     basic_ostream& operator<<(const volatile void* __p) {
         return operator<<(const_cast<const void*>(__p));
     }
 #endif
 
     basic_ostream& operator<<(basic_streambuf<char_type, traits_type>* __sb);
 
+#if _LIBCPP_STD_VER > 14
+// LWG 2221 - nullptr. This is not backported to older standards modes.
+// See https://reviews.llvm.org/D127033 for more info on the rationale.
     _LIBCPP_INLINE_VISIBILITY
     basic_ostream& operator<<(nullptr_t)
     { return *this << "nullptr"; }
+#endif
 
     // 27.7.2.7 Unformatted output:
     basic_ostream& put(char_type __c);
     basic_ostream& write(const char_type* __s, streamsize __n);
     basic_ostream& flush();
 
     // 27.7.2.5 seeks:
     inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1

libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters/streambuf.pass.cpp

int main(int, char**)
     {
         testbuf<char> sb;
         std::ostream os(&sb);
         testbuf<char> sb2("testing...");
         assert(sb.str() == "");
         os << &sb2;
         assert(sb.str() == "testing...");
     }
-    { // LWG 2221 - nullptr
+#if TEST_STD_VER > 14
+// LWG 2221 - nullptr. This is not backported to older standards modes.
+// See https://reviews.llvm.org/D127033 for more info on the rationale.
+    {
         testbuf<char> sb;
         std::ostream os(&sb);
         os << nullptr;
         assert(sb.str().size() != 0);
         LIBCPP_ASSERT(sb.str() == "nullptr");
     }
+#endif
 
   return 0;
 }