Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement SSO for constexpr string #1735

Merged
merged 5 commits into from
Jun 19, 2022
Merged

Implement SSO for constexpr string #1735

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6368f07
Allow for SSO for constexpr string
miscco Mar 13, 2021
e5a45a8
Drop one more constexpr SSO workaround.
StephanTLavavej Jun 17, 2022
806301f
Centralize code to activate SSO elements
CaseyCarter Jun 18, 2022
66eeb12
Even tinier code review feedback.
StephanTLavavej Jun 18, 2022
3d2e9cd
_Activate_SSO_buffer can/does not throw
CaseyCarter Jun 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 32 additions & 107 deletions stl/inc/xstring
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2279,12 +2279,18 @@ public:
}

_CONSTEXPR20 bool _Large_string_engaged() const noexcept {
return _BUF_SIZE <= _Myres;
}

constexpr void _Activate_SSO_buffer() {
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
// begin the lifetime of the array elements (e.g., before copying into them)
#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
return true;
for (size_type _Idx = 0; _Idx < _BUF_SIZE; ++_Idx) {
_Bx._Buf[_Idx] = value_type();
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}
}
#endif // _HAS_CXX20
return _BUF_SIZE <= _Myres;
}

_CONSTEXPR20 void _Check_offset(const size_type _Off) const {
Expand All @@ -2311,7 +2317,7 @@ public:
}

union _Bxty { // storage for small buffer or pointer to larger one
_CONSTEXPR20 _Bxty() noexcept : _Ptr() {} // user-provided, for fancy pointers
_CONSTEXPR20 _Bxty() noexcept : _Buf() {} // user-provided, for fancy pointers
miscco marked this conversation as resolved.
Show resolved Hide resolved

_CONSTEXPR20 ~_Bxty() noexcept {} // user-provided, for fancy pointers

Expand Down Expand Up @@ -2674,6 +2680,8 @@ private:
enum class _Construct_strategy : uint8_t { _From_char, _From_ptr, _From_string };
template <_Construct_strategy _Strat, class _Char_or_ptr>
_CONSTEXPR20 void _Construct(const _Char_or_ptr _Arg, _CRT_GUARDOVERFLOW const size_type _Count) {
// Pre: *this is in SSO mode; the lifetime of the SSO elements has already begun

if constexpr (_Strat == _Construct_strategy::_From_char) {
_STL_INTERNAL_STATIC_ASSERT(is_same_v<_Char_or_ptr, _Elem>);
} else {
Expand All @@ -2689,24 +2697,7 @@ private:
auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Al);
_Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data);

#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
_My_data._Myres = _BUF_SIZE; // TRANSITION: constexpr SSO
}

const bool _Stay_small = _Count < _BUF_SIZE && !_STD is_constant_evaluated();
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
const bool _Stay_small = _Count < _BUF_SIZE;
#endif // _HAS_CXX20

if (_Stay_small) {
#if _HAS_CXX20
// TRANSITION: This is currently unused until SSO support is merged
if (_STD is_constant_evaluated()) {
_Construct_in_place(_My_data._Bx);
}
#endif // _HAS_CXX20

if (_Count < _BUF_SIZE) {
_My_data._Mysize = _Count;
_My_data._Myres = _BUF_SIZE - 1;
if constexpr (_Strat == _Construct_strategy::_From_char) {
Expand Down Expand Up @@ -2757,32 +2748,23 @@ private:

template <class _Iter>
_CONSTEXPR20 void _Construct_from_iter(_Iter _First, const _Iter _Last) {
// Pre: *this is in SSO mode; the lifetime of the SSO elements has already begun

auto& _My_data = _Mypair._Myval2;
auto& _Al = _Getal();
auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Al);
_Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data);

_My_data._Mysize = 0;
_My_data._Myres = _BUF_SIZE - 1;
#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
_Construct_in_place(_My_data._Bx);
_My_data._Myres = _BUF_SIZE; // TRANSITION: constexpr SSO
}
#endif // _HAS_CXX20

if constexpr (_Is_fwd_iter_v<_Iter>) {
const auto _Count = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_First, _Last)));
if (_Count > max_size()) {
_Xlen_string(); // result too long
}

