[libc++] Fix input-only range handling for basic_string

[frederick-vs-ja]

By calling std::move for related functions when the iterator is possibly input-only. Also slightly changes the conditions of branch for contiguous iterators to avoid error.

Fixes #116502.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

By calling std::move for related functions when the iterator is possibly input-only. Also slightly changes the conditions of branch for contiguous iterators to avoid error.

Fixes #116502.

---

Full diff: https://github.com/llvm/llvm-project/pull/116890.diff

3 Files Affected:

• (modified) libcxx/include/string (+6-5)
• (modified) libcxx/test/std/strings/basic.string/string.cons/from_range.pass.cpp (+19)
• (modified) libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp (+21)

diff --git a/libcxx/include/string b/libcxx/include/string
index bf7fc3c37ecd7a..97951fc9388074 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1921,7 +1921,8 @@ private:
   __copy_non_overlapping_range(_ForwardIter __first, _Sent __last, value_type* __dest) {
 #ifndef _LIBCPP_CXX03_LANG
     if constexpr (__libcpp_is_contiguous_iterator<_ForwardIter>::value &&
-                  is_same<value_type, __iter_value_type<_ForwardIter>>::value && is_same<_ForwardIter, _Sent>::value) {
+                  is_same<value_type, __remove_cvref_t<decltype(*__first)>>::value &&
+                  is_same<_ForwardIter, _Sent>::value) {
       _LIBCPP_ASSERT_INTERNAL(
           !std::__is_overlapping_range(std::__to_address(__first), std::__to_address(__last), __dest),
           "__copy_non_overlapping_range called with an overlapping range!");
@@ -1954,7 +1955,7 @@ private:
     __sz += __n;
     __set_size(__sz);
     traits_type::assign(__p[__sz], value_type());
-    __copy_non_overlapping_range(__first, __last, __p + __ip);
+    __copy_non_overlapping_range(std::move(__first), std::move(__last), __p + __ip);
 
     return begin() + __ip;
   }
@@ -2490,7 +2491,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init_with_size(_InputIterator __fir
 #if _LIBCPP_HAS_EXCEPTIONS
   try {
 #endif // _LIBCPP_HAS_EXCEPTIONS
-    auto __end = __copy_non_overlapping_range(__first, __last, std::__to_address(__p));
+    auto __end = __copy_non_overlapping_range(std::move(__first), std::move(__last), std::__to_address(__p));
     traits_type::assign(*__end, value_type());
 #if _LIBCPP_HAS_EXCEPTIONS
   } catch (...) {
@@ -3082,9 +3083,9 @@ basic_string<_CharT, _Traits, _Allocator>::__insert_with_size(
     return begin() + __ip;
 
   if (__string_is_trivial_iterator<_Iterator>::value && !__addr_in_range(*__first)) {
-    return __insert_from_safe_copy(__n, __ip, __first, __last);
+    return __insert_from_safe_copy(__n, __ip, std::move(__first), std::move(__last));
   } else {
-    const basic_string __temp(__init_with_sentinel_tag(), __first, __last, __alloc_);
+    const basic_string __temp(__init_with_sentinel_tag(), std::move(__first), std::move(__last), __alloc_);
     return __insert_from_safe_copy(__n, __ip, __temp.begin(), __temp.end());
   }
 }
diff --git a/libcxx/test/std/strings/basic.string/string.cons/from_range.pass.cpp b/libcxx/test/std/strings/basic.string/string.cons/from_range.pass.cpp
index 6a1b45b25ef032..bef8872c2bce25 100644
--- a/libcxx/test/std/strings/basic.string/string.cons/from_range.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.cons/from_range.pass.cpp
@@ -12,6 +12,7 @@
 //   constexpr basic_string(from_range_t, R&& rg, const Allocator& a = Allocator());           // since C++23
 
 #include <algorithm>
+#include <sstream>
 #include <string>
 #include <utility>
 #include <vector>
@@ -82,6 +83,13 @@ constexpr void test_with_input(std::vector<char> input) {
     assert(std::ranges::equal(input, c));
     LIBCPP_ASSERT(is_string_asan_correct(c));
   }
+
+  { // Ensure input-only sized ranges are accepted.
+    using input_iter = cpp20_input_iterator<const char*>;
+    const char in[]{'q', 'w', 'e', 'r'};
+    std::string s(std::from_range, std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
+    assert(s == "qwer");
+  }
 }
 
 void test_string_exception_safety_throwing_allocator() {
@@ -116,6 +124,15 @@ constexpr bool test_inputs() {
   return true;
 }
 
+#ifndef TEST_HAS_NO_LOCALIZATION
+void test_counted_istream_view() {
+  std::istringstream is{"qwert"};
+  auto vals = std::views::istream<char>(is);
+  std::string s(std::from_range, std::views::counted(vals.begin(), 3));
+  assert(v == "qwe");
+}
+#endif
+
 int main(int, char**) {
   test_inputs();
   static_assert(test_inputs());
@@ -125,5 +142,7 @@ int main(int, char**) {
   // Note: `test_exception_safety_throwing_copy` doesn't apply because copying a `char` cannot throw.
   test_string_exception_safety_throwing_allocator();
 
+  test_counted_istream_view();
+
   return 0;
 }
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
index 45d1f620e90542..415d78ac1b5223 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
@@ -11,6 +11,7 @@
 // template<container-compatible-range<charT> R>
 //   constexpr iterator insert_range(const_iterator p, R&& rg);                                // C++23
 
+#include <sstream>
 #include <string>
 
 #include "../../../../containers/sequences/insert_range_sequence_containers.h"
@@ -27,9 +28,27 @@ constexpr bool test_constexpr() {
         []([[maybe_unused]] auto&& c) { LIBCPP_ASSERT(c.__invariants()); });
   });
 
+  { // Ensure input-only sized ranges are accepted.
+    using input_iter = cpp20_input_iterator<const char*>;
+    const char in[]{'q', 'w', 'e', 'r'};
+    std::string s = "zxcv";
+    s.insert_range(s.begin(), std::views::counted(input_iter{std::ranges::begin(in)}, std::ranges::ssize(in)));
+    assert(s == "qwerzxcv");
+  }
+
   return true;
 }
 
+#ifndef TEST_HAS_NO_LOCALIZATION
+void test_counted_istream_view() {
+  std::istringstream is{"qwert"};
+  auto vals     = std::views::istream<char>(is);
+  std::string s = "zxcv";
+  s.insert_range(s.begin(), std::views::counted(vals.begin(), 3));
+  assert(s == "qwezxcv");
+}
+#endif
+
 int main(int, char**) {
   static_assert(test_constraints_insert_range<std::basic_string, char, int>());
 
@@ -39,6 +58,8 @@ int main(int, char**) {
   });
   static_assert(test_constexpr());
 
+  test_counted_istream_view();
+
   // Note: `test_insert_range_exception_safety_throwing_copy` doesn't apply because copying chars cannot throw.
   {
 #if !defined(TEST_HAS_NO_EXCEPTIONS)