[libc++] Add noexcept clauses to swap per P0408R7 (Efficient Access to basic_stringbuf's Buffer)

[Mordante]

Thanks this smaller patch is a lot easier to quickly review!
A few minor issues, otherwise looks fine!

libcxx/include/sstream

39–40

This is how we typically do these changes.

61

Please verify the layout looks nice after applying this comment.

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp

33

I like to see a few additional tests

• An allocator without propagate_on_container_swap
• An allocator without is_always_equal with different values for is_empty

http://eel.is/c++draft/allocator.traits.types#9
http://eel.is/c++draft/allocator.traits.types#10
http://eel.is/c++draft/meta.unary.prop#lib:is_empty,class

37

Please use descriptive names here. That makes reading the test a lot easier.

[pfusik]

Reword comments.
More tests, descriptive classes.

[Mordante]

In general happy one comment.

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp

92

Can you add some test where noexcept(buf.swap(buf1)) == false.

[pfusik]

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp

92

See five lines up.

[Mordante]

LGTM!

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp

92

Ah sorry I overlooked that.

[pfusik]

Please land this change as I have no commit access. Thank you!

[Mordante]

In D153427#4446551, @pfusik wrote:

Please land this change as I have no commit access. Thank you!

I've landed it on your behalf, thanks for the patch.

libcxx/include/sstream

public:
     basic_stringbuf() : basic_stringbuf(ios_base::in | ios_base::out) {}               // C++20
     explicit basic_stringbuf(ios_base::openmode which);                                // C++20
     explicit basic_stringbuf(const basic_string<char_type, traits_type, allocator_type>& str,
                              ios_base::openmode which = ios_base::in | ios_base::out);
     basic_stringbuf(basic_stringbuf&& rhs);
 
     // [stringbuf.assign] Assign and swap:
     basic_stringbuf& operator=(basic_stringbuf&& rhs);
-    void swap(basic_stringbuf& rhs);
+    void swap(basic_stringbuf& rhs) noexcept(see below);                               // conditionally noexcept since C++20
 
     // [stringbuf.members] Member functions:
     basic_string<char_type, traits_type, allocator_type> str() const;
     void str(const basic_string<char_type, traits_type, allocator_type>& s);
     basic_string_view<char_type, traits_type> view() const noexcept; // C++20
 
 protected:
     // [stringbuf.virtuals] Overridden virtual functions:
     virtual int_type underflow();
     virtual int_type pbackfail(int_type c = traits_type::eof());
     virtual int_type overflow (int_type c = traits_type::eof());
     virtual basic_streambuf<char_type, traits_type>* setbuf(char_type*, streamsize);
     virtual pos_type seekoff(off_type off, ios_base::seekdir way,
                              ios_base::openmode which = ios_base::in | ios_base::out);
     virtual pos_type seekpos(pos_type sp,
                              ios_base::openmode which = ios_base::in | ios_base::out);
 };
 
 // [stringbuf.assign] non member swap
 template <class charT, class traits, class Allocator>
-  void swap(basic_stringbuf<charT, traits, Allocator>& x,
-            basic_stringbuf<charT, traits, Allocator>& y);
+          basic_stringbuf<charT, traits, Allocator>& y); // conditionally noexcept since C++20
 
 typedef basic_stringbuf<char>    stringbuf;
 typedef basic_stringbuf<wchar_t> wstringbuf;
 
 // Class template basic_istringstream [istringstream]
 template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
 class basic_istringstream
     : public basic_istream<charT, traits>
public:
     // [istringstream.members] Member functions:
     basic_stringbuf<char_type, traits_type, allocator_type>* rdbuf() const;
     basic_string<char_type, traits_type, allocator_type> str() const;
     void str(const basic_string<char_type, traits_type, allocator_type>& s);
     basic_string_view<char_type, traits_type> view() const noexcept; // C++20
 };
 
 template <class charT, class traits, class Allocator>
