From patchwork Mon Sep 5 18:30:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Hawkins X-Patchwork-Id: 57387 X-Patchwork-Delegate: jwakely.gcc@gmail.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 039DE383CCCD for ; Mon, 5 Sep 2022 18:31:09 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qt1-x82c.google.com (mail-qt1-x82c.google.com [IPv6:2607:f8b0:4864:20::82c]) by sourceware.org (Postfix) with ESMTPS id 7B565385AC33 for ; Mon, 5 Sep 2022 18:30:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B565385AC33 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-qt1-x82c.google.com with SMTP id e28so6593632qts.1 for ; Mon, 05 Sep 2022 11:30:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=obs-cr.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date; bh=eAAOoBRii5R6L+S4fxfzmsmp1gB22hDz9AjvltmAkEk=; b=vBtJO1mH+C1IUvFM9r60oIRpLZM3EltL+PfbfyEzZocikgtb4mReijCMxME/Q+Qkqa 1mJcQS8jrxL/aJZ45eU8Pln94VSpV69jahuWjVsRZfdfvmX/JTsDOUClmJjPd/d3V8dr qq+dNqtMl5Gyxa5KQUm7GPYpNlf9tWBhx55eHowelNY+ePKi98m058ZUfF1BbzSc/xr6 IT9lv1/r4/z8wAT/Ilmbwdz2gtnKWu2bKcKJozrWkVQUPx40oWSAW5OD9rflqsZQsu93 1YQZXyO+D/tPvhXPQ5bFn6Pz0I30rYX6KlwY0tf+g1ngV46/8U4UI1vxKO/KphdhP0Se a74g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date; bh=eAAOoBRii5R6L+S4fxfzmsmp1gB22hDz9AjvltmAkEk=; b=rzMqSQhQmZKUX/GUb3FIGhrz6tVR9Vsg7zRysRSf9bsjkFq/MOOZOpIo+tWmDizhtA SH91yYrf4kD0+tFU20j1vyNJZzjiait/rSpSmTgYLdsRUYxBVBIm/hXLIOaWUkD3ktnX hg6UeQGlKjF5NogFy5AzGJOJ+KcB0wu98H/AjCksB0yaby2p7fSr9JEKB+VaGAINN9JF skr/JA6O7PifLuR735Vs9+dazfq7YcKxWHcQvS58kLaw1b9sBCHV4ax5NC8YjT3ea4ZK 7U03h2aHI6ul/oEmdVNYLD9JjAvcscssT5KNaOZAzOtiRVM3Z6UCP2xnSwjyWJRudU4X tfrw== X-Gm-Message-State: ACgBeo3PfibMZQ98XOhCD0TIwcac1oaeulwVYmSo/fGf/HeFnUSWAt2o i+5vc1nx/rSdXtVfxKIGo1kv1NCRJyao/Pv3 X-Google-Smtp-Source: AA6agR46tGbxm4XDY10xddQoVPOwbNeHSdQTaSNlsKYcXet0j441C0XhqfRp6MilatuWY+ok3Iw9fQ== X-Received: by 2002:ac8:5b05:0:b0:342:fd04:a25c with SMTP id m5-20020ac85b05000000b00342fd04a25cmr41087597qtw.303.1662402631507; Mon, 05 Sep 2022 11:30:31 -0700 (PDT) Received: from localhost.localdomain (ip-192-24-137-251.dynamic.fuse.net. [192.24.137.251]) by smtp.gmail.com with ESMTPSA id o11-20020ac872cb000000b003422c7ccbc5sm7490610qtp.59.2022.09.05.11.30.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Sep 2022 11:30:30 -0700 (PDT) From: Will Hawkins To: gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Refactor implementation of operator+ for std::string Date: Mon, 5 Sep 2022 14:30:12 -0400 Message-Id: <20220905183011.43874-1-whh8b@obs.cr> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Will Hawkins , libstdc++@gcc.gnu.org Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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 --- 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 + _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. @@ -3490,13 +3508,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()); } /** @@ -3507,9 +3526,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. @@ -3519,8 +3545,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. @@ -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 - _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