public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Refactor implementation of operator+ for std::string
@ 2022-09-05 18:30 Will Hawkins
  2022-09-08 17:50 ` François Dumont
  0 siblings, 1 reply; 8+ messages in thread
From: Will Hawkins @ 2022-09-05 18:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: libstdc++, Will Hawkins

Based on Jonathan's work, here is a patch for the implementation of operator+
on std::string that makes sure we always use the best allocation strategy.

I have attempted to learn from all the feedback that I got on a previous
submission -- I hope I did the right thing.

Passes abi and conformance testing on x86-64 trunk.

Sincerely,
Will

-- >8 --

Create a single function that performs one-allocation string concatenation
that can be used by various different version of operator+. 

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h:
	Add common function that performs single-allocation string
	concatenation. (__str_cat)
	Use __str_cat to perform optimized operator+, where relevant.
	* include/bits/basic_string.tcc::
	Remove single-allocation implementation of operator+.

Signed-off-by: Will Hawkins <whh8b@obs.cr>
---
 libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
 libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
 2 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 0df64ea98ca..4078651fadb 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 _GLIBCXX_END_NAMESPACE_CXX11
 #endif
 
+  template<typename _Str>
+    _GLIBCXX20_CONSTEXPR
+    inline _Str
+    __str_concat(typename _Str::value_type const* __lhs,
+		 typename _Str::size_type __lhs_len,
+		 typename _Str::value_type const* __rhs,
+		 typename _Str::size_type __rhs_len,
+		 typename _Str::allocator_type const& __a)
+    {
+      typedef typename _Str::allocator_type allocator_type;
+      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
+      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
+      __str.reserve(__lhs_len + __rhs_len);
+      __str.append(__lhs, __lhs_len);
+      __str.append(__rhs, __rhs_len);
+      return __str;
+    }
+
   // operator+
   /**
    *  @brief  Concatenate two strings.
@@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
+    inline basic_string<_CharT, _Traits, _Alloc>
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
 	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
     {
-      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-      __str.append(__rhs);
-      return __str;
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __rhs.c_str(), __rhs.size(),
+				     __lhs.get_allocator());
     }
 
   /**
@@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT,_Traits,_Alloc>
+    inline basic_string<_CharT,_Traits,_Alloc>
     operator+(const _CharT* __lhs,
-	      const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+	      const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+    {
+      __glibcxx_requires_string(__lhs);
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
+				     __rhs.c_str(), __rhs.size(),
+				     __rhs.get_allocator());
+    }
 
   /**
    *  @brief  Concatenate character and string.
@@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT,_Traits,_Alloc>
-    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+    inline basic_string<_CharT,_Traits,_Alloc>
+    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+    {
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
+				     __rhs.c_str(), __rhs.size(),
+				     __rhs.get_allocator());
+    }
 
   /**
    *  @brief  Concatenate string and C string.
@@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
 	      const _CharT* __rhs)
     {
-      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-      __str.append(__rhs);
-      return __str;
+      __glibcxx_requires_string(__rhs);
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __rhs, _Traits::length(__rhs),
+				     __lhs.get_allocator());
     }
-
   /**
    *  @brief  Concatenate string and character.
    *  @param __lhs  First string.
@@ -3550,11 +3583,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
     inline basic_string<_CharT, _Traits, _Alloc>
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
     {
-      typedef basic_string<_CharT, _Traits, _Alloc>	__string_type;
-      typedef typename __string_type::size_type		__size_type;
-      __string_type __str(__lhs);
-      __str.append(__size_type(1), __rhs);
-      return __str;
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __builtin_addressof(__rhs), 1,
+				     __lhs.get_allocator());
     }
 
 #if __cplusplus >= 201103L
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 4563c61429a..07a94d36757 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -599,47 +599,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
 # define _GLIBCXX_STRING_CONSTEXPR
 #endif
-
-  template<typename _CharT, typename _Traits, typename _Alloc>
-    _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
-    operator+(const _CharT* __lhs,
-	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
-    {
-      __glibcxx_requires_string(__lhs);
-      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
-      typedef typename __string_type::size_type	  __size_type;
-      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
-	rebind<_CharT>::other _Char_alloc_type;
-      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
-      const __size_type __len = _Traits::length(__lhs);
-      __string_type __str(_Alloc_traits::_S_select_on_copy(
-          __rhs.get_allocator()));
-      __str.reserve(__len + __rhs.size());
-      __str.append(__lhs, __len);
-      __str.append(__rhs);
-      return __str;
-    }
-
-  template<typename _CharT, typename _Traits, typename _Alloc>
-    _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
-    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
-    {
-      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
-      typedef typename __string_type::size_type	  __size_type;
-      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
-	rebind<_CharT>::other _Char_alloc_type;
-      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
-      __string_type __str(_Alloc_traits::_S_select_on_copy(
-          __rhs.get_allocator()));
-      const __size_type __len = __rhs.size();
-      __str.reserve(__len + 1);
-      __str.append(__size_type(1), __lhs);
-      __str.append(__rhs);
-      return __str;
-    }
-
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_STRING_CONSTEXPR
     typename basic_string<_CharT, _Traits, _Alloc>::size_type
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-09-05 18:30 [PATCH] libstdc++: Refactor implementation of operator+ for std::string Will Hawkins
@ 2022-09-08 17:50 ` François Dumont
  2022-09-08 18:05   ` Jonathan Wakely
  2022-09-09 20:28   ` Jonathan Wakely
  0 siblings, 2 replies; 8+ messages in thread
From: François Dumont @ 2022-09-08 17:50 UTC (permalink / raw)
  To: Will Hawkins, gcc-patches; +Cc: libstdc++