#if _HAS_CXX20
const bool _Become_large = _Count >= _BUF_SIZE || _STD is_constant_evaluated();
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
const bool _Become_large = _Count >= _BUF_SIZE;
#endif // _HAS_CXX20
if (_Become_large) {
if (_Count >= _BUF_SIZE) {
const size_type _New_capacity = _Calculate_growth(_Count);
const pointer _New_ptr = _Al.allocate(_New_capacity + 1); // throws
_Construct_in_place(_My_data._Bx._Ptr, _New_ptr);
Expand Down Expand Up @@ -2873,18 +2855,10 @@ public:
auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal());
_Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws

#if _HAS_CXX20
const bool _Activate_large_mode = _New_capacity < _New_size || _STD is_constant_evaluated();
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
const bool _Activate_large_mode = _New_capacity < _New_size;
#endif // _HAS_CXX20

if (_Activate_large_mode) {
// we should never allocate less than _BUF_SIZE space (_New_size could be small if constant evaluated)
const size_type _Requested_size = (_STD max)(_New_size, _BUF_SIZE);
_New_capacity = _Calculate_growth(_Requested_size, _BUF_SIZE - 1, max_size());
const pointer _Fancyptr = _Getal().allocate(_New_capacity + 1); // throws
_Ptr = _Unfancy(_Fancyptr);
if (_New_capacity < _New_size) {
_New_capacity = _Calculate_growth(_New_size, _BUF_SIZE - 1, max_size());
const pointer _Fancyptr = _Getal().allocate(_New_capacity + 1); // throws
_Ptr = _Unfancy(_Fancyptr);
_Construct_in_place(_My_data._Bx._Ptr, _Fancyptr);

#if _HAS_CXX20
Expand Down Expand Up @@ -3146,11 +3120,7 @@ private:
_Right_data._Bx._Ptr = nullptr;
_Swap_proxy_and_iterators(_Right);
} else { // copy small string buffer
#if _HAS_CXX20
if (_STD is_constant_evaluated()) { // begin the lifetime of the array elements before copying into them
_Construct_in_place(_Mypair._Myval2._Bx);
}
#endif // _HAS_CXX20
_Mypair._Myval2._Activate_SSO_buffer();
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Traits::copy(_My_data._Bx._Buf, _Right_data._Bx._Buf, _Right_data._Mysize + 1);
_Right_data._Orphan_all();
}
Expand Down Expand Up @@ -3222,9 +3192,6 @@ public:
private:
_CONSTEXPR20 void _Copy_assign_val_from_small(const basic_string& _Right) {
// TRANSITION, VSO-761321; inline into only caller when that's fixed
#if _HAS_CXX20
_STL_ASSERT(!_STD is_constant_evaluated(), "SSO should be disabled in a constexpr context");
#endif // _HAS_CXX20
_Tidy_deallocate();
if constexpr (_Can_memcpy_val) {
#if _HAS_CXX20
Expand Down Expand Up @@ -4007,22 +3974,12 @@ public:
return;
}

#if _HAS_CXX20
const bool _Do_become_small = _My_data._Mysize < _BUF_SIZE && !_STD is_constant_evaluated();
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
const bool _Do_become_small = _My_data._Mysize < _BUF_SIZE;
#endif // _HAS_CXX20
if (_Do_become_small) {
if (_My_data._Mysize < _BUF_SIZE) {
_Become_small();
return;
}

size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size());
#if _HAS_CXX20
// must allocate at least _BUF_SIZE space
_Target_capacity = (_STD max)(_Target_capacity, _BUF_SIZE);
#endif // _HAS_CXX20

const size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size());
if (_Target_capacity < _My_data._Myres) { // worth shrinking, do it
auto& _Al = _Getal();
const pointer _New_ptr = _Al.allocate(_Target_capacity + 1); // throws
Expand Down Expand Up @@ -4280,11 +4237,7 @@ public:
// exchange a string in large mode with one in small mode
const pointer _Ptr = _Starts_large._Bx._Ptr;
_Destroy_in_place(_Starts_large._Bx._Ptr);
#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
_Construct_in_place(_Starts_large._Bx);
}
#endif // _HAS_CXX20
_Starts_large._Activate_SSO_buffer();
_Traits::copy(_Starts_large._Bx._Buf, _Starts_small._Bx._Buf, _BUF_SIZE);
_Construct_in_place(_Starts_small._Bx._Ptr, _Ptr);
}
Expand Down Expand Up @@ -4853,11 +4806,7 @@ private:
const pointer _Ptr = _My_data._Bx._Ptr;
auto& _Al = _Getal();
_Destroy_in_place(_My_data._Bx._Ptr);
mnatsuhara marked this conversation as resolved.
Show resolved Hide resolved
#if _HAS_CXX20
if (_STD is_constant_evaluated()) { // begin the lifetime of the array elements before copying into them
_Construct_in_place(_Mypair._Myval2._Bx);
}
#endif // _HAS_CXX20
_Mypair._Myval2._Activate_SSO_buffer();
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Traits::copy(_My_data._Bx._Buf, _Unfancy(_Ptr), _My_data._Mysize + 1);
_Al.deallocate(_Ptr, _My_data._Myres + 1);
_My_data._Myres = _BUF_SIZE - 1;
Expand All @@ -4872,24 +4821,11 @@ private:
_CONSTEXPR20 void _Tidy_init() noexcept { // initialize basic_string data members
auto& _My_data = _Mypair._Myval2;
_My_data._Mysize = 0;
_My_data._Myres = _BUF_SIZE - 1;
_My_data._Activate_SSO_buffer();

#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
_My_data._Myres = _BUF_SIZE; // SSO disabled in constexpr context
auto& _Al = _Getal();
const pointer _New_ptr = _Al.allocate(_BUF_SIZE + 1); // throws
_My_data._Bx._Ptr = _New_ptr;

_Elem* const _Raw_new = _Unfancy(_New_ptr);
_Traits::assign(_Raw_new, _BUF_SIZE + 1, _Elem());
} else
#endif // _HAS_CXX20
{
_My_data._Myres = _BUF_SIZE - 1;
// the _Traits::assign is last so the codegen doesn't think the char write can alias this
_Traits::assign(_My_data._Bx._Buf[0], _Elem());
}

// the _Traits::assign is last so the codegen doesn't think the char write can alias this
_Traits::assign(_My_data._Bx._Buf[0], _Elem());
_ASAN_STRING_CREATE(*this);
}

