[libcxx-commits] [PATCH] D113482: [libc++] Implement P1147R1 (Printing volatile T*)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added a comment.

LGTM as well.



================
Comment at: libcxx/include/ostream:58
     basic_ostream& operator<<(const void* p);
+    basic_ostream& operator<<(const volatile void* val);
     basic_ostream& operator<<(basic_streambuf<char_type,traits>* sb);
----------------
ldionne wrote:
> Nitpick: add `// C++23`.
> 
> I'll do this when committing on your behalf.
`// since C++23` :)


================
Comment at: libcxx/include/ostream:216-218
+  _LIBCPP_HIDE_FROM_ABI basic_ostream& operator<<(const volatile void* __val) {
+    return operator<<(const_cast<const void*>(__val));
+  }
----------------
These lines are misindented; should be 4 spaces.
Also consider `s/__val/__p/g` for consistency with line 213, but whatever.
Also consider linebreaking after `_LIBCPP_HIDE_FROM_ABI`.


================
Comment at: libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters.arithmetic/pointer.volatile.pass.cpp:64
+  assert(os3.good());
+  std::string s3(sb3.str());
+
----------------
Style nit: Lines 51, 57, 64: IIUC, `sb.str()` returns something of type `std::string` already, so the cast-like direct-initialization syntax is unnecessary. `std::string s3 = sb3.str();` would be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113482