On 05/09/22 20:30, Will Hawkins wrote:
> Based on Jonathan's work, here is a patch for the implementation of operator+
> on std::string that makes sure we always use the best allocation strategy.
>
> I have attempted to learn from all the feedback that I got on a previous
> submission -- I hope I did the right thing.
>
> Passes abi and conformance testing on x86-64 trunk.
>
> Sincerely,
> Will
>
> -- >8 --
>
> Create a single function that performs one-allocation string concatenation
> that can be used by various different version of operator+.
>
> libstdc++-v3/ChangeLog:
>
> 	* include/bits/basic_string.h:
> 	Add common function that performs single-allocation string
> 	concatenation. (__str_cat)
> 	Use __str_cat to perform optimized operator+, where relevant.
> 	* include/bits/basic_string.tcc::
> 	Remove single-allocation implementation of operator+.
>
> Signed-off-by: Will Hawkins <whh8b@obs.cr>
> ---
>   libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
>   libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
>   2 files changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index 0df64ea98ca..4078651fadb 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>   _GLIBCXX_END_NAMESPACE_CXX11
>   #endif
>   
> +  template<typename _Str>
> +    _GLIBCXX20_CONSTEXPR
> +    inline _Str
> +    __str_concat(typename _Str::value_type const* __lhs,
> +		 typename _Str::size_type __lhs_len,
> +		 typename _Str::value_type const* __rhs,
> +		 typename _Str::size_type __rhs_len,
> +		 typename _Str::allocator_type const& __a)
> +    {
> +      typedef typename _Str::allocator_type allocator_type;
> +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> +      __str.reserve(__lhs_len + __rhs_len);
> +      __str.append(__lhs, __lhs_len);
> +      __str.append(__rhs, __rhs_len);
> +      return __str;
> +    }
> +
>     // operator+
>     /**
>      *  @brief  Concatenate two strings.
> @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      */
>     template<typename _CharT, typename _Traits, typename _Alloc>
>       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> +    inline basic_string<_CharT, _Traits, _Alloc>
>       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>   	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>       {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +				     __rhs.c_str(), __rhs.size(),

You should use data() rather than c_str() here and all other operators.

It is currently the same but is more accurate in your context. Maybe one 
day it will make a difference.

> +				     __lhs.get_allocator());
>       }
>   
>     /**
> @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      */
>     template<typename _CharT, typename _Traits, typename _Alloc>
>       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> +    inline basic_string<_CharT,_Traits,_Alloc>

Why inlining ?

I guess it is done this way to limit code bloat.

