[PATCH] libstdc++: Use constexpr-if for std::basic_string::_S_copy_chars

[Tomasz Kaminski]

On Thu, Apr 10, 2025 at 6:29 PM Jonathan Wakely <jwakely@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_NOEXCEPT
>
The  `_GLIBCXX_DEFINING_STRING_INSTANTIATIONS` seem to be used for
c++11/string-inst.cc, in
which case the noexcept was meaningful. The macro is not used in
c++17/string-inst.cc file, so noexcept
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?

> +      _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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250411/2f851b64/attachment-0001.htm>