libstdc++: Implement rvalue overload for basic_string::substr() [PR119745]

This paper implement the changes from P2438R2 basic_string::substr() &&
paper into C++26. The additional substr and constructor overload are
implemented only for SSO string, as they require mutating the content
(if reused), and thus would require copy of the string anyway for COW
strings. (In consequence allocators implicitly constructible from int
remain ambiguous for C++11 ABI strings, see r16-7497-gfa1149534d8580).

In addition to cases when the allocators are not compatible (equal),
this patch does not reuse (transfer) allocator storage, if the selected
substring fits inside the SSO buffer, so we do not risk keeping large
chunk of memory for few characters. (This also covers cases when the
source stored the content in the local buffer).

As this additional overloads are meant to be optimization, in contrast
to move constructor, the source is left unmodified if the allocation
is not transferred. This avoid introducing a write (of null terminator)
to previously untouched, heap allocated, memory.

Separate overloads for substr(size_type __pos, size_type __n) and
substr(size_type __pos == 0), that delegate to corresponding constructor,
are provided to avoid the check of __n against the length() in the later
case.

Finally, the signatures of existing substr() overload are not modified
(no longer required since C++20), which avoid any impact on the ABI.

	PR libstdc++/119745

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h (basic_string::_M_construct)
	[__cplusplus >= 202302L]: Declare.
	(basic_string::basic_string(basic_string&&, size_type, const _Alloc&))
	(basic_string(basic_string&&, size_type, size_type, const _Alloc&))
	(basic_string::substr(size_type, size_type) &&)
	(basic_string::substr(size_type) &&) [__cplusplus >= 202302L]: Define.
	* include/bits/basic_string.tcc (basic_string::_M_construct)
	[__cplusplus >= 202302L]: Define.
	* testsuite/21_strings/basic_string/operations/substr/rvalue.cc: New test.

Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
Reviewed-by: Patrick Palka <ppalka@redhat.com>
Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index cd6f312f1bd2dcbac8eed2386a7c859a846caef7..9bbe16507d0087c00c6dbffaab5aedc643795a06 100644 (file)
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -354,6 +354,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
        void
        _M_construct(const _CharT *__c, size_type __n);
 
+#if __cplusplus >= 202302L
+      constexpr void
+      _M_construct(basic_string&& __str, size_type __pos,  size_type __n);
+#endif
+
       _GLIBCXX20_CONSTEXPR
       allocator_type&
       _M_get_allocator()
@@ -671,6 +676,26 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
                     std::forward_iterator_tag());
       }
 