>       operator+(const _CharT* __lhs,
> -	      const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +	      const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      __glibcxx_requires_string(__lhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> +				     __rhs.c_str(), __rhs.size(),
> +				     __rhs.get_allocator());
> +    }
>   
>     /**
>      *  @brief  Concatenate character and string.
> @@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      */
>     template<typename _CharT, typename _Traits, typename _Alloc>
>       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +    inline basic_string<_CharT,_Traits,_Alloc>
> +    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> +				     __rhs.c_str(), __rhs.size(),
> +				     __rhs.get_allocator());
> +    }
>   
>     /**
>      *  @brief  Concatenate string and C string.
> @@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
>       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>   	      const _CharT* __rhs)
>       {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      __glibcxx_requires_string(__rhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +				     __rhs, _Traits::length(__rhs),
> +				     __lhs.get_allocator());
>       }
> -
>     /**
>      *  @brief  Concatenate string and character.
>      *  @param __lhs  First string.
> @@ -3550,11 +3583,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
>       inline basic_string<_CharT, _Traits, _Alloc>
>       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
>       {
> -      typedef basic_string<_CharT, _Traits, _Alloc>	__string_type;
> -      typedef typename __string_type::size_type		__size_type;
> -      __string_type __str(__lhs);
> -      __str.append(__size_type(1), __rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +				     __builtin_addressof(__rhs), 1,
> +				     __lhs.get_allocator());
>       }
>   
>   #if __cplusplus >= 201103L
> diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
> index 4563c61429a..07a94d36757 100644
> --- a/libstdc++-v3/include/bits/basic_string.tcc
> +++ b/libstdc++-v3/include/bits/basic_string.tcc
> @@ -599,47 +599,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   #else
>   # define _GLIBCXX_STRING_CONSTEXPR
>   #endif
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(const _CharT* __lhs,
> -	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      __glibcxx_requires_string(__lhs);
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type	  __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -	rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      const __size_type __len = _Traits::length(__lhs);
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      __str.reserve(__len + __rhs.size());
> -      __str.append(__lhs, __len);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type	  __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -	rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      const __size_type __len = __rhs.size();
> -      __str.reserve(__len + 1);
> -      __str.append(__size_type(1), __lhs);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
>     template<typename _CharT, typename _Traits, typename _Alloc>
>       _GLIBCXX_STRING_CONSTEXPR
>       typename basic_string<_CharT, _Traits, _Alloc>::size_type



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-09-08 17:50 ` François Dumont
@ 2022-09-08 18:05   ` Jonathan Wakely
  2022-09-09 13:53     ` Will Hawkins
  2022-09-09 20:28   ` Jonathan Wakely
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2022-09-08 18:05 UTC (permalink / raw)
  To: François Dumont; +Cc: Will Hawkins, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 9642 bytes --]

On Thu, 8 Sep 2022, 18:51 François Dumont via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> On 05/09/22 20:30, Will Hawkins wrote:
> > Based on Jonathan's work, here is a patch for the implementation of
> operator+
> > on std::string that makes sure we always use the best allocation
> strategy.
> >
> > I have attempted to learn from all the feedback that I got on a previous
> > submission -- I hope I did the right thing.
> >
> > Passes abi and conformance testing on x86-64 trunk.
> >
> > Sincerely,
> > Will
> >
> > -- >8 --
> >
> > Create a single function that performs one-allocation string
> concatenation
> > that can be used by various different version of operator+.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       * include/bits/basic_string.h:
> >       Add common function that performs single-allocation string
> >       concatenation. (__str_cat)
> >       Use __str_cat to perform optimized operator+, where relevant.
> >       * include/bits/basic_string.tcc::
> >       Remove single-allocation implementation of operator+.
> >
> > Signed-off-by: Will Hawkins <whh8b@obs.cr>
> > ---
> >   libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
> >   libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
> >   2 files changed, 49 insertions(+), 58 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> > index 0df64ea98ca..4078651fadb 100644
> > --- a/libstdc++-v3/include/bits/basic_string.h
> > +++ b/libstdc++-v3/include/bits/basic_string.h
> > @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >   _GLIBCXX_END_NAMESPACE_CXX11
> >   #endif
> >
> > +  template<typename _Str>
> > +    _GLIBCXX20_CONSTEXPR
> > +    inline _Str
> > +    __str_concat(typename _Str::value_type const* __lhs,
> > +              typename _Str::size_type __lhs_len,
> > +              typename _Str::value_type const* __rhs,
> > +              typename _Str::size_type __rhs_len,
> > +              typename _Str::allocator_type const& __a)
> > +    {
> > +      typedef typename _Str::allocator_type allocator_type;
> > +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> > +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> > +      __str.reserve(__lhs_len + __rhs_len);
> > +      __str.append(__lhs, __lhs_len);
> > +      __str.append(__rhs, __rhs_len);
> > +      return __str;
> > +    }
> > +
> >     // operator+
> >     /**
> >      *  @brief  Concatenate two strings.
> > @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >      */
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT, _Traits, _Alloc>
> > +    inline basic_string<_CharT, _Traits, _Alloc>
> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> >             const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> >       {
> > -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> > -      __str.append(__rhs);
> > -      return __str;
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> > +                                  __rhs.c_str(), __rhs.size(),
>
> You should use data() rather than c_str() here and all other operators.
>
> It is currently the same but is more accurate in your context. Maybe one
> day it will make a difference.
>


I don't think so, that would be a major breaking change, for no benefit. I
think it's safe to assume they will always stay equivalent now.



> > +                                  __lhs.get_allocator());
> >       }
> >
> >     /**
> > @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >      */
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT,_Traits,_Alloc>
> > +    inline basic_string<_CharT,_Traits,_Alloc>
>
> Why inlining ?
>
> I guess it is done this way to limit code bloat.
>
> >       operator+(const _CharT* __lhs,
> > -           const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> > +           const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> > +    {
> > +      __glibcxx_requires_string(__lhs);
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> > +                                  __rhs.c_str(), __rhs.size(),
> > +                                  __rhs.get_allocator());
> > +    }
> >
> >     /**
> >      *  @brief  Concatenate character and string.
> > @@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >      */
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT,_Traits,_Alloc>
> > -    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>&
> __rhs);
> > +    inline basic_string<_CharT,_Traits,_Alloc>
> > +    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>&
> __rhs)
> > +    {
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> > +                                  __rhs.c_str(), __rhs.size(),
> > +                                  __rhs.get_allocator());
> > +    }
> >
> >     /**
> >      *  @brief  Concatenate string and C string.
> > @@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> >             const _CharT* __rhs)
> >       {
> > -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> > -      __str.append(__rhs);
> > -      return __str;
> > +      __glibcxx_requires_string(__rhs);
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> > +                                  __rhs, _Traits::length(__rhs),
> > +                                  __lhs.get_allocator());
> >       }
> > -
> >     /**
> >      *  @brief  Concatenate string and character.
> >      *  @param __lhs  First string.
> > @@ -3550,11 +3583,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >       inline basic_string<_CharT, _Traits, _Alloc>
> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> _CharT __rhs)
> >       {
> > -      typedef basic_string<_CharT, _Traits, _Alloc>  __string_type;
> > -      typedef typename __string_type::size_type
> __size_type;
> > -      __string_type __str(__lhs);
> > -      __str.append(__size_type(1), __rhs);
> > -      return __str;
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> > +                                  __builtin_addressof(__rhs), 1,
> > +                                  __lhs.get_allocator());
> >       }
> >
> >   #if __cplusplus >= 201103L
> > diff --git a/libstdc++-v3/include/bits/basic_string.tcc
> b/libstdc++-v3/include/bits/basic_string.tcc
> > index 4563c61429a..07a94d36757 100644
> > --- a/libstdc++-v3/include/bits/basic_string.tcc
> > +++ b/libstdc++-v3/include/bits/basic_string.tcc
> > @@ -599,47 +599,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >   #else
> >   # define _GLIBCXX_STRING_CONSTEXPR
> >   #endif
> > -
> > -  template<typename _CharT, typename _Traits, typename _Alloc>
> > -    _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT, _Traits, _Alloc>
> > -    operator+(const _CharT* __lhs,
> > -           const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> > -    {
> > -      __glibcxx_requires_string(__lhs);
> > -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> > -      typedef typename __string_type::size_type        __size_type;
> > -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> > -     rebind<_CharT>::other _Char_alloc_type;
> > -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> > -      const __size_type __len = _Traits::length(__lhs);
> > -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> > -          __rhs.get_allocator()));
> > -      __str.reserve(__len + __rhs.size());
> > -      __str.append(__lhs, __len);
> > -      __str.append(__rhs);
> > -      return __str;
> > -    }
> > -
> > -  template<typename _CharT, typename _Traits, typename _Alloc>
> > -    _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT, _Traits, _Alloc>
> > -    operator+(_CharT __lhs, const basic_string<_CharT, _Traits,
> _Alloc>& __rhs)
> > -    {
> > -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> > -      typedef typename __string_type::size_type        __size_type;
> > -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> > -     rebind<_CharT>::other _Char_alloc_type;
> > -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> > -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> > -          __rhs.get_allocator()));
> > -      const __size_type __len = __rhs.size();
> > -      __str.reserve(__len + 1);
> > -      __str.append(__size_type(1), __lhs);
> > -      __str.append(__rhs);
> > -      return __str;
> > -    }
> > -
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_STRING_CONSTEXPR
> >       typename basic_string<_CharT, _Traits, _Alloc>::size_type
>
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-09-08 18:05   ` Jonathan Wakely
@ 2022-09-09 13:53     ` Will Hawkins
  0 siblings, 0 replies; 8+ messages in thread
From: Will Hawkins @ 2022-09-09 13:53 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: François Dumont, gcc-patches, libstdc++

On Thu, Sep 8, 2022 at 2:05 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
>
>
> On Thu, 8 Sep 2022, 18:51 François Dumont via Libstdc++, <libstdc++@gcc.gnu.org> wrote:
>>
>> On 05/09/22 20:30, Will Hawkins wrote:
>> > Based on Jonathan's work, here is a patch for the implementation of operator+
>> > on std::string that makes sure we always use the best allocation strategy.
>> >
>> > I have attempted to learn from all the feedback that I got on a previous
>> > submission -- I hope I did the right thing.
>> >
>> > Passes abi and conformance testing on x86-64 trunk.
>> >
>> > Sincerely,
>> > Will
>> >
>> > -- >8 --
>> >
>> > Create a single function that performs one-allocation string concatenation
>> > that can be used by various different version of operator+.
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> >       * include/bits/basic_string.h:
>> >       Add common function that performs single-allocation string
>> >       concatenation. (__str_cat)
>> >       Use __str_cat to perform optimized operator+, where relevant.
>> >       * include/bits/basic_string.tcc::
>> >       Remove single-allocation implementation of operator+.
>> >
>> > Signed-off-by: Will Hawkins <whh8b@obs.cr>
>> > ---
>> >   libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
>> >   libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
>> >   2 files changed, 49 insertions(+), 58 deletions(-)
>> >
>> > diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
>> > index 0df64ea98ca..4078651fadb 100644
>> > --- a/libstdc++-v3/include/bits/basic_string.h
>> > +++ b/libstdc++-v3/include/bits/basic_string.h
>> > @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>> >   _GLIBCXX_END_NAMESPACE_CXX11
>> >   #endif
>> >
>> > +  template<typename _Str>
>> > +    _GLIBCXX20_CONSTEXPR
>> > +    inline _Str
>> > +    __str_concat(typename _Str::value_type const* __lhs,
>> > +              typename _Str::size_type __lhs_len,
>> > +              typename _Str::value_type const* __rhs,
>> > +              typename _Str::size_type __rhs_len,
>> > +              typename _Str::allocator_type const& __a)
>> > +    {
>> > +      typedef typename _Str::allocator_type allocator_type;
>> > +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
>> > +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
>> > +      __str.reserve(__lhs_len + __rhs_len);
>> > +      __str.append(__lhs, __lhs_len);
>> > +      __str.append(__rhs, __rhs_len);
>> > +      return __str;
>> > +    }
>> > +
>> >     // operator+
>> >     /**
>> >      *  @brief  Concatenate two strings.
>> > @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> >      */
>> >     template<typename _CharT, typename _Traits, typename _Alloc>
>> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>> > -    basic_string<_CharT, _Traits, _Alloc>
>> > +    inline basic_string<_CharT, _Traits, _Alloc>
>> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>> >             const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>> >       {
>> > -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
>> > -      __str.append(__rhs);
>> > -      return __str;
>> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
>> > +                                  __rhs.c_str(), __rhs.size(),
>>
>> You should use data() rather than c_str() here and all other operators.
>>
>> It is currently the same but is more accurate in your context. Maybe one
>> day it will make a difference.
>
>
>
> I don't think so, that would be a major breaking change, for no benefit. I think it's safe to assume they will always stay equivalent now.

Happy to make any changes to the patch that the group thinks are necessary!

Will


>
>
>>
>> > +                                  __lhs.get_allocator());
>> >       }
>> >
>> >     /**
>> > @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> >      */
>> >     template<typename _CharT, typename _Traits, typename _Alloc>
>> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>> > -    basic_string<_CharT,_Traits,_Alloc>
>> > +    inline basic_string<_CharT,_Traits,_Alloc>
>>
>> Why inlining ?
>>
>> I guess it is done this way to limit code bloat.
>>
>> >       operator+(const _CharT* __lhs,
>> > -           const basic_string<_CharT,_Traits,_Alloc>& __rhs);
>> > +           const basic_string<_CharT,_Traits,_Alloc>& __rhs)
>> > +    {
>> > +      __glibcxx_requires_string(__lhs);
>> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > +      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
>> > +                                  __rhs.c_str(), __rhs.size(),
>> > +                                  __rhs.get_allocator());
>> > +    }
>> >
>> >     /**
>> >      *  @brief  Concatenate character and string.
>> > @@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> >      */
>> >     template<typename _CharT, typename _Traits, typename _Alloc>
>> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>> > -    basic_string<_CharT,_Traits,_Alloc>
>> > -    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
>> > +    inline basic_string<_CharT,_Traits,_Alloc>
>> > +    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
>> > +    {
>> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > +      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
>> > +                                  __rhs.c_str(), __rhs.size(),
>> > +                                  __rhs.get_allocator());
>> > +    }
>> >
>> >     /**
>> >      *  @brief  Concatenate string and C string.
>> > @@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>> >             const _CharT* __rhs)
>> >       {
>> > -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
>> > -      __str.append(__rhs);
>> > -      return __str;
>> > +      __glibcxx_requires_string(__rhs);
>> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
>> > +                                  __rhs, _Traits::length(__rhs),
>> > +                                  __lhs.get_allocator());
>> >       }
>> > -
>> >     /**
>> >      *  @brief  Concatenate string and character.
>> >      *  @param __lhs  First string.
>> > @@ -3550,11 +3583,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> >       inline basic_string<_CharT, _Traits, _Alloc>
>> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
>> >       {
>> > -      typedef basic_string<_CharT, _Traits, _Alloc>  __string_type;
>> > -      typedef typename __string_type::size_type              __size_type;
>> > -      __string_type __str(__lhs);
>> > -      __str.append(__size_type(1), __rhs);
>> > -      return __str;
>> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
>> > +                                  __builtin_addressof(__rhs), 1,
>> > +                                  __lhs.get_allocator());
>> >       }
>> >
>> >   #if __cplusplus >= 201103L
>> > diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
>> > index 4563c61429a..07a94d36757 100644
>> > --- a/libstdc++-v3/include/bits/basic_string.tcc
>> > +++ b/libstdc++-v3/include/bits/basic_string.tcc
>> > @@ -599,47 +599,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >   #else
>> >   # define _GLIBCXX_STRING_CONSTEXPR
>> >   #endif
>> > -
>> > -  template<typename _CharT, typename _Traits, typename _Alloc>
>> > -    _GLIBCXX20_CONSTEXPR
>> > -    basic_string<_CharT, _Traits, _Alloc>
>> > -    operator+(const _CharT* __lhs,
>> > -           const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>> > -    {
>> > -      __glibcxx_requires_string(__lhs);
>> > -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
>> > -      typedef typename __string_type::size_type        __size_type;
>> > -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
>> > -     rebind<_CharT>::other _Char_alloc_type;
>> > -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
>> > -      const __size_type __len = _Traits::length(__lhs);
>> > -      __string_type __str(_Alloc_traits::_S_select_on_copy(
>> > -          __rhs.get_allocator()));
>> > -      __str.reserve(__len + __rhs.size());
>> > -      __str.append(__lhs, __len);
>> > -      __str.append(__rhs);
>> > -      return __str;
>> > -    }
>> > -
>> > -  template<typename _CharT, typename _Traits, typename _Alloc>
>> > -    _GLIBCXX20_CONSTEXPR
>> > -    basic_string<_CharT, _Traits, _Alloc>
>> > -    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>> > -    {
>> > -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
>> > -      typedef typename __string_type::size_type        __size_type;
>> > -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
>> > -     rebind<_CharT>::other _Char_alloc_type;
>> > -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
>> > -      __string_type __str(_Alloc_traits::_S_select_on_copy(
>> > -          __rhs.get_allocator()));
>> > -      const __size_type __len = __rhs.size();
>> > -      __str.reserve(__len + 1);
>> > -      __str.append(__size_type(1), __lhs);
>> > -      __str.append(__rhs);
>> > -      return __str;
>> > -    }
>> > -
>> >     template<typename _CharT, typename _Traits, typename _Alloc>
>> >       _GLIBCXX_STRING_CONSTEXPR
>> >       typename basic_string<_CharT, _Traits, _Alloc>::size_type
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-09-08 17:50 ` François Dumont
  2022-09-08 18:05   ` Jonathan Wakely
@ 2022-09-09 20:28   ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2022-09-09 20:28 UTC (permalink / raw)
  To: François Dumont; +Cc: Will Hawkins, gcc-patches, libstdc++

On Thu, 8 Sept 2022 at 18:51, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On 05/09/22 20:30, Will Hawkins wrote:
> > Based on Jonathan's work, here is a patch for the implementation of operator+
> > on std::string that makes sure we always use the best allocation strategy.
> >
> > I have attempted to learn from all the feedback that I got on a previous
> > submission -- I hope I did the right thing.
> >
> > Passes abi and conformance testing on x86-64 trunk.
> >
> > Sincerely,
> > Will
> >
> > -- >8 --
> >
> > Create a single function that performs one-allocation string concatenation
> > that can be used by various different version of operator+.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       * include/bits/basic_string.h:
> >       Add common function that performs single-allocation string
> >       concatenation. (__str_cat)
> >       Use __str_cat to perform optimized operator+, where relevant.
> >       * include/bits/basic_string.tcc::
> >       Remove single-allocation implementation of operator+.
> >
> > Signed-off-by: Will Hawkins <whh8b@obs.cr>
> > ---
> >   libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
> >   libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
> >   2 files changed, 49 insertions(+), 58 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> > index 0df64ea98ca..4078651fadb 100644
> > --- a/libstdc++-v3/include/bits/basic_string.h
> > +++ b/libstdc++-v3/include/bits/basic_string.h
> > @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >   _GLIBCXX_END_NAMESPACE_CXX11
> >   #endif
> >
> > +  template<typename _Str>
> > +    _GLIBCXX20_CONSTEXPR
> > +    inline _Str
> > +    __str_concat(typename _Str::value_type const* __lhs,
> > +              typename _Str::size_type __lhs_len,
> > +              typename _Str::value_type const* __rhs,
> > +              typename _Str::size_type __rhs_len,
> > +              typename _Str::allocator_type const& __a)
> > +    {
> > +      typedef typename _Str::allocator_type allocator_type;
> > +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> > +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> > +      __str.reserve(__lhs_len + __rhs_len);
> > +      __str.append(__lhs, __lhs_len);
> > +      __str.append(__rhs, __rhs_len);
> > +      return __str;
> > +    }
> > +
> >     // operator+
> >     /**
> >      *  @brief  Concatenate two strings.
> > @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >      */
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT, _Traits, _Alloc>
> > +    inline basic_string<_CharT, _Traits, _Alloc>
> >       operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> >             const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> >       {
> > -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> > -      __str.append(__rhs);
> > -      return __str;
> > +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> > +                                  __rhs.c_str(), __rhs.size(),
>
> You should use data() rather than c_str() here and all other operators.
>
> It is currently the same but is more accurate in your context. Maybe one
> day it will make a difference.

As I said, it will never make a difference, so there's no technical
reason to change it. I suppose data() is a little more expressive
here, in that we only care about the characters, not the null
terminator that c_str() implies (even though data() has the null
terminator too, as it's the same pointer returned).

>
> > +                                  __lhs.get_allocator());
> >       }
> >
> >     /**
> > @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >      */
> >     template<typename _CharT, typename _Traits, typename _Alloc>
> >       _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -    basic_string<_CharT,_Traits,_Alloc>
> > +    inline basic_string<_CharT,_Traits,_Alloc>
>
> Why inlining ?

