[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

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

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

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.cons/substr.allocator.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <string>
+
+#include "test_allocator.h"
+#include "test_macros.h"
+
+TEST_CONSTEXPR_CXX20 bool test() {
+  {
+    using alloc  = test_allocator<char>;
+    using string = std::basic_string<char, std::char_traits<char>, alloc>;
+    test_allocator_statistics stats;
+    {
+      string str((alloc(&stats)));
+      // Before C++17 the above constructor call may generate a call to the move constructor of the allocator
+      stats = test_allocator_statistics();
+      (void)str.substr();
+      assert(stats.moved == 0);
+      assert(stats.copied == 0);
+    }
+    {
+      string str((alloc(&stats)));
+      // Before C++17 the above constructor call may generate a call to the move constructor of the allocator
+      stats = test_allocator_statistics();
+      (void)std::move(str).substr();
+      assert(stats.moved == 0);
+      assert(stats.copied == 0);
+    }
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+#if TEST_STD_VER >= 20
+  static_assert(test());
+#endif
+
+  return 0;
+}