+#if __cplusplus >= 202302L
+      _GLIBCXX20_CONSTEXPR
+      basic_string(basic_string&& __str, size_type __pos,
+                  const _Alloc& __a = _Alloc())
+      : _M_dataplus(_M_local_data(), __a)
+      {
+       __pos = __str._M_check(__pos, "string::string");
+       _M_construct(std::move(__str), __pos, __str.length() - __pos);
+      }
+
+      _GLIBCXX20_CONSTEXPR
+      basic_string(basic_string&& __str, size_type __pos, size_type __n,
+                  const _Alloc& __a = _Alloc())
+      : _M_dataplus(_M_local_data(), __a)
+      {
+       __pos = __str._M_check(__pos, "string::string");
+       _M_construct(std::move(__str), __pos, __str._M_limit(__pos, __n));
+      }
+#endif // C++23
+
       /**
        *  @brief  Construct string initialized by a character %array.
        *  @param  __s  Source character %array.
@@ -3442,6 +3467,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       { return basic_string(*this,
                            _M_check(__pos, "basic_string::substr"), __n); }
 
+#if __cplusplus >= 202302L
+      _GLIBCXX_NODISCARD
+      constexpr basic_string
+      substr(size_type __pos = 0) &&
+      { return basic_string(std::move(*this), __pos); }
+
+      _GLIBCXX_NODISCARD
+      constexpr basic_string
+      substr(size_type __pos, size_type __n) &&
+      { return basic_string(std::move(*this), __pos, __n); }
+#endif // C++23
+
 #ifdef __glibcxx_string_subview // >= C++26
       /**
        *  @brief  Get a subview.

diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 75ffea42cedfb9969e68655b3182c2ab00738a34..a223edf67acc266b7f995d19d9790df25e31f2f3 100644 (file)
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -302,6 +302,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        traits_type::assign(_M_data()[__n], _CharT());
     }
 
+#if __cplusplus >= 202302L
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    constexpr void
+    basic_string<_CharT, _Traits, _Alloc>::
+    _M_construct(basic_string&& __str, size_type __pos,  size_type __n)
+    {
+      const _CharT* __start = __str._M_data() + __pos;
+      if (__n <= _S_local_capacity)
+       {
+         _M_init_local_buf();
+         traits_type::copy(_M_local_buf, __start, __n);
+         _M_set_length(__n);
+         return;
+       }
+
+      if constexpr (!allocator_traits<_Alloc>::is_always_equal::value)
+       if (get_allocator() != __str.get_allocator())
+         {
+           _M_construct<false>(__start, __n);
+           return;
+         }
+
+      _M_data(__str._M_data());
+      _M_capacity(__str._M_allocated_capacity);
+      __str._M_data(__str._M_use_local_data());
+      __str._M_set_length(0);
+
+      _S_move(_M_data(), _M_data() + __pos, __n);
+      _M_set_length(__n);
+    }
+#endif // C++23
+
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX20_CONSTEXPR
     void

diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/substr/rvalue.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/substr/rvalue.cc
new file mode 100644 (file)
index 0000000..70ccea6
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/substr/rvalue.cc
@@ -0,0 +1,359 @@
+// { dg-do run { target c++26 } }
+
+#include <string_view>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+// When selection is short enough to fit into SSO, the rhs
+// is left unchanged.
+template<typename CharT, typename Allocator>
+constexpr void
+do_test_short_in(const CharT* cstr, Allocator alloc)
+{
+  using StringView = std::basic_string_view<CharT>;
+  using String = std::basic_string<CharT, std::char_traits<CharT>, Allocator>;
+
+  const Allocator defAlloc;
+  String src, res;
+
+  // substr(0)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(0);
+  VERIFY( res == StringView(cstr).substr(0) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res1(std::move(src), 0);
+  VERIFY( res1 == StringView(cstr).substr(0) );
+  VERIFY( res1.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res2(std::move(src), 0, alloc);
+  VERIFY( res2 == StringView(cstr).substr(0) );
+  VERIFY( res2.get_allocator() == alloc );
+  VERIFY( src == cstr );
+
+  // substr(1)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(1);
+  VERIFY( res == StringView(cstr).substr(1) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res3(std::move(src), 1);
+  VERIFY( res3 == StringView(cstr).substr(1) );
+  VERIFY( res3.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res4(std::move(src), 1, alloc);
+  VERIFY( res4 == StringView(cstr).substr(1) );
+  VERIFY( res4.get_allocator() == alloc );
+  VERIFY( src == cstr );
+
+  // substr(1, 1000)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(1, 1000);
+  VERIFY( res == StringView(cstr).substr(1, 1000) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res5(std::move(src), 1, 1000);
+  VERIFY( res5 == StringView(cstr).substr(1, 1000) );
+  VERIFY( res5.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res6(std::move(src), 1, 1000, alloc);
+  VERIFY( res6 == StringView(cstr).substr(1, 1000) );
+  VERIFY( res6.get_allocator() == alloc );
+  VERIFY( src == cstr );
+}
+
+template<typename CharT, typename Allocator>
+constexpr void
+do_test_short_sel(const CharT* cstr, Allocator alloc)
+{
+  using StringView = std::basic_string_view<CharT>;
+  using String = std::basic_string<CharT, std::char_traits<CharT>, Allocator>;
+
+  const Allocator defAlloc;
+  String src, res;
+
+  // substr(0, 2)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(0, 2);
+  VERIFY( res == StringView(cstr).substr(0, 2) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res1(std::move(src), 0, 2);
+  VERIFY( res1 == StringView(cstr).substr(0, 2) );
+  VERIFY( res1.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res2(std::move(src), 0, 2, alloc);
+  VERIFY( res2 == StringView(cstr).substr(0, 2) );
+  VERIFY( res2.get_allocator() == alloc );
+  VERIFY( src == cstr );
+
+  // substr(1, 2)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(1, 2);
+  VERIFY( res == StringView(cstr).substr(1, 2) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res3(std::move(src), 1, 2);
+  VERIFY( res3 == StringView(cstr).substr(1, 2) );
+  VERIFY( res3.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res4(std::move(src), 1, 2, alloc);
+  VERIFY( res4 == StringView(cstr).substr(1, 2) );
+  VERIFY( res4.get_allocator() == alloc );
+  VERIFY( src == cstr );
+
+  // libstdc++ specific
+  constexpr int max_short = (sizeof(CharT) == 1) ? 15 : 3;
+  // substr(0, max_short)
+  src = String(cstr, alloc);
+  res = std::move(src).substr(0, max_short);
+  VERIFY( res == StringView(cstr).substr(0, max_short) );
+  VERIFY( res.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res5(std::move(src), 0, max_short);
+  VERIFY( res5 == StringView(cstr).substr(0, max_short) );
+  VERIFY( res5.get_allocator() == defAlloc );
+  VERIFY( src == cstr );
+
+  src = String(cstr, alloc);
+  String res6(std::move(src), 0, max_short, alloc);
+  VERIFY( res6 == StringView(cstr).substr(0, max_short) );
+  VERIFY( res6.get_allocator() == alloc );
+  VERIFY( src == cstr );
+}
+
+// If the selection is long enough to do not fit into SSO,
+// the memory is moved if allocators are compatible.
+template<typename CharT, typename Allocator>
+constexpr void
+do_test_long(const CharT* cstr, Allocator alloc)
+{
+  using StringView = std::basic_string_view<CharT>;
+  using String = std::basic_string<CharT, std::char_traits<CharT>, Allocator>;
+
+  const Allocator defAlloc;
+  String src, res;
+  const CharT* data;
+
+  auto verifyMove = [&](const String& str)
+  {
+    // Only SSO string provide rvalue overloads of
+    // substr and corresponding constructor.
+#if _GLIBCXX_USE_CXX11_ABI || !_GLIBCXX_USE_DUAL_ABI
+    if (str.get_allocator() == alloc) {
+      VERIFY( str.data() == data );
+      VERIFY( src.empty() );
+    } else
+#endif
+      VERIFY( src == cstr );
+  };
+
+  // substr(0)
+  src = String(cstr, alloc);
+  data = src.data();
+  res = std::move(src).substr(0);
+  VERIFY( res == StringView(cstr).substr(0) );
+  VERIFY( res.get_allocator() == defAlloc );
+  verifyMove(res);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res1(std::move(src), 0);
+  VERIFY( res1 == StringView(cstr).substr(0) );
+  VERIFY( res1.get_allocator() == defAlloc );
+  verifyMove(res1);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res2(std::move(src), 0, alloc);
+  VERIFY( res2 == StringView(cstr).substr(0) );
+  VERIFY( res2.get_allocator() == alloc );
+  verifyMove(res2);
+
+  // substr(5)
+  src = String(cstr, alloc);
+  data = src.data();
+  res = std::move(src).substr(5);
+  VERIFY( res == StringView(cstr).substr(5) );
+  VERIFY( res.get_allocator() == defAlloc );
+  verifyMove(res);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res3(std::move(src), 5);
+  VERIFY( res3 == StringView(cstr).substr(5) );
+  VERIFY( res3.get_allocator() == defAlloc );
+  verifyMove(res3);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res4(std::move(src), 5, alloc);
+  VERIFY( res4 == StringView(cstr).substr(5) );
+  verifyMove(res4);
+
+  // substr(0, 50)
+  src = String(cstr, alloc);
+  data = src.data();
+  res = std::move(src).substr(0, 50);
+  VERIFY( res == StringView(cstr).substr(0, 50) );
+  VERIFY( res.get_allocator() == defAlloc );
+  verifyMove(res);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res5(std::move(src), 0, 50);
+  VERIFY( res5 == StringView(cstr).substr(0, 50) );
+  VERIFY( res5.get_allocator() == defAlloc );
+  verifyMove(res5);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res6(std::move(src), 0, 50, alloc);
+  VERIFY( res6 == StringView(cstr).substr(0, 50) );
+  VERIFY( res6.get_allocator() == alloc );
+  verifyMove(res6);
+
+  // substr(5, 50)
+  src = String(cstr, alloc);
+  data = src.data();
+  res = std::move(src).substr(5, 50);
+  VERIFY( res == StringView(cstr).substr(5, 50) );
+  VERIFY( res.get_allocator() == defAlloc );
+  verifyMove(res);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res7(std::move(src), 5, 50);
+  VERIFY( res7 == StringView(cstr).substr(5, 50) );
+  VERIFY( res7.get_allocator() == defAlloc );
+  verifyMove(res7);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res8(std::move(src), 5, 50, alloc);
+  VERIFY( res8 == StringView(cstr).substr(5, 50) );
+  VERIFY( res8.get_allocator() == alloc );
+  verifyMove(res8);
+
+  // substr(5, 100)
+  src = String(cstr, alloc);
+  data = src.data();
+  res = std::move(src).substr(5, 1000);
+  VERIFY( res == StringView(cstr).substr(5, 1000) );
+  VERIFY( res.get_allocator() == defAlloc );
+  verifyMove(res);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res9(std::move(src), 5, 1000);
+  VERIFY( res9 == StringView(cstr).substr(5, 1000) );
+  VERIFY( res9.get_allocator() == defAlloc );
+  verifyMove(res9);
+
+  src = String(cstr, alloc);
+  data = src.data();
+  String res10(std::move(src), 5, 1000, alloc);
+  VERIFY( res10 == StringView(cstr).substr(5, 1000) );
+  VERIFY( res10.get_allocator() == alloc );
+  verifyMove(res10);
+}
+
+template<typename CharT, typename Allocator>
+constexpr void
+do_test_alloc(const CharT* sht, const CharT* lng, Allocator alloc)
+{
+  do_test_short_in(sht, alloc);
+  do_test_short_sel(sht, alloc);
+  do_test_short_sel(lng, alloc);
+  do_test_long(lng, alloc);
+}
+
+template<typename CharT>
+constexpr void
+do_test_bounds(const CharT* cstr)
+{
+  using String = std::basic_string<CharT>;
+  try
+  {
+    auto res = String(cstr).substr(30);
+    VERIFY(false);
+  } catch (const std::out_of_range&) {
+    VERIFY(true);
+  }
+
+  try
+  {
+    auto res = String(cstr).substr(30, 20);
+    VERIFY(false);
+  } catch (const std::out_of_range&) {
+    VERIFY(true);
+  }
+
+  try
+  {
+    String res(String(cstr), 30);
+    VERIFY(false);
+  } catch (const std::out_of_range&) {
+    VERIFY(true);
+  }
+
+  try
+  {
+    String res(String(cstr), 30, 20);
+    VERIFY(false);
+  } catch (const std::out_of_range&) {
+    VERIFY(true);
+  }
+}
+
+template<typename CharT>
+constexpr void
+do_test(const CharT* sht, const CharT* lng)
+{
+  do_test_alloc(sht, lng, std::allocator<CharT>());
+  if ! consteval {
+    do_test_alloc(sht, lng, __gnu_test::uneq_allocator<CharT>());
+    do_test_alloc(sht, lng, __gnu_test::uneq_allocator<CharT>(42));
+    do_test_bounds(sht);
+  }
+}
+
+constexpr bool
+test_all()
+{
+  do_test(
+    "abcdefghijklmop",
+    "some very very long string, that will not use SSO, and have  at least fifty characters");
+  do_test(
+    L"abc", 
+    L"some very very long string, that will not use SSO, and have  at least fifty characters");
+
+  return true;
+}
+
+int main()
+{ 
+  test_all();
+}