Because it's a one line function that just calls another function.
That's an ideal candidate for being inline.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-10-20  0:05 Will Hawkins
  2022-11-02 20:25 ` Will Hawkins
@ 2022-11-08 17:44 ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2022-11-08 17:44 UTC (permalink / raw)
  To: Will Hawkins; +Cc: gcc-patches, libstdc++

On Thu, 20 Oct 2022 at 01:06, Will Hawkins wrote:
>
> Sorry for the delay. Tested on x86-64 Linux.
>
> -->8--
>
> After consultation with Jonathan, it seemed like a good idea to create a
> single function that performed one-allocation string concatenation that
> could be used by various different version of operator+. This patch adds
> such a function and calls it from the relevant implementations.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/basic_string.h:
>         Add common function that performs single-allocation string
>         concatenation. (__str_cat)
>         Use __str_cat to perform optimized operator+, where relevant.
>         * include/bits/basic_string.tcc::
>         Remove single-allocation implementation of operator+.
>
> Signed-off-by: Will Hawkins <whh8b@obs.cr>

I've pushed this patch to trunk now. I changed the commit message
significantly though:

   libstdc++: Refactor implementation of operator+ for std::string

   Until now operator+(char*, string) and operator+(string, char*) had
   different performance characteristics. The former required a single
   memory allocation and the latter required two. This patch makes the
   performance equal.

   After consultation with Jonathan, it seemed like a good idea to create a
   single function that performed one-allocation string concatenation that
   could be used by various different version of operator+. This patch adds
   such a function and calls it from the relevant implementations.

   Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

   libstdc++-v3/ChangeLog:

           * include/bits/basic_string.h (__str_cat): Add common function
           that performs single-allocation string concatenation.
           (operator+): Use __str_cat.
           * include/bits/basic_string.tcc (operator+): Move to .h and
           define inline using __str_cat.

   Signed-off-by: Will Hawkins <whh8b@obs.cr>

Specifically, I restored part of your earlier commit's message, which
gives the necessary context for the commit. Just starting with "After
consultation with Jonathan, ..." doesn't say anything about the patch
itself and is not very helpful without the earlier context from the
mailing list.

I added myself as Co-author, since the new patch was largely based on
a patch I sent in a private email.

And I changed the changelog part to better meet the format of GNU ChangeLogs.
https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html

The change is on trunk now (and I didn't see any libgomp test failures
this time).






> ---
>  libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
>  libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
>  2 files changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index cd244191df4..9c2b57f5a1d 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>  _GLIBCXX_END_NAMESPACE_CXX11
>  #endif
>
> +  template<typename _Str>
> +    _GLIBCXX20_CONSTEXPR
> +    inline _Str
> +    __str_concat(typename _Str::value_type const* __lhs,
> +                typename _Str::size_type __lhs_len,
> +                typename _Str::value_type const* __rhs,
> +                typename _Str::size_type __rhs_len,
> +                typename _Str::allocator_type const& __a)
> +    {
> +      typedef typename _Str::allocator_type allocator_type;
> +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> +      __str.reserve(__lhs_len + __rhs_len);
> +      __str.append(__lhs, __lhs_len);
> +      __str.append(__rhs, __rhs_len);
> +      return __str;
> +    }
> +
>    // operator+
>    /**
>     *  @brief  Concatenate two strings.
> @@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> +    inline basic_string<_CharT, _Traits, _Alloc>
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>               const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>      {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __lhs.get_allocator());
>      }
>
>    /**
> @@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> +    inline basic_string<_CharT,_Traits,_Alloc>
>      operator+(const _CharT* __lhs,
> -             const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +             const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      __glibcxx_requires_string(__lhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __rhs.get_allocator());
> +    }
>
>    /**
>     *  @brief  Concatenate character and string.
> @@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +    inline basic_string<_CharT,_Traits,_Alloc>
> +    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __rhs.get_allocator());
> +    }
>
>    /**
>     *  @brief  Concatenate string and C string.
> @@ -3538,11 +3570,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>               const _CharT* __rhs)
>      {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      __glibcxx_requires_string(__rhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __rhs, _Traits::length(__rhs),
> +                                    __lhs.get_allocator());
>      }
> -
>    /**
>     *  @brief  Concatenate string and character.
>     *  @param __lhs  First string.
> @@ -3554,11 +3587,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      inline basic_string<_CharT, _Traits, _Alloc>
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
>      {
> -      typedef basic_string<_CharT, _Traits, _Alloc>    __string_type;
> -      typedef typename __string_type::size_type                __size_type;
> -      __string_type __str(__lhs);
> -      __str.append(__size_type(1), __rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __builtin_addressof(__rhs), 1,
> +                                    __lhs.get_allocator());
>      }
>
>  #if __cplusplus >= 201103L
> diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
> index 710c2dfe030..56114f120ba 100644
> --- a/libstdc++-v3/include/bits/basic_string.tcc
> +++ b/libstdc++-v3/include/bits/basic_string.tcc
> @@ -605,47 +605,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  #else
>  # define _GLIBCXX_STRING_CONSTEXPR
>  #endif
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(const _CharT* __lhs,
> -             const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      __glibcxx_requires_string(__lhs);
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type          __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -       rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      const __size_type __len = _Traits::length(__lhs);
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      __str.reserve(__len + __rhs.size());
> -      __str.append(__lhs, __len);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type          __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -       rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      const __size_type __len = __rhs.size();
> -      __str.reserve(__len + 1);
> -      __str.append(__size_type(1), __lhs);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_STRING_CONSTEXPR
>      typename basic_string<_CharT, _Traits, _Alloc>::size_type
> --
> 2.37.3
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
  2022-10-20  0:05 Will Hawkins
@ 2022-11-02 20:25 ` Will Hawkins
  2022-11-08 17:44 ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Will Hawkins @ 2022-11-02 20:25 UTC (permalink / raw)
  To: gcc-patches, libstdc++