-  void swap(basic_istringstream<charT, traits, Allocator>& x,
-            basic_istringstream<charT, traits, Allocator>& y);
 
 typedef basic_istringstream<char>    istringstream;
 typedef basic_istringstream<wchar_t> wistringstream;
 
 // Class template basic_ostringstream [ostringstream]
 template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
 class basic_ostringstream
     : public basic_ostream<charT, traits>
public:
     // [ostringstream.members] Member functions:
     basic_stringbuf<char_type, traits_type, allocator_type>* rdbuf() const;
     basic_string<char_type, traits_type, allocator_type> str() const;
     void str(const basic_string<char_type, traits_type, allocator_type>& s);
     basic_string_view<char_type, traits_type> view() const noexcept; // C++20
 };
 
 template <class charT, class traits, class Allocator>
-  void swap(basic_ostringstream<charT, traits, Allocator>& x,
-            basic_ostringstream<charT, traits, Allocator>& y);
 
 typedef basic_ostringstream<char>    ostringstream;
 typedef basic_ostringstream<wchar_t> wostringstream;
 
 // Class template basic_stringstream [stringstream]
 template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
 class basic_stringstream
     : public basic_iostream<charT, traits>
public:
     // [stringstream.members] Member functions:
     basic_stringbuf<char_type, traits_type, allocator_type>* rdbuf() const;
     basic_string<char_type, traits_type, allocator_type> str() const;
     void str(const basic_string<char_type, traits_type, allocator_type>& str);
     basic_string_view<char_type, traits_type> view() const noexcept; // C++20
 };
 
 template <class charT, class traits, class Allocator>
-  void swap(basic_stringstream<charT, traits, Allocator>& x,
-            basic_stringstream<charT, traits, Allocator>& y);
 
 typedef basic_stringstream<char>    stringstream;
 typedef basic_stringstream<wchar_t> wstringstream;
 
 }  // std
 
 */
 
public:
     {
         str(__s);
     }
 
     basic_stringbuf(basic_stringbuf&& __rhs);
 
     // [stringbuf.assign] Assign and swap:
     basic_stringbuf& operator=(basic_stringbuf&& __rhs);
-    void swap(basic_stringbuf& __rhs);
+    void swap(basic_stringbuf& __rhs)
+#if _LIBCPP_STD_VER >= 20
+        noexcept(allocator_traits<allocator_type>::propagate_on_container_swap::value ||
+                 allocator_traits<allocator_type>::is_always_equal::value)
+#endif
+        ;
 
     // [stringbuf.members] Member functions:
     string_type str() const;
     void str(const string_type& __s);
 #if _LIBCPP_STD_VER >= 20
     _LIBCPP_HIDE_FROM_ABI
     basic_string_view<char_type, traits_type> view() const noexcept;
 #endif
basic_stringbuf<_CharT, _Traits, _Allocator>::operator=(basic_stringbuf&& __rhs)
     __rhs.__hm_ = __p;
     this->pubimbue(__rhs.getloc());
     return *this;
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void
 basic_stringbuf<_CharT, _Traits, _Allocator>::swap(basic_stringbuf& __rhs)
