From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x12c.google.com (mail-il1-x12c.google.com [IPv6:2607:f8b0:4864:20::12c]) by sourceware.org (Postfix) with ESMTPS id 6B6A23857C68 for ; Wed, 2 Nov 2022 20:26:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6B6A23857C68 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=obs.cr Authentication-Results: sourceware.org; spf=none smtp.mailfrom=obs.cr Received: by mail-il1-x12c.google.com with SMTP id l6so45962ilq.3 for ; Wed, 02 Nov 2022 13:26:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=obs-cr.20210112.gappssmtp.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=wRCm7fzORYjAfrLWwsdtf5RV0e/BzAokaIl62mDAcS8=; b=YPYiRfnrnnP2FLzA6/YUU+15VxpBgbpvdAF/pSHLN9aEOmfMtRn5xuKQcIdcy99iw3 RQ/8pRi4G/tqNRta2A1JeTegfcZKFN1q4SutIK6STy84fID97RyOOJyzFDaJb/+fk5MM Fc8OcTKxtW+bM0XQe9/ZxX9PeZOsSgPu+IlIZgsCCXbaJxvbb69UkCIpxMqpSiNjOSyg fIaGUTLq+zOVxDcGIeC9Vq9WKmSu2fpAVUN34DhpnY6ZF3R0fDy5M1wt6UzOPKnh3mcd ufGshSyhSDysTkbC2xFetrwoDPW8Re0Q+LWC3Bd6mvk/5kAkjzrVgMPXTymJ+GQode7G R/UA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wRCm7fzORYjAfrLWwsdtf5RV0e/BzAokaIl62mDAcS8=; b=eV8U9Gk6mzIJ6Oabhv6z83ctSQDxNwOIDEPQIRMMciCE5hNFQiYeHg3HoRGofP9fM2 rAE7Mxrg6nxNuwyssPVPluM18gKPRIlXIzGnI0PBe2Uwo1audq+OpZQAECUdKLDp1cii wfubU85u7bx1Vwp1h3q3eXXkzUBTI5kHosyYSNDGuyaY7+IFcrzANzBy4EQQ8nuIzp4Z t9XzKGOvw8qPWnd7s6Xl4GHOzBeNRehptd5qC6sVzMTtsoFtTARegusDsf5Mh4V7zwUE fv81ZCdOEIgJfo7mddmOHgKvteRnXI4ozwhPKcK6yf7EtWJjijR/IC5BGJ1nMApKwWeg itfg== X-Gm-Message-State: ACrzQf1FsT9wppepPvf5HkYg0QvsqmJhgKYbAzmUXcqeF0AHYILPJ/b3 JCNJ8kzB9ovuXvlFCfZa2ugzSzpT/nD54w/Pi7y00hy6NGU= X-Google-Smtp-Source: AMsMyM7G/DKsEzDrf3TcIFACdXI842tcQbMvlju4ewiZspA6/tRGCrpR1BjdTub3wUnKCVu3oVn+jeOnsDTejsg4PMA= X-Received: by 2002:a92:c0d0:0:b0:300:a256:389a with SMTP id t16-20020a92c0d0000000b00300a256389amr12879402ilf.153.1667420764646; Wed, 02 Nov 2022 13:26:04 -0700 (PDT) MIME-Version: 1.0 References: <20221020000559.371886-1-whh8b@obs.cr> In-Reply-To: <20221020000559.371886-1-whh8b@obs.cr> From: Will Hawkins Date: Wed, 2 Nov 2022 16:25:53 -0400 Message-ID: Subject: Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string To: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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 > --- > 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 > + _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 _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 > _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 > _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 > _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 > - _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 > - _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 > _GLIBCXX_STRING_CONSTEXPR > typename basic_string<_CharT, _Traits, _Alloc>::size_type > -- > 2.37.3 >