[libcxx-commits] [PATCH] D151896: [libc++] Implement C++20 P0408R7 (Efficient Access to basic_stringbuf's Buffer)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this. I mainly glossed over the patch.



================
Comment at: libcxx/docs/Status/Cxx20Papers.csv:102
 "`P0325R4 <https://wg21.link/P0325R4>`__","LWG","to_array from LFTS with updates","Cologne","|Complete|","10.0"
-"`P0408R7 <https://wg21.link/P0408R7>`__","LWG","Efficient Access to basic_stringbuf's Buffer","Cologne","|In Progress| [#note-P0408]_",""
+"`P0408R7 <https://wg21.link/P0408R7>`__","LWG","Efficient Access to basic_stringbuf's Buffer","Cologne","|Complete|","17.0"
 "`P0466R5 <https://wg21.link/P0466R5>`__","LWG","Layout-compatibility and Pointer-interconvertibility Traits","Cologne","",""
----------------
Please mention this in `ReleaseNotes.rst` too


================
Comment at: libcxx/include/sstream:36
+    explicit basic_stringbuf(const allocator_type& a)
+        : basic_stringbuf(ios_base::in | ios_base::out, a) {} // C++20
+    basic_stringbuf(ios_base::openmode which, const allocator_type& a); // C++20
----------------
Pleas align `// C++20` in the same column


================
Comment at: libcxx/include/sstream:285
+#  define _LIBCPP_REF_SINCE_CXX20
+#endif
 
----------------
I really dislike this macro. It's quite unclear what it does. Signatures like `string str() &&` are valid. Looking at the number of use cases I rather ifdef at these places.


================
Comment at: libcxx/include/sstream:342
+                                                   ios_base::openmode __wch = ios_base::in | ios_base::out)
+        : __str_(_VSTD::move(__s)), __hm_(nullptr), __mode_(__wch) {
+        __init_buf_ptrs();
----------------
We use `std` in new code. We don't change existing code to avoid merge conflicts with open patches.


================
Comment at: libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.members/str.alloc.pass.cpp:17
+// template <class SAlloc>
+// basic_string<charT, traits, SAlloc> str(const SAlloc& sa) const;
+// template <class SAlloc>
----------------
I miss tests for const member functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151896