This is an archive of the discontinued LLVM Phabricator instance.

Add nullptr output operator overload (2221)
ClosedPublic

Authored by zoecarver on Jun 8 2019, 4:33 PM.

Details

Summary

This patch fixes LWG issue 2221 by adding a nullptr overload to basic_ostream.

Diff Detail

Event Timeline

zoecarver created this revision.Jun 8 2019, 4:33 PM
EricWF added inline comments.Jun 8 2019, 11:43 PM
include/ostream
219

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.

717

inline or just define the thing inside the class.

720

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.

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

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).

mclow.lists added inline comments.Jun 10 2019, 6:53 AM
include/ostream
720

I suggested this approach to Zoe, because I thought that that was what GCC did. (print 0x0)
Turns out that I was wrong; GCC prints nullptr (at least that's what wandbox claims about gcc 9).

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

Agreed.

zoecarver marked 4 inline comments as done.Jun 10 2019, 8:06 AM
zoecarver added inline comments.
include/ostream
219

Thanks, will do.

717

I'll do the later.

720

Alright. I like printing nullptr better anyway, so I will do that if it's OK with both of you.

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

Will do.

  • fix inlining
  • fix visibility
  • fix "nullptr" output value
  • update tests
mclow.lists accepted this revision.Jun 10 2019, 2:08 PM

Once you fix the nits, I think this is good to go.
Be sure to update www/cxx1z_status.html as well.

include/ostream
223

Don't need the \0; the compiler does that for you.
Nor the static cast; the type of a string literal is const char * already.
Just say:

{ return *this << "nullptr"; }
This revision is now accepted and ready to land.Jun 10 2019, 2:08 PM
zoecarver updated this revision to Diff 203963.Jun 10 2019, 7:58 PM
zoecarver marked an inline comment as done.
  • update status
  • fix operator implementation
include/ostream
223

🤦‍♂️but I liked the static_cast so much \s.

ldionne accepted this revision.Jun 11 2019, 8:27 AM
mclow.lists closed this revision.Jul 1 2019, 9:20 AM

Committed as revision 364802 (with a minor change; I updated the synopsis)