[libc++] Don't pass the allocator in substr()

[philnik]

Summary

This bug was dicoved when implementing P2438R2.

Fixes #57190

[ldionne]

LGTM w/ comment. Also please rebase and get a green CI run.

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp

1 ↗

(On Diff #475603)

I think this should be added to libcxx/test/std/strings/basic.string/string.ops/string_substr/substr.pass.cpp instead.

[Mordante]

LGTM, after addressing all review comments.

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp

21 ↗

(On Diff #475603)

This comment sounds like this test tests libc++ specific behaviour. Can you add some guards to the libc++ specific tests so it will pass of other Standard libraries too?

[philnik]

Address comments

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp

21 ↗

(On Diff #475603)

The allocator isn't actually forwarded, so we always copy it. Removed the comment.

libcxx/include/string

 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
     basic_string& replace(const_iterator __i1, const_iterator __i2, initializer_list<value_type> __il)
         {return replace(__i1, __i2, __il.begin(), __il.end());}
 #endif // _LIBCPP_CXX03_LANG
 
     _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type copy(value_type* __s, size_type __n, size_type __pos = 0) const;
 
-    // TODO: Maybe don't pass in the allocator. See https://llvm.org/PR57190
 #if _LIBCPP_STD_VER <= 20
     _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
     basic_string substr(size_type __pos = 0, size_type __n = npos) const {
-      return basic_string(*this, __pos, __n, __alloc());
+      return basic_string(*this, __pos, __n);
     }
 #else
     _LIBCPP_HIDE_FROM_ABI constexpr
     basic_string substr(size_type __pos = 0, size_type __n = npos) const& {
-      return basic_string(*this, __pos, __n, __alloc());
+      return basic_string(*this, __pos, __n);
     }
 
     _LIBCPP_HIDE_FROM_ABI constexpr
     basic_string substr(size_type __pos = 0, size_type __n = npos) && {
-      return basic_string(std::move(*this), __pos, __n, __alloc());
+      return basic_string(std::move(*this), __pos, __n);
     }
 #endif
 
     _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
     void swap(basic_string& __str)
 #if _LIBCPP_STD_VER >= 14
         _NOEXCEPT;
 #else

libcxx/test/std/strings/basic.string/string.ops/string_substr/substr.pass.cpp

 // basic_string substr(size_type pos = 0, size_type n = npos) const; // constexpr since C++20, removed in C++23
 // basic_string substr(size_type pos = 0, size_type n = npos) const&; // since in C++23
 
 #include <string>
 #include <stdexcept>
 #include <algorithm>
 #include <cassert>
 
+#include "test_allocator.h"
 #include "test_macros.h"
 #include "min_allocator.h"
 
 template <class S>
 TEST_CONSTEXPR_CXX20 void
 test(const S& s, typename S::size_type pos, typename S::size_type n)
 {
     if (pos <= s.size())
TEST_CONSTEXPR_CXX20 bool test() {
   test_string<std::string>();
 #if TEST_STD_VER >= 11
   test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
 #endif
 
   return true;
 }
 
+TEST_CONSTEXPR_CXX20 bool test_alloc() {
+  {
+    using alloc  = test_allocator<char>;
+    using string = std::basic_string<char, std::char_traits<char>, alloc>;
+    test_allocator_statistics stats;
+    {
+      string str((alloc(&stats)));
+      stats = test_allocator_statistics();
+      (void)str.substr();
+      assert(stats.moved == 0);
+      assert(stats.copied == 0);
+    }
+    {
+      string str((alloc(&stats)));
+      stats = test_allocator_statistics();
+      (void)std::move(str).substr();
+      assert(stats.moved == 0);
+      assert(stats.copied == 0);
+    }
+  }
+
+  return true;
+}
+
 int main(int, char**)
 {
   test();
+  test_alloc();
 #if TEST_STD_VER > 17
   static_assert(test());
+  static_assert(test_alloc());
 #endif
 
   return 0;
 }