Expand All @@ -4901,25 +4837,14 @@ private:
const pointer _Ptr = _My_data._Bx._Ptr;
auto& _Al = _Getal();
_Destroy_in_place(_My_data._Bx._Ptr);
#if _HAS_CXX20
if (_STD is_constant_evaluated()) { // begin the lifetime of the array elements before copying into them
_Construct_in_place(_My_data._Bx);
}
#endif // _HAS_CXX20
_My_data._Activate_SSO_buffer();
_Al.deallocate(_Ptr, _My_data._Myres + 1);
}

_My_data._Mysize = 0;
#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
_My_data._Myres = 0;
} else
#endif // _HAS_CXX20
{
_My_data._Myres = _BUF_SIZE - 1;
// the _Traits::assign is last so the codegen doesn't think the char write can alias this
_Traits::assign(_My_data._Bx._Buf[0], _Elem());
}
_My_data._Myres = _BUF_SIZE - 1;
// the _Traits::assign is last so the codegen doesn't think the char write can alias this
_Traits::assign(_My_data._Bx._Buf[0], _Elem());
}

public:
Expand Down
14 changes: 5 additions & 9 deletions tests/std/tests/P0980R1_constexpr_strings/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,16 +613,12 @@ constexpr bool test_interface() {
literal_constructed.shrink_to_fit();
miscco marked this conversation as resolved.
Show resolved Hide resolved

const auto c4 = literal_constructed.capacity();
if (is_constant_evaluated()) { // check minimum allocation of _BUF_SIZE when constant evaluated
assert(c4 == 16 / sizeof(CharType));
if constexpr (is_same_v<CharType, char16_t> || is_same_v<CharType, wchar_t>) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
assert(c4 == 7);
} else if constexpr (is_same_v<CharType, char32_t>) {
assert(c4 == 3);
} else {
if constexpr (is_same_v<CharType, char16_t> || is_same_v<CharType, wchar_t>) {
assert(c4 == 7);
} else if constexpr (is_same_v<CharType, char32_t>) {
assert(c4 == 3);
} else {
assert(c4 == 15);
}
assert(c4 == 15);
}
}

Expand Down