<sstream>: Don't swap basic_stringbufs in move assignment and allocator-extended construction

stl/inc/sstream

@@ -18,6 +18,12 @@ _STL_DISABLE_CLANG_WARNINGS
 #undef new
 _STD_BEGIN
 
+#if !_HAS_CXX20
+struct _Stringbuf_from_allocator_tag {
+    explicit _Stringbuf_from_allocator_tag() = default;
+};
+#endif // !_HAS_CXX20
+
 _EXPORT_STD template <class _Elem, class _Traits, class _Alloc>
 class basic_stringbuf
     : public basic_streambuf<_Elem, _Traits> { // stream buffer maintaining an allocated character array
@@ -67,23 +73,94 @@ public:
         : basic_stringbuf(_Str, _Mode, _Alloc{}) {}
 
     basic_stringbuf(basic_stringbuf&& _Right, const _Alloc& _Al_) : _Mystate(0), _Al(_Al_) {
-        _Assign_rv(_STD move(_Right));
+        if constexpr (!allocator_traits<_Alloc>::is_always_equal::value) {
+            if (_Al != _Right._Al) {
+                _Copy_into_self_and_tidy(_STD move(_Right));
+                return;
+            }
+        }
+
+        _Take_contents(_STD move(_Right));
     }
-#endif // _HAS_CXX20
+#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
+    basic_stringbuf(_Stringbuf_from_allocator_tag _Tag, const _Alloc& _Al_)
+        : basic_stringbuf(_Tag, ios_base::in | ios_base::out, _Al_) {}
 
-    basic_stringbuf(basic_stringbuf&& _Right) : _Mystate(0) {
-        _Assign_rv(_STD move(_Right));
+    basic_stringbuf(_Stringbuf_from_allocator_tag, ios_base::openmode _Mode, const _Alloc& _Al_)
+        : _Seekhigh(nullptr), _Mystate(_Getstate(_Mode)), _Al(_Al_) {}
+#endif // ^^^ !_HAS_CXX20 ^^^
+
+    basic_stringbuf(basic_stringbuf&& _Right)
+        : _Seekhigh(_STD exchange(_Right._Seekhigh, nullptr)), _Mystate(_STD exchange(_Right._Mystate, 0)),
+          _Al(_Right._Al) {
+        _Mysb::swap(_Right);
     }
 
-    basic_stringbuf& operator=(basic_stringbuf&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
+    basic_stringbuf& operator=(basic_stringbuf&& _Right) noexcept(
+        _Choose_pocma_v<_Alloc> != _Pocma_values::_No_propagate_allocators) /* strengthened */ {
+        if (this != _STD addressof(_Right)) {
+            _Assign_rv_no_alias(_STD move(_Right));
+        }
         return *this;
     }
 
-    void _Assign_rv(basic_stringbuf&& _Right) noexcept {
-        if (this != _STD addressof(_Right)) {
-            _Tidy();
-            this->swap(_Right);
+    void _Take_contents(basic_stringbuf&& _Right) noexcept {
+        // pre: *this holds no dynamic buffer and _Al == _Right._Al
+        _Mystate  = _STD exchange(_Right._Mystate, 0);
+        _Seekhigh = _STD exchange(_Right._Seekhigh, nullptr);
+        _Mysb::swap(_Right);
+    }
+
+    void _Copy_into_self_and_tidy(basic_stringbuf&& _Right) {
+        // pre: *this holds no dynamic buffer and _Al != _Right._Al
+        if ((_Right._Mystate & _Allocated) == 0) {
+            return;
+        }
+
+        const auto _Right_data     = _Right._Mysb::eback();
+        const auto _Right_capacity = static_cast<typename allocator_traits<_Alloc>::size_type>(
+            (_Right._Mysb::pptr() ? _Right._Mysb::epptr() : _Right._Mysb::egptr()) - _Right_data);
+
+        auto _New_capacity   = _Right_capacity;
+        const auto _New_data = _STD _Unfancy(_STD _Allocate_at_least_helper(_Al, _New_capacity));
+
+        _Mysb::setg(_New_data, _New_data + (_Right._Mysb::gptr() - _Right_data),
+            _New_data + (_Right._Mysb::egptr() - _Right_data));
+        if (_Right._Mysb::pbase() != nullptr) {
+            const auto _Pbase_diff = _Right._Mysb::pbase() - _Right_data;
+            const auto _Pptr_diff  = _Right._Mysb::pptr() - _Right_data;
+            const auto _Epptr_diff = _Right._Mysb::epptr() - _Right_data;
+            _Mysb::setp(_New_data + _Pbase_diff, _New_data + _Pptr_diff, _New_data + _Epptr_diff);
+        } else {
+            _Mysb::setp(nullptr, nullptr, nullptr);
+        }
+
+        const auto _Right_view = _Right._Get_buffer_view();
+        if (_Right_view._Ptr != nullptr) {
+            _Traits::copy(_New_data + (_Right_view._Ptr - _Right_data), _Right_view._Ptr, _Right_view._Size);
+        }
+
+        _Seekhigh = _New_data + _New_capacity;
+        _Mystate  = _Right._Mystate;
+
+        _Right._Tidy();
+    }
+
+    void _Assign_rv_no_alias(basic_stringbuf&& _Right) noexcept(
+        _Choose_pocma_v<_Alloc> != _Pocma_values::_No_propagate_allocators) {
+        // pre: this != std::addressof(_Right)
+        _Tidy();
+        if constexpr (_Choose_pocma_v<_Alloc> == _Pocma_values::_No_propagate_allocators) {
+            if (_Al == _Right._Al) {
+                _Take_contents(_STD move(_Right));
+            } else {
+                _Copy_into_self_and_tidy(_STD move(_Right));
+            }
+        } else {
+            if constexpr (_Choose_pocma_v<_Alloc> == _Pocma_values::_Propagate_allocators) {
+                _Al = _STD move(_Al);
+            }
+            _Take_contents(_STD move(_Right));
         }
     }
 
@@ -222,7 +299,11 @@ public:
     _NODISCARD allocator_type get_allocator() const noexcept {
         return _Al;
     }
-#endif // _HAS_CXX20
+#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
+    _NODISCARD allocator_type _Get_allocator() const noexcept {
+        return _Al;
+    }
+#endif // ^^^ !_HAS_CXX20 ^^^
 
 protected:
     int_type overflow(int_type _Meta = _Traits::eof()) override { // put an element to stream
@@ -584,20 +665,25 @@ public:
         : _Mybase(_STD addressof(_Stringbuffer)), _Stringbuffer(_Str, _Mode | ios_base::in) {}
 #endif // _HAS_CXX20
 
-    basic_istringstream(basic_istringstream&& _Right) : _Mybase(_STD addressof(_Stringbuffer)) {
-        _Assign_rv(_STD move(_Right));
-    }
-
-    basic_istringstream& operator=(basic_istringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
+    basic_istringstream(basic_istringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+#if _HAS_CXX20
+          _Stringbuffer(_Right._Stringbuffer.get_allocator())
+#else // ^^^ _HAS_CXX20 / !_HAX_CXX20 vvv
+          _Stringbuffer(_Stringbuf_from_allocator_tag{}, _Right._Stringbuffer._Get_allocator())
+#endif // ^^^ !_HAS_CXX20 ^^^
+    {
+        _Mybase::swap(_Right);
+        _Stringbuffer._Take_contents(_STD move(_Right._Stringbuffer));
     }
 
-    void _Assign_rv(basic_istringstream&& _Right) noexcept {
+    basic_istringstream& operator=(basic_istringstream&& _Right) noexcept(
+        _Choose_pocma_v<_Alloc> != _Pocma_values::_No_propagate_allocators) /* strengthened */ {
         if (this != _STD addressof(_Right)) {
             _Mybase::swap(_Right);
-            _Stringbuffer._Assign_rv(_STD move(_Right._Stringbuffer));
+            _Stringbuffer._Assign_rv_no_alias(_STD move(_Right._Stringbuffer));
         }
+        return *this;
     }
 
     void swap(basic_istringstream& _Right) noexcept /* strengthened */ {
@@ -704,20 +790,25 @@ public:
         : _Mybase(_STD addressof(_Stringbuffer)), _Stringbuffer(_Str, _Mode | ios_base::out) {}
 #endif // _HAS_CXX20
 
-    basic_ostringstream(basic_ostringstream&& _Right) : _Mybase(_STD addressof(_Stringbuffer)) {
-        _Assign_rv(_STD move(_Right));
-    }
-
-    basic_ostringstream& operator=(basic_ostringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
+    basic_ostringstream(basic_ostringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+#if _HAS_CXX20
+          _Stringbuffer(_Right._Stringbuffer.get_allocator())
+#else // ^^^ _HAS_CXX20 / !_HAX_CXX20 vvv
+          _Stringbuffer(_Stringbuf_from_allocator_tag{}, _Right._Stringbuffer._Get_allocator())
+#endif // ^^^ !_HAS_CXX20 ^^^
+    {
+        _Mybase::swap(_Right);
+        _Stringbuffer._Take_contents(_STD move(_Right._Stringbuffer));
     }
 
-    void _Assign_rv(basic_ostringstream&& _Right) noexcept {
+    basic_ostringstream& operator=(basic_ostringstream&& _Right) noexcept(
+        _Choose_pocma_v<_Alloc> != _Pocma_values::_No_propagate_allocators) /* strengthened */ {
         if (this != _STD addressof(_Right)) {
             _Mybase::swap(_Right);
-            _Stringbuffer._Assign_rv(_STD move(_Right._Stringbuffer));
+            _Stringbuffer._Assign_rv_no_alias(_STD move(_Right._Stringbuffer));
         }
+        return *this;
     }
 
     void swap(basic_ostringstream& _Right) noexcept /* strengthened */ {
@@ -830,20 +921,25 @@ public:
         : _Mybase(_STD addressof(_Stringbuffer)), _Stringbuffer(_Str, _Mode) {}
 #endif // _HAS_CXX20
 
-    basic_stringstream(basic_stringstream&& _Right) : _Mybase(_STD addressof(_Stringbuffer)) {
-        _Assign_rv(_STD move(_Right));
-    }
-
-    basic_stringstream& operator=(basic_stringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
+    basic_stringstream(basic_stringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+#if _HAS_CXX20
+          _Stringbuffer(_Right._Stringbuffer.get_allocator())
+#else // ^^^ _HAS_CXX20 / !_HAX_CXX20 vvv
+          _Stringbuffer(_Stringbuf_from_allocator_tag{}, _Right._Stringbuffer._Get_allocator())
+#endif // ^^^ !_HAS_CXX20 ^^^
+    {
+        _Mybase::swap(_Right);
+        _Stringbuffer._Take_contents(_STD move(_Right._Stringbuffer));
     }
 
-    void _Assign_rv(basic_stringstream&& _Right) noexcept {
+    basic_stringstream& operator=(basic_stringstream&& _Right) noexcept(
+        _Choose_pocma_v<_Alloc> != _Pocma_values::_No_propagate_allocators) /* strengthened */ {
         if (this != _STD addressof(_Right)) {
             _Mybase::swap(_Right);
-            _Stringbuffer._Assign_rv(_STD move(_Right._Stringbuffer));
+            _Stringbuffer._Assign_rv_no_alias(_STD move(_Right._Stringbuffer));
         }
+        return *this;
     }
 
     void swap(basic_stringstream& _Right) noexcept /* strengthened */ {

tests/std/tests/GH_001858_iostream_exception/test.cpp

@@ -8,6 +8,7 @@
 #include <ios>
 #include <iostream>
 #include <istream>
+#include <memory_resource>
 #include <ostream>
 #include <sstream>
 #include <streambuf>
@@ -322,6 +323,27 @@ STATIC_ASSERT(is_nothrow_move_assignable_v<wostringstream>);
 STATIC_ASSERT(is_nothrow_move_assignable_v<stringstream>);
 STATIC_ASSERT(is_nothrow_move_assignable_v<wstringstream>);
 
+// GH-4232: <sstream>: basic_stringbuf shouldn't implement moving with swapping
+// These operations need to be able to throw exceptions when the source and target allocators are unequal.
+#if _HAS_CXX17
+STATIC_ASSERT(
+    !is_nothrow_move_assignable_v<basic_stringbuf<char, char_traits<char>, pmr::polymorphic_allocator<char>>>);
+STATIC_ASSERT(
+    !is_nothrow_move_assignable_v<basic_stringbuf<wchar_t, char_traits<wchar_t>, pmr::polymorphic_allocator<wchar_t>>>);
+STATIC_ASSERT(
+    !is_nothrow_move_assignable_v<basic_istringstream<char, char_traits<char>, pmr::polymorphic_allocator<char>>>);
+STATIC_ASSERT(!is_nothrow_move_assignable_v<
+              basic_istringstream<wchar_t, char_traits<wchar_t>, pmr::polymorphic_allocator<wchar_t>>>);
+STATIC_ASSERT(
+    !is_nothrow_move_assignable_v<basic_ostringstream<char, char_traits<char>, pmr::polymorphic_allocator<char>>>);
+STATIC_ASSERT(!is_nothrow_move_assignable_v<
+              basic_ostringstream<wchar_t, char_traits<wchar_t>, pmr::polymorphic_allocator<wchar_t>>>);
+STATIC_ASSERT(
+    !is_nothrow_move_assignable_v<basic_stringstream<char, char_traits<char>, pmr::polymorphic_allocator<char>>>);
+STATIC_ASSERT(!is_nothrow_move_assignable_v<
+              basic_stringstream<wchar_t, char_traits<wchar_t>, pmr::polymorphic_allocator<wchar_t>>>);
+#endif // _HAS_CXX17
+
 STATIC_ASSERT(is_nothrow_std_swappable<stringbuf>);
 STATIC_ASSERT(is_nothrow_std_swappable<wstringbuf>);
 STATIC_ASSERT(is_nothrow_std_swappable<istringstream>);

tests/std/tests/P0408R7_efficient_access_to_stringbuf_buffer/test.cpp

@@ -9,6 +9,7 @@
 #include <sstream>
 #include <string>
 #include <string_view>
+#include <type_traits>
 #include <utility>
 
 #include <test_death.hpp>
@@ -116,6 +117,22 @@ struct test_pmr_allocator {
         }
         Stream stream2{init_value, init_value.get_allocator()};
         assert(stream2.rdbuf()->get_allocator() == init_value.get_allocator());
+
+        // GH-4232: <sstream>: basic_stringbuf shouldn't implement moving with swapping
+
+        // Move to another stream buffer with different allocators
+        using SBuf = remove_pointer_t<decltype(stream.rdbuf())>;
+        SBuf& sbuf = *stream.rdbuf();
+        SBuf sbuf2{move(sbuf), init_value.get_allocator()};
+        assert(sbuf2.get_allocator() != sbuf.get_allocator());
+        assert(stream.view().empty());
+        assert(sbuf2.view() == init_value);
+
+        // Move assignment between different memory resources
+        sbuf = move(sbuf2);
+        assert(sbuf2.get_allocator() != sbuf.get_allocator());
+        assert(sbuf2.view().empty());
+        assert(stream.view() == init_value);
     }
 };