From f064ac20c86dc72605d8e70bc58a34bfa285d2a7 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser Date: Tue, 7 Oct 2025 11:38:00 +0200 Subject: [PATCH] [libc++] Simplify the implementation of string::{append,assign,assign_range} --- libcxx/include/string | 40 ++++++------------- .../string_append/iterator.pass.cpp | 15 +++++-- libcxx/test/support/test_iterators.h | 38 ++++++++++++++++++ 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/libcxx/include/string b/libcxx/include/string index 363f27a51648c..1db64e36ae972 100644 --- a/libcxx/include/string +++ b/libcxx/include/string @@ -698,6 +698,8 @@ __concatenate_strings(const _Allocator& __alloc, __type_identity_t > __str1, __type_identity_t > __str2); +// This is true if we know for a fact that dereferencing the iterator won't access any part of the `string` we're +// modifying if __addr_in_range(*it) returns false. template inline const bool __string_is_trivial_iterator_v = false; @@ -1413,24 +1415,16 @@ public: _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(const value_type* _LIBCPP_DIAGNOSE_NULLPTR __s); _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(size_type __n, value_type __c); - template ::value, int> = 0> - _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& - append(_InputIterator __first, _InputIterator __last) { - const basic_string __temp(__first, __last, __alloc_); - append(__temp.data(), __temp.size()); - return *this; - } - - template ::value, int> = 0> + template _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& - append(_ForwardIterator __first, _ForwardIterator __last) { - size_type __sz = size(); - size_type __cap = capacity(); - size_type __n = static_cast(std::distance(__first, __last)); - if (__n == 0) - return *this; + append(_InputIterator __first, _InputIterator __last) { + if (__string_is_trivial_iterator_v<_InputIterator> && !__addr_in_range(*__first)) { + size_type __sz = size(); + size_type __cap = capacity(); + size_type __n = static_cast(std::distance(__first, __last)); + if (__n == 0) + return *this; - if (__string_is_trivial_iterator_v<_ForwardIterator> && !__addr_in_range(*__first)) { if (__cap - __sz < __n) __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0); __annotate_increase(__n); @@ -1540,17 +1534,10 @@ public: _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& assign(const value_type* _LIBCPP_DIAGNOSE_NULLPTR __s); _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& assign(size_type __n, value_type __c); - template ::value, int> = 0> + template _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& assign(_InputIterator __first, _InputIterator __last) { - __assign_with_sentinel(__first, __last); - return *this; - } - - template ::value, int> = 0> - _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& - assign(_ForwardIterator __first, _ForwardIterator __last) { - if (__string_is_trivial_iterator_v<_ForwardIterator>) { + if _LIBCPP_CONSTEXPR (__string_is_trivial_iterator_v<_InputIterator>) { size_type __n = static_cast(std::distance(__first, __last)); __assign_trivial(__first, __last, __n); } else { @@ -1563,8 +1550,7 @@ public: # if _LIBCPP_STD_VER >= 23 template <_ContainerCompatibleRange<_CharT> _Range> _LIBCPP_HIDE_FROM_ABI constexpr basic_string& assign_range(_Range&& __range) { - if constexpr (__string_is_trivial_iterator_v> && - (ranges::forward_range<_Range> || ranges::sized_range<_Range>)) { + if constexpr (__string_is_trivial_iterator_v>) { size_type __n = static_cast(ranges::distance(__range)); __assign_trivial(ranges::begin(__range), ranges::end(__range), __n); diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/iterator.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/iterator.pass.cpp index 684661aeab8fc..e357f65408bae 100644 --- a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/iterator.pass.cpp +++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/iterator.pass.cpp @@ -11,13 +11,14 @@ // template // basic_string& append(InputIterator first, InputIterator last); // constexpr since C++20 -#include #include +#include +#include -#include "test_macros.h" -#include "test_iterators.h" -#include "min_allocator.h" #include "asan_testing.h" +#include "min_allocator.h" +#include "test_iterators.h" +#include "test_macros.h" template TEST_CONSTEXPR_CXX20 void test(S s, It first, It last, S expected) { @@ -221,6 +222,12 @@ TEST_CONSTEXPR_CXX20 void test_string() { s.append(MoveIt(It(std::begin(p))), MoveIt(It(std::end(p) - 1))); assert(s == "ABCD"); } + { + std::vector buffer(5, 'a'); + S s; + s.append(single_pass_iterator>(buffer), single_pass_iterator >()); + assert(s == "aaaaa"); + } } TEST_CONSTEXPR_CXX20 bool test() { diff --git a/libcxx/test/support/test_iterators.h b/libcxx/test/support/test_iterators.h index 0335a4c561017..6a2f2bae7f297 100644 --- a/libcxx/test/support/test_iterators.h +++ b/libcxx/test/support/test_iterators.h @@ -115,6 +115,44 @@ template cpp17_input_iterator(It) -> cpp17_input_iterator; #endif +template +class single_pass_iterator { + Container* container_; + +public: + using iterator_category = std::input_iterator_tag; + using value_type = typename Container::value_type; + using difference_type = typename Container::difference_type; + using pointer = typename Container::pointer; + using reference = typename Container::reference; + + TEST_CONSTEXPR explicit single_pass_iterator() = default; + TEST_CONSTEXPR explicit single_pass_iterator(Container& container) + : container_(container.empty() ? nullptr : &container) {} + + TEST_CONSTEXPR reference operator*() const { return container_->back(); } + + TEST_CONSTEXPR_CXX14 single_pass_iterator& operator++() { + container_->pop_back(); + if (container_->empty()) + container_ = nullptr; + return *this; + } + + TEST_CONSTEXPR_CXX14 single_pass_iterator operator++(int) { return ++(*this); } + + friend TEST_CONSTEXPR bool operator==(const single_pass_iterator& lhs, const single_pass_iterator& rhs) { + return lhs.container_ == rhs.container_; + } + + friend TEST_CONSTEXPR bool operator!=(const single_pass_iterator& lhs, const single_pass_iterator& rhs) { + return !(lhs == rhs); + } + + template + void operator,(const T&) = delete; +}; + #if TEST_STD_VER > 17 static_assert(std::input_iterator>); #endif