Just wanted to see if there was anything else I can do to help move
this over the finish line! Thanks for all the work that you all do!

Sincerely,
Will

On Wed, Oct 19, 2022 at 8:06 PM Will Hawkins <whh8b@obs.cr> wrote:
>
> Sorry for the delay. Tested on x86-64 Linux.
>
> -->8--
>
> After consultation with Jonathan, it seemed like a good idea to create a
> single function that performed one-allocation string concatenation that
> could be used by various different version of operator+. This patch adds
> such a function and calls it from the relevant implementations.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/basic_string.h:
>         Add common function that performs single-allocation string
>         concatenation. (__str_cat)
>         Use __str_cat to perform optimized operator+, where relevant.
>         * include/bits/basic_string.tcc::
>         Remove single-allocation implementation of operator+.
>
> Signed-off-by: Will Hawkins <whh8b@obs.cr>
> ---
>  libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
>  libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
>  2 files changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index cd244191df4..9c2b57f5a1d 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>  _GLIBCXX_END_NAMESPACE_CXX11
>  #endif
>
> +  template<typename _Str>
> +    _GLIBCXX20_CONSTEXPR
> +    inline _Str
> +    __str_concat(typename _Str::value_type const* __lhs,
> +                typename _Str::size_type __lhs_len,
> +                typename _Str::value_type const* __rhs,
> +                typename _Str::size_type __rhs_len,
> +                typename _Str::allocator_type const& __a)
> +    {
> +      typedef typename _Str::allocator_type allocator_type;
> +      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
> +      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> +      __str.reserve(__lhs_len + __rhs_len);
> +      __str.append(__lhs, __lhs_len);
> +      __str.append(__rhs, __rhs_len);
> +      return __str;
> +    }
> +
>    // operator+
>    /**
>     *  @brief  Concatenate two strings.
> @@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> +    inline basic_string<_CharT, _Traits, _Alloc>
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>               const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>      {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __lhs.get_allocator());
>      }
>
>    /**
> @@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> +    inline basic_string<_CharT,_Traits,_Alloc>
>      operator+(const _CharT* __lhs,
> -             const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +             const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      __glibcxx_requires_string(__lhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __rhs.get_allocator());
> +    }
>
>    /**
>     *  @brief  Concatenate character and string.
> @@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>     */
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT,_Traits,_Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> +    inline basic_string<_CharT,_Traits,_Alloc>
> +    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +    {
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> +                                    __rhs.c_str(), __rhs.size(),
> +                                    __rhs.get_allocator());
> +    }
>
>    /**
>     *  @brief  Concatenate string and C string.
> @@ -3538,11 +3570,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>               const _CharT* __rhs)
>      {
> -      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> -      __str.append(__rhs);
> -      return __str;
> +      __glibcxx_requires_string(__rhs);
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __rhs, _Traits::length(__rhs),
> +                                    __lhs.get_allocator());
>      }
> -
>    /**
>     *  @brief  Concatenate string and character.
>     *  @param __lhs  First string.
> @@ -3554,11 +3587,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
>      inline basic_string<_CharT, _Traits, _Alloc>
>      operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
>      {
> -      typedef basic_string<_CharT, _Traits, _Alloc>    __string_type;
> -      typedef typename __string_type::size_type                __size_type;
> -      __string_type __str(__lhs);
> -      __str.append(__size_type(1), __rhs);
> -      return __str;
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +                                    __builtin_addressof(__rhs), 1,
> +                                    __lhs.get_allocator());
>      }
>
>  #if __cplusplus >= 201103L
> diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
> index 710c2dfe030..56114f120ba 100644
> --- a/libstdc++-v3/include/bits/basic_string.tcc
> +++ b/libstdc++-v3/include/bits/basic_string.tcc
> @@ -605,47 +605,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  #else
>  # define _GLIBCXX_STRING_CONSTEXPR
>  #endif
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(const _CharT* __lhs,
> -             const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      __glibcxx_requires_string(__lhs);
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type          __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -       rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      const __size_type __len = _Traits::length(__lhs);
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      __str.reserve(__len + __rhs.size());
> -      __str.append(__lhs, __len);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
> -  template<typename _CharT, typename _Traits, typename _Alloc>
> -    _GLIBCXX20_CONSTEXPR
> -    basic_string<_CharT, _Traits, _Alloc>
> -    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> -    {
> -      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> -      typedef typename __string_type::size_type          __size_type;
> -      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> -       rebind<_CharT>::other _Char_alloc_type;
> -      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> -      __string_type __str(_Alloc_traits::_S_select_on_copy(
> -          __rhs.get_allocator()));
> -      const __size_type __len = __rhs.size();
> -      __str.reserve(__len + 1);
> -      __str.append(__size_type(1), __lhs);
> -      __str.append(__rhs);
> -      return __str;
> -    }
> -
>    template<typename _CharT, typename _Traits, typename _Alloc>
>      _GLIBCXX_STRING_CONSTEXPR
>      typename basic_string<_CharT, _Traits, _Alloc>::size_type
> --
> 2.37.3
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] libstdc++: Refactor implementation of operator+ for std::string
@ 2022-10-20  0:05 Will Hawkins
  2022-11-02 20:25 ` Will Hawkins
  2022-11-08 17:44 ` Jonathan Wakely
  0 siblings, 2 replies; 8+ messages in thread
From: Will Hawkins @ 2022-10-20  0:05 UTC (permalink / raw)
  To: gcc-patches, libstdc++; +Cc: Will Hawkins

Sorry for the delay. Tested on x86-64 Linux.

-->8--

After consultation with Jonathan, it seemed like a good idea to create a
single function that performed one-allocation string concatenation that
could be used by various different version of operator+. This patch adds
such a function and calls it from the relevant implementations.

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h:
	Add common function that performs single-allocation string
	concatenation. (__str_cat)
	Use __str_cat to perform optimized operator+, where relevant.
	* include/bits/basic_string.tcc::
	Remove single-allocation implementation of operator+.

Signed-off-by: Will Hawkins <whh8b@obs.cr>
---
 libstdc++-v3/include/bits/basic_string.h   | 66 ++++++++++++++++------
 libstdc++-v3/include/bits/basic_string.tcc | 41 --------------
 2 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index cd244191df4..9c2b57f5a1d 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 _GLIBCXX_END_NAMESPACE_CXX11
 #endif
 
+  template<typename _Str>
+    _GLIBCXX20_CONSTEXPR
+    inline _Str
+    __str_concat(typename _Str::value_type const* __lhs,
+		 typename _Str::size_type __lhs_len,
+		 typename _Str::value_type const* __rhs,
+		 typename _Str::size_type __rhs_len,
+		 typename _Str::allocator_type const& __a)
+    {
+      typedef typename _Str::allocator_type allocator_type;
+      typedef __gnu_cxx::__alloc_traits<allocator_type> _Alloc_traits;
+      _Str __str(_Alloc_traits::_S_select_on_copy(__a));
+      __str.reserve(__lhs_len + __rhs_len);
+      __str.append(__lhs, __lhs_len);
+      __str.append(__rhs, __rhs_len);
+      return __str;
+    }
+
   // operator+
   /**
    *  @brief  Concatenate two strings.
@@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
+    inline basic_string<_CharT, _Traits, _Alloc>
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
 	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
     {
-      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-      __str.append(__rhs);
-      return __str;
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __rhs.c_str(), __rhs.size(),
+				     __lhs.get_allocator());
     }
 
   /**
@@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT,_Traits,_Alloc>
+    inline basic_string<_CharT,_Traits,_Alloc>
     operator+(const _CharT* __lhs,
-	      const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+	      const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+    {
+      __glibcxx_requires_string(__lhs);
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
+				     __rhs.c_str(), __rhs.size(),
+				     __rhs.get_allocator());
+    }
 
   /**
    *  @brief  Concatenate character and string.
@@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
    */
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT,_Traits,_Alloc>
-    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+    inline basic_string<_CharT,_Traits,_Alloc>
+    operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+    {
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
+				     __rhs.c_str(), __rhs.size(),
+				     __rhs.get_allocator());
+    }
 
   /**
    *  @brief  Concatenate string and C string.
@@ -3538,11 +3570,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
 	      const _CharT* __rhs)
     {
-      basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-      __str.append(__rhs);
-      return __str;
+      __glibcxx_requires_string(__rhs);
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __rhs, _Traits::length(__rhs),
+				     __lhs.get_allocator());
     }
-
   /**
    *  @brief  Concatenate string and character.
    *  @param __lhs  First string.
@@ -3554,11 +3587,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
     inline basic_string<_CharT, _Traits, _Alloc>
     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs, _CharT __rhs)
     {
-      typedef basic_string<_CharT, _Traits, _Alloc>	__string_type;
-      typedef typename __string_type::size_type		__size_type;
-      __string_type __str(__lhs);
-      __str.append(__size_type(1), __rhs);
-      return __str;
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+      return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+				     __builtin_addressof(__rhs), 1,
+				     __lhs.get_allocator());
     }
 
 #if __cplusplus >= 201103L
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 710c2dfe030..56114f120ba 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -605,47 +605,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
 # define _GLIBCXX_STRING_CONSTEXPR
 #endif
-
-  template<typename _CharT, typename _Traits, typename _Alloc>
-    _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
-    operator+(const _CharT* __lhs,
-	      const basic_string<_CharT, _Traits, _Alloc>& __rhs)
-    {
-      __glibcxx_requires_string(__lhs);
-      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
-      typedef typename __string_type::size_type	  __size_type;
-      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
-	rebind<_CharT>::other _Char_alloc_type;
-      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
-      const __size_type __len = _Traits::length(__lhs);
-      __string_type __str(_Alloc_traits::_S_select_on_copy(
-          __rhs.get_allocator()));
-      __str.reserve(__len + __rhs.size());
-      __str.append(__lhs, __len);
-      __str.append(__rhs);
-      return __str;
-    }
-
-  template<typename _CharT, typename _Traits, typename _Alloc>
-    _GLIBCXX20_CONSTEXPR
-    basic_string<_CharT, _Traits, _Alloc>
-    operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Alloc>& __rhs)
-    {
-      typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
-      typedef typename __string_type::size_type	  __size_type;
-      typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
-	rebind<_CharT>::other _Char_alloc_type;
-      typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
-      __string_type __str(_Alloc_traits::_S_select_on_copy(
-          __rhs.get_allocator()));
-      const __size_type __len = __rhs.size();
-      __str.reserve(__len + 1);
-      __str.append(__size_type(1), __lhs);
-      __str.append(__rhs);
-      return __str;
-    }
-
   template<typename _CharT, typename _Traits, typename _Alloc>
     _GLIBCXX_STRING_CONSTEXPR
     typename basic_string<_CharT, _Traits, _Alloc>::size_type
-- 
2.37.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-08 17:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05 18:30 [PATCH] libstdc++: Refactor implementation of operator+ for std::string Will Hawkins
2022-09-08 17:50 ` François Dumont
2022-09-08 18:05   ` Jonathan Wakely
2022-09-09 13:53     ` Will Hawkins
2022-09-09 20:28   ` Jonathan Wakely
2022-10-20  0:05 Will Hawkins
2022-11-02 20:25 ` Will Hawkins
2022-11-08 17:44 ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).