+#if _LIBCPP_STD_VER >= 20
+    noexcept(allocator_traits<_Allocator>::propagate_on_container_swap::value ||
+             allocator_traits<_Allocator>::is_always_equal::value)
+#endif
 {
     char_type* __p = const_cast<char_type*>(__rhs.__str_.data());
     ptrdiff_t __rbinp = -1;
     ptrdiff_t __rninp = -1;
     ptrdiff_t __reinp = -1;
     if (__rhs.eback() != nullptr)
     {
         __rbinp = __rhs.eback() - __p;
#endif
     this->pubimbue(__tl);
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 swap(basic_stringbuf<_CharT, _Traits, _Allocator>& __x,
      basic_stringbuf<_CharT, _Traits, _Allocator>& __y)
+#if _LIBCPP_STD_VER >= 20
+    noexcept(noexcept(__x.swap(__y)))
+#endif
 {
     __x.swap(__y);
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>
 basic_stringbuf<_CharT, _Traits, _Allocator>::str() const
 {

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringbuf
+
+// void swap(basic_stringbuf& rhs)
+//   noexcept(allocator_traits<allocator_type>::propagate_on_container_swap::value ||
+//            allocator_traits<allocator_type>::is_always_equal::value);
+
+#include <sstream>
+#include <cassert>
+
+#include "test_macros.h"
+
+template <class T>
+struct test_alloc {
+  using value_type = T;
+
+  [[nodiscard]] constexpr T* allocate(std::size_t) { return nullptr; }
+  void deallocate(void*, unsigned) {}
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+};
+
+template <class T>
+struct test_alloc_is_always_equal : test_alloc<T> {
+  using is_always_equal = std::true_type;
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap_is_always_equal : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+  using is_always_equal             = std::true_type;
+};
+
+template <class T>
+struct test_alloc_not_empty : test_alloc<T> {
+  bool dummy;
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap_not_empty : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+  bool dummy;
+};
+
+template <class CharT>
+static void test() {
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc<CharT>> buf;
+    static_assert(noexcept(buf.swap(buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap<CharT>> buf;
+    static_assert(noexcept(buf.swap(buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_is_always_equal<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_is_always_equal<CharT>> buf;
+    static_assert(noexcept(buf.swap(buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_is_always_equal<CharT>>
+        buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_is_always_equal<CharT>>
+        buf;
+    static_assert(noexcept(buf.swap(buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf;
+    static_assert(!noexcept(buf.swap(buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf;
+    static_assert(noexcept(buf.swap(buf1)));
+  }
+}
+
+int main(int, char**) {
+  test<char>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test<wchar_t>();
+#endif
+  return 0;
+}

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/nonmember_swap_noexcept.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <sstream>
+
+// template <class charT, class traits, class Allocator>
+// void swap(basic_stringbuf<charT, traits, Allocator>& x,
+//           basic_stringbuf<charT, traits, Allocator>& y)
+//   noexcept(allocator_traits<allocator_type>::propagate_on_container_swap::value ||
+//            allocator_traits<allocator_type>::is_always_equal::value);
+
+#include <sstream>
+#include <cassert>
+
+#include "test_macros.h"
+
+template <class T>
+struct test_alloc {
+  using value_type = T;
+
+  [[nodiscard]] constexpr T* allocate(std::size_t) { return nullptr; }
+  void deallocate(void*, unsigned) {}
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+};
+
+template <class T>
+struct test_alloc_is_always_equal : test_alloc<T> {
+  using is_always_equal = std::true_type;
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap_is_always_equal : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+  using is_always_equal             = std::true_type;
+};
+
+template <class T>
+struct test_alloc_not_empty : test_alloc<T> {
+  bool dummy;
+};
+
+template <class T>
+struct test_alloc_propagate_on_container_swap_not_empty : test_alloc<T> {
+  using propagate_on_container_swap = std::true_type;
+  bool dummy;
+};
+
+template <class CharT>
+static void test() {
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc<CharT>> buf;
+    static_assert(noexcept(swap(buf, buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap<CharT>> buf;
+    static_assert(noexcept(swap(buf, buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_is_always_equal<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_is_always_equal<CharT>> buf;
+    static_assert(noexcept(swap(buf, buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_is_always_equal<CharT>>
+        buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_is_always_equal<CharT>>
+        buf;
+    static_assert(noexcept(swap(buf, buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf;
+    static_assert(!noexcept(swap(buf, buf1)));
+  }
+  {
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf1;
+    std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf;
+    static_assert(noexcept(swap(buf, buf1)));
+  }
+}
+
+int main(int, char**) {
+  test<char>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test<wchar_t>();
+#endif
+  return 0;
+}