Skip to content

Commit

Permalink
<sstream>: Don't swap basic_stringbufs in move assignment and all…
Browse files Browse the repository at this point in the history 
…ocator-extended construction (#4239)

Co-authored-by: Stephan T. Lavavej <[email protected]>
  • Loading branch information
@frederick-vs-ja @StephanTLavavej
frederick-vs-ja and StephanTLavavej authored Jan 24, 2024
1 parent 2644138 commit 63aeb77
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 43 deletions.
134 changes: 94 additions & 40 deletions stl/inc/sstream
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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 */ {
Expand Down Expand Up @@ -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 */ {
Expand Down Expand Up @@ -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 */ {
Expand Down
3 changes: 0 additions & 3 deletions tests/libcxx/expected_results.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions tests/std/tests/GH_001858_iostream_exception/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>);
Expand Down
17 changes: 17 additions & 0 deletions tests/std/tests/P0408R7_efficient_access_to_stringbuf_buffer/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <sstream>
#include <string>
#include <string_view>
#include <type_traits>
#include <utility>

#include <test_death.hpp>
Expand Down Expand Up @@ -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);
}
};

Expand Down

0 comments on commit 63aeb77

Please sign in to comment.