[libcxx-commits] [PATCH] D148641: [libc++] Add C++20 stringstream::view()
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Apr 27 10:43:16 PDT 2023
Mordante added a comment.
Thanks for working on this! I mainly looked at the high-level parts not compared against the paper yet.
Does this patch implement the entire paper or not? If not please make sure you add a note on the status page to explain which parts are and are not done. (We might remove the synopsis in the future.)
================
Comment at: libcxx/include/sstream:37
// [stringbuf.assign] Assign and swap:
basic_stringbuf& operator=(basic_stringbuf&& rhs);
----------------
Please update `libcxx/docs/Status/Cxx20.rst` with the implementation status of P0408R7. It would be great to put that number+paper name in the commit message too. That makes it easier to find the associated paper.
================
Comment at: libcxx/include/sstream:449-457
- if (__mode_ & ios_base::out)
- {
- if (__hm_ < this->pptr())
- __hm_ = this->pptr();
- return string_type(this->pbase(), __hm_, __str_.get_allocator());
- }
- else if (__mode_ & ios_base::in)
----------------
The feature should be guarded so it is only available in C++20 and later.
The same applies to where you add the `view` member.
================
Comment at: libcxx/include/sstream:492
+basic_string_view<_CharT, _Traits>
+basic_stringbuf<_CharT, _Traits, _Allocator>::view() const noexcept
+{
----------------
Please format the new code with clang-format so it matches our "normal" style.
================
Comment at: libcxx/test/std/input.output/string.streams/istringstream/istringstream.members/str.pass.cpp:14
// void str(const basic_string<charT,traits,Allocator>& s);
----------------
Please create a new test for the view (can be a copy past of this file) and update the synopsis here.
That test will need an `// UNSUPPORTED: c++03, c++11, c++14, c++17` line since it is C++20 only.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148641/new/
https://reviews.llvm.org/D148641
More information about the libcxx-commits
mailing list