On 11/04/25 09:49 +0200, Tomasz Kaminski wrote:
On Thu, Apr 10, 2025 at 6:29 PM Jonathan Wakely <jwak...@redhat.com> wrote:For C++11 and later we can remvoe four overloads of _S_copy_chars and use constexpr-if in the generic _S_copy_chars. This simplifies overload resolution for _S_copy_chars, and also means that we optimize for other iterators such as std::vector<char>::iterator. We still need all the _S_copy_chars overloads to be part of the explicit instantiation definition, so make them depend on the macro that is defined by src/c++11/string-inst.cc for that purpose. libstdc++-v3/ChangeLog: * include/bits/basic_string.h (_S_copy_chars): Replace overloads with constexpr-if and extend optimization to all contiguous iterators. * src/c++11/string-inst.cc: Extend comment. --- Tested x86_64-linux.LGTM with one clarification question.libstdc++-v3/include/bits/basic_string.h | 31 ++++++++++++++++++------ libstdc++-v3/src/c++11/string-inst.cc | 3 ++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 886e7e6b19e..067c7915c76 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -468,6 +468,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 traits_type::assign(__d, __n, __c); } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++17-extensions" // _S_copy_chars is a separate template to permit specialization // to optimize for the common case of pointers as iterators. template<class _Iterator> @@ -475,31 +477,44 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 static void _S_copy_chars(_CharT* __p, _Iterator __k1, _Iterator __k2) { +#if __cplusplus >= 201103L + using _IterBase = decltype(std::__niter_base(__k1)); + if constexpr (__or_<is_same<_IterBase, _CharT*>, + 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>) + { + const auto __d = __k2 - __k1; + (void) (__k1 + __d); // See P3349R1 + _S_copy(__p, std::to_address(__k1), static_cast<size_type>(__d)); + } +#endif + else +#endif for (; __k1 != __k2; ++__k1, (void)++__p) traits_type::assign(*__p, *__k1); // These types are off. } +#pragma GCC diagnostic pop - _GLIBCXX20_CONSTEXPR +#if __cplusplus < 201103L || defined _GLIBCXX_DEFINING_STRING_INSTANTIATIONS static void - _S_copy_chars(_CharT* __p, iterator __k1, iterator __k2) _GLIBCXX_NOEXCEPTThe `_GLIBCXX_DEFINING_STRING_INSTANTIATIONS` seem to be used for c++11/string-inst.cc, in which case the noexcept was meaningful.
Is it?
The macro is not used in c++17/string-inst.cc file, so noexcept
The src/c++17/string-inst.cc file only instantiates a handful of member functions, and none of them uses _S_copy_chars. So I don't think it matters for src/c++17/string-inst.cc at all.
would be part of the function signature, so I do not think we change emitted symbols. Is this the reason why did you remove it?
I removed it because those overloads of _S_copy_chars are only visible in the header for C++98, where _GLIBCXX_NOEXCEPT expands to nothing. You're correct that in src/c++11/string-inst.cc those overloads are instantiated for C++11 (using -std=gnu++11), where it would expand to 'noexcept'. But I don't think it's meaningful whether it's present or not. In C++11 the noexcept isn't part of the type, and it never affects the mangled name even in C++17. For these _S_copy_chars overloads, the compiler can easily see that they don't throw, as they only call std::char_traits<char>::assign or std::char_traits<char>::copy (or the wchar_t equivalents) and those are noexcept. So for the explicit instantiation definitions, the compiler knows they can't throw, and so it never needs to use a noexcept landing pad to terminate if an exception happens. So noexcept on the definition does nothing. Those functions can't throw, and we don't need (or want) extra code emitted to enforce the noexcept by terminating. So the noexcept is only meaningful at the call site, so that callers can see that those functions don't throw (and don't need their own exception handling code). The call sites in string-inst.cc should be able to see that they don't throw (they're compiled with -O2, and I checked on x86_64 and there are no calls to _S_copy_chars left in the assembly code). The call sites in user code can only see those overloads for C++98, where _GLIBCXX_NOEXCEPT expands to nothing. So I don't think the 'noexcept' is ever meaningful.
+ _S_copy_chars(_CharT* __p, iterator __k1, iterator __k2) { _S_copy_chars(__p, __k1.base(), __k2.base()); } - _GLIBCXX20_CONSTEXPR static void _S_copy_chars(_CharT* __p, const_iterator __k1, const_iterator __k2) - _GLIBCXX_NOEXCEPT { _S_copy_chars(__p, __k1.base(), __k2.base()); } - _GLIBCXX20_CONSTEXPR static void - _S_copy_chars(_CharT* __p, _CharT* __k1, _CharT* __k2) _GLIBCXX_NOEXCEPT + _S_copy_chars(_CharT* __p, _CharT* __k1, _CharT* __k2) { _S_copy(__p, __k1, __k2 - __k1); } - _GLIBCXX20_CONSTEXPR static void _S_copy_chars(_CharT* __p, const _CharT* __k1, const _CharT* __k2) - _GLIBCXX_NOEXCEPT { _S_copy(__p, __k1, __k2 - __k1); } +#endif _GLIBCXX20_CONSTEXPR static int diff --git a/libstdc++-v3/src/c++11/string-inst.cc b/libstdc++-v3/src/c++11/string-inst.cc index c4864794d40..34df909b31a 100644 --- a/libstdc++-v3/src/c++11/string-inst.cc +++ b/libstdc++-v3/src/c++11/string-inst.cc @@ -40,7 +40,8 @@ // replaced by constrained function templates, so that we instantiate the // pre-C++17 definitions. // This also causes the instantiation of the non-standard C++0x-era -// insert(iterator, initializer_list<C>) overload, see PR libstdc++/83328 +// insert(iterator, initializer_list<C>) overload, see PR libstdc++/83328, +// and overloads of _S_copy_chars for string iterators and pointers. #define _GLIBCXX_DEFINING_STRING_INSTANTIATIONS 1 #include <string> -- 2.49.0
