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

Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>

Author: frederick-vs-ja

stl/inc/sstream

@@ -66,24 +66,87 @@ public:
         const basic_string<_Elem, _Traits, _Alloc2>& _Str, ios_base::openmode _Mode = ios_base::in | ios_base::out)
         : basic_stringbuf(_Str, _Mode, _Alloc{}) {}
 
-    basic_stringbuf(basic_stringbuf&& _Right, const _Alloc& _Al_) : _Mystate(0), _Al(_Al_) {
-        _Assign_rv(_STD move(_Right));
+    basic_stringbuf(basic_stringbuf&& _Right, const _Alloc& _Al_) : _Seekhigh(nullptr), _Mystate(0), _Al(_Al_) {
+        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
 
-    basic_stringbuf(basic_stringbuf&& _Right) : _Mystate(0) {
-        _Assign_rv(_STD move(_Right));
+    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
+        _Seekhigh = _STD exchange(_Right._Seekhigh, nullptr);
+        _Mystate  = _STD exchange(_Right._Mystate, 0);
+        _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 {
+            _Pocma(_Al, _Right._Al);
+            _Take_contents(_STD move(_Right));
         }
     }
 
@@ -584,20 +647,17 @@ 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(basic_istringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+          _Stringbuffer((_Mybase::swap(_Right), _STD move(_Right._Stringbuffer))) {}
 
-    basic_istringstream& operator=(basic_istringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
-    }
-
-    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 +764,17 @@ 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(basic_ostringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+          _Stringbuffer((_Mybase::swap(_Right), _STD move(_Right._Stringbuffer))) {}
 
-    basic_ostringstream& operator=(basic_ostringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
-    }
-
-    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 +887,17 @@ 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(basic_stringstream&& _Right)
+        : _Mybase(_STD addressof(_Stringbuffer)),
+          _Stringbuffer((_Mybase::swap(_Right), _STD move(_Right._Stringbuffer))) {}
 
-    basic_stringstream& operator=(basic_stringstream&& _Right) noexcept /* strengthened */ {
-        _Assign_rv(_STD move(_Right));
-        return *this;
-    }
-
-    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/libcxx/expected_results.txt

@@ -513,9 +513,6 @@ std/algorithms/robust_against_adl.compile.pass.cpp FAIL
 # GH-3100: <memory> etc.: ADL should be avoided when calling _Construct_in_place and its friends
 std/utilities/function.objects/func.wrap/func.wrap.func/robust_against_adl.pass.cpp FAIL
 
-# GH-4232: <sstream>: basic_stringbuf shouldn't implement moving with swapping
-std/input.output/string.streams/stringbuf/stringbuf.cons/move.alloc.pass.cpp FAIL
-
 # GH-4238: <chrono>: file_clock::to_utc() overflows for file_time<nanoseconds>
 # This test also has a bogus assumption about the file_time epoch, and file_time<nanoseconds> is doomed on Windows.
 std/time/time.clock/time.clock.file/ostream.pass.cpp FAIL

tests/std/tests/GH_001858_iostream_exception/test.cpp

@@ -15,6 +15,10 @@
 #include <type_traits>
 #include <utility>
 
+#if _HAS_CXX17
+#include <memory_resource>
+#endif // _HAS_CXX17
+
 #if _HAS_CXX20
 #include <syncstream>
 #endif // _HAS_CXX20
@@ -322,6 +326,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);
     }
 };