On Sat, Apr 12, 2025 at 4:01 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> My recent r15-9381-g648d5c26e25497 change assumes that a contiguous > iterator with the correct value_type can be converted to a const charT* > but that's not true for volatile charT*. The optimization should only be > done if it can be converted to the right pointer type. > > Additionally, the generic loop for non-contiguous iterators needs an > explicit cast to deal with iterators that have a reference type that is > not explicitly convertible to charT. > > libstdc++-v3/ChangeLog: > > PR libstdc++/119748 > * include/bits/basic_string.h (_S_copy_chars): Only optimize for > contiguous iterators that are convertible to const charT*. Use > explicit conversion to charT after dereferencing iterator. > (_S_copy_range): Likewise for contiguous ranges. > * include/bits/cow_string.h (_S_copy_chars): Likewise. > (basic_string(from_range_t, R&&, const Alloc&)): Likewise. > * testsuite/21_strings/basic_string/cons/char/119748.cc: New > test. > * testsuite/21_strings/basic_string/cons/wchar_t/119748.cc: > New test. > --- > > Tested x86_64-linux. > > The static_cast<_CharT>(*__k1) bit in _S_copy_chars is relevant for the > branches too. Thinking about it now, I should probably add another > test with iterators that have a value_type with 'explicit operator char()' > because volatile char* isn't the only case that fails. > The range constructor is constrained by *container-compatible-range* <https://eel.is/c++draft/containers#concept:container-compatible-range> (https://eel.is/c++draft/containers#concept:container-compatible-range), that requires conversion to value type to be implicit. Similar requirement exists for iterator-pair algorithms: https://eel.is/c++draft/containers#sequence.reqmts-2.5 > > libstdc++-v3/include/bits/basic_string.h | 24 +++++++++++++------ > libstdc++-v3/include/bits/cow_string.h | 17 +++++++++---- > .../basic_string/cons/char/119748.cc | 11 +++++++++ > .../basic_string/cons/wchar_t/119748.cc | 11 +++++++++ > 4 files changed, 51 insertions(+), 12 deletions(-) > create mode 100644 > libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc > create mode 100644 > libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc > > diff --git a/libstdc++-v3/include/bits/basic_string.h > b/libstdc++-v3/include/bits/basic_string.h > index 9c431c765ab..c90bd099b63 100644 > --- a/libstdc++-v3/include/bits/basic_string.h > +++ b/libstdc++-v3/include/bits/basic_string.h > @@ -488,8 +488,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 > is_same<_IterBase, const _CharT*>>::value) > _S_copy(__p, std::__niter_base(__k1), __k2 - __k1); > #if __cpp_lib_concepts > - else if constexpr (contiguous_iterator<_Iterator> > - && is_same_v<iter_value_t<_Iterator>, > _CharT>) > + else if constexpr (requires { > + requires contiguous_iterator<_Iterator>; > + { std::to_address(__k1) } > + -> convertible_to<const _CharT*>; > + }) > { > const auto __d = __k2 - __k1; > (void) (__k1 + __d); // See P3349R1 > @@ -499,7 +502,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 > else > #endif > for (; __k1 != __k2; ++__k1, (void)++__p) > - traits_type::assign(*__p, *__k1); // These types are off. > + traits_type::assign(*__p, static_cast<_CharT>(*__k1)); > } > #pragma GCC diagnostic pop > > @@ -527,12 +530,19 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 > static constexpr void > _S_copy_range(pointer __p, _Rg&& __rg, size_type __n) > { > - if constexpr (ranges::contiguous_range<_Rg> > - && is_same_v<ranges::range_value_t<_Rg>, _CharT>) > + if constexpr (requires { > + requires ranges::contiguous_range<_Rg>; > + { ranges::data(std::forward<_Rg>(__rg)) } > + -> convertible_to<const _CharT*>; > + }) > _S_copy(__p, ranges::data(std::forward<_Rg>(__rg)), __n); > else > - for (auto&& __e : __rg) > - traits_type::assign(*__p++, > std::forward<decltype(__e)>(__e)); > + { > + auto __first = ranges::begin(__rg); > + const auto __last = ranges::end(__rg); > + for (; __first != __last; ++__first) > + traits_type::assign(*__p++, static_cast<_CharT>(*__first)); > + } > } > #endif > > diff --git a/libstdc++-v3/include/bits/cow_string.h > b/libstdc++-v3/include/bits/cow_string.h > index b250397151b..f9df2be20be 100644 > --- a/libstdc++-v3/include/bits/cow_string.h > +++ b/libstdc++-v3/include/bits/cow_string.h > @@ -423,7 +423,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > _S_copy_chars(_CharT* __p, _Iterator __k1, _Iterator __k2) > { > for (; __k1 != __k2; ++__k1, (void)++__p) > - traits_type::assign(*__p, *__k1); // These types are off. > + traits_type::assign(*__p, static_cast<_CharT>(*__k1)); > } > > static void > @@ -656,12 +656,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > reserve(__n); > pointer __p = _M_data(); > - if constexpr (ranges::contiguous_range<_Rg> > - && is_same_v<ranges::range_value_t<_Rg>, > _CharT>) > + if constexpr (requires { > + requires ranges::contiguous_range<_Rg>; > + { ranges::data(std::forward<_Rg>(__rg)) } > + -> convertible_to<const _CharT*>; > + }) > _M_copy(__p, ranges::data(std::forward<_Rg>(__rg)), __n); > else > - for (auto&& __e : __rg) > - traits_type::assign(*__p++, > std::forward<decltype(__e)>(__e)); > + { > + auto __first = ranges::begin(__rg); > + const auto __last = ranges::end(__rg); > + for (; __first != __last; ++__first) > + traits_type::assign(*__p++, > static_cast<_CharT>(*__first)); > + } > _M_rep()->_M_set_length_and_sharable(__n); > } > else > diff --git > a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc > b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc > new file mode 100644 > index 00000000000..f0c0e1fbec3 > --- /dev/null > +++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc > @@ -0,0 +1,11 @@ > +// { dg-do compile } > + > +// Bug 119748 > +// string(InputIterator, InputIterator) rejects volatile charT* as > iterator > + > +#include <string> > +volatile char vs[42] = {}; > +std::string s(vs+0, vs+42); > +#ifdef __cpp_lib_containers_ranges > +std::string s2(std::from_range, vs); > +#endif > diff --git > a/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc > b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc > new file mode 100644 > index 00000000000..715c17588bc > --- /dev/null > +++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc > @@ -0,0 +1,11 @@ > +// { dg-do compile } > + > +// Bug 119748 > +// string(InputIterator, InputIterator) rejects volatile charT* as > iterator > + > +#include <string> > +volatile wchar_t vs[42] = {}; > +std::wstring s(vs+0, vs+42); > +#ifdef __cpp_lib_containers_ranges > +std::wstring s2(std::from_range, vs); > +#endif > -- > 2.49.0 > >
