[libcxx-commits] [PATCH] D63053: Add nullptr output operator overload (2221)

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: include/ostream:219
     basic_ostream& operator<<(const void* __p);
+    basic_ostream& operator<<(nullptr_t);
     basic_ostream& operator<<(basic_streambuf<char_type, traits_type>* __sb);
----------------
We externally instantiate the rest of these methods, but we can't do that here (for backwards compat reasons).

You need to add `_LIBCPP_INLINE_VISIBILITY` and maybe put it at the bottom of the group because it's unique in this way.


================
Comment at: include/ostream:714
+template<class _CharT, class _Traits>
+basic_ostream<_CharT, _Traits>&
+basic_ostream<_CharT, _Traits>::operator<<(nullptr_t)
----------------
`inline` or just define the thing inside the class.


================
Comment at: include/ostream:717
+{
+    return *this << static_cast<const void*>(0x0);
+}
----------------
`static_cast<const void*>(nullptr)`;

But I'm not sure we want to forward to the `const void*` overload. My understanding of that overload is that it exists to print "bool" values for types like `ostream` that used to implement `if (stream)` via a conversion to `void*`. 

I think we may just want to print `nill`.


================
Comment at: test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters/streambuf.pass.cpp:74
+        os << nullptr;
+        assert(sb.str().c_str()[0] == '0');
+    }
----------------
I don't think this test is portable. Implementations can print `null` or `nill` or "hubbabalo" for that matter.
We should probably just make this a `LIBCPP_ASSERT`. And maybe we should also assert the string isn't empty (for when other STL's use our test suite).


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63053/new/

https://reviews.llvm.org/D63053