From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 056A83858D1E for ; Thu, 11 May 2023 20:18:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 056A83858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683836329; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Y0kqsDTyySkOtnfKMx/QDc3V1Sxpg3fTvpX6znnxJRU=; b=bGtU30+toeO5YnB6u1oIuRcpEbMnu6Z5czUPxvHIg4yivuQabtuGIXmn65HLLkxT/DWQZx 7JfUK5JDxTbPdI19zOcqZtDpzuN02Y1gTvczhieE76hhjIp87/HaTkmzzr23j9omt+dMMj wxkRFQr8kezHo3/XXJKE8KSA94fy4hk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-554-ql31847hPh2MQ8snlF2CKA-1; Thu, 11 May 2023 16:18:48 -0400 X-MC-Unique: ql31847hPh2MQ8snlF2CKA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 324D8185A790; Thu, 11 May 2023 20:18:48 +0000 (UTC) Received: from localhost (unknown [10.42.28.178]) by smtp.corp.redhat.com (Postfix) with ESMTP id EE1982166B26; Thu, 11 May 2023 20:18:47 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Enforce value_type consistency in strings and streams Date: Thu, 11 May 2023 21:18:47 +0100 Message-Id: <20230511201847.1056247-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tested powerpc64le-linux. Pushed to trunk. I don't plan to backport the assertions, because they're an API change that isn't suitable for the branches. But removing _Alloc_traits_impl and replacing it with _S_allocate should be done for gcc-13 to keep the contents of the two libstdc++.so.6.0.32 libraries in sync. -- >8 -- P1463R1 made it ill-formed for allocator-aware containers (including std::basic_string) to use an allocator that has a different value_type from the container itself. We already enforce that for other containers (since r8-4828-g866e4d3853ccc0), but not for std::basic_string. We traditionally accepted it as an extension and rebound the allocator, so this change only adds the enforcement for C++20 and later. Similarly, P1148R0 made it ill-formed for strings and streams to use a traits type that has an incorrect char_type. We already enforce that for std::basic_string_view, so we just need to add it to std::basic_ios and std::basic_string. The assertion for the allocator's value_type caused some testsuite regressions: FAIL: 21_strings/basic_string/cons/char/deduction.cc (test for excess errors) FAIL: 21_strings/basic_string/cons/wchar_t/deduction.cc (test for excess errors) FAIL: 21_strings/basic_string/requirements/explicit_instantiation/debug.cc (test for excess errors) FAIL: 21_strings/basic_string/requirements/explicit_instantiation/int.cc (test for excess errors) The last two are testing the traditional extension that rebinds the allocator, so need to be disabled for C++20. The first two are similar to LWG 3076 where an incorrect constructor is considered for CTAD. In this case, determining that it's not viable requires instantiating std::basic_string, Alloc> which then fails the new assertion, because Alloc::value_type is not the same as Iter. This is only a problem because the size_type parameter of the non-viable constructor is an alias for _Alloc_traits_impl::size_type which is a nested type, and so the enclosing basic_string specialization needs to be instantiated. If we remove the _Alloc_traits_impl wrapper that was added in r12-5413-g2d76292bd6719d, then the definition of size_type no longer depends on basic_string, and we don't instantiate an invalid specialization and don't fail the assertion. The work done by _Alloc_traits_impl::allocate can be done in a _S_allocate function instead, which is probably more efficient to compile anyway. libstdc++-v3/ChangeLog: * config/abi/pre/gnu.ver: Export basic_string::_S_allocate. * include/bits/basic_ios.h: Add static assertion checking traits_type::value_type. * include/bits/basic_string.h: Likewise. Do not rebind allocator, and add static assertion checking its value_type. (basic_string::_Alloc_traits_impl): Remove class template. (basic_string::_S_allocate): New static member function. (basic_string::assign): Use _S_allocate. * include/bits/basic_string.tcc (basic_string::_M_create) (basic_string::reserve, basic_string::_M_replace): Likewise. * testsuite/21_strings/basic_string/requirements/explicit_instantiation/debug.cc: Disable for C++20 and later. * testsuite/21_strings/basic_string/requirements/explicit_instantiation/int.cc: Likweise. --- libstdc++-v3/config/abi/pre/gnu.ver | 5 +- libstdc++-v3/include/bits/basic_ios.h | 4 ++ libstdc++-v3/include/bits/basic_string.h | 55 ++++++++----------- libstdc++-v3/include/bits/basic_string.tcc | 8 +-- .../explicit_instantiation/debug.cc | 2 +- .../explicit_instantiation/int.cc | 2 +- 6 files changed, 37 insertions(+), 39 deletions(-) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 36bb87880d7..768cd4a4a6c 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1759,7 +1759,9 @@ GLIBCXX_3.4.21 { #endif # ABI-tagged std::basic_string - _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE1[01]**; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE10_M_[dr]*; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE10_S_compareE[jmy][jmy]; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE11_M_capacityE[jmy]; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE12_Alloc_hiderC[12]EP[cw]RKS3_; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE12_M*; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE13*; @@ -2516,6 +2518,7 @@ GLIBCXX_3.4.31 { GLIBCXX_3.4.32 { _ZSt21ios_base_library_initv; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE11_S_allocateERS3_[jmy]; } GLIBCXX_3.4.31; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/basic_ios.h b/libstdc++-v3/include/bits/basic_ios.h index de5719c1d68..c7c391c0f49 100644 --- a/libstdc++-v3/include/bits/basic_ios.h +++ b/libstdc++-v3/include/bits/basic_ios.h @@ -66,6 +66,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template class basic_ios : public ios_base { +#if __cplusplus >= 202002L + static_assert(is_same_v<_CharT, typename _Traits::char_type>); +#endif + public: ///@{ /** diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index b16b2898b62..d15f0f0589f 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -86,40 +86,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 template class basic_string { +#if __cplusplus >= 202002L + static_assert(is_same_v<_CharT, typename _Traits::char_type>); + static_assert(is_same_v<_CharT, typename _Alloc::value_type>); + using _Char_alloc_type = _Alloc; +#else typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template rebind<_CharT>::other _Char_alloc_type; - -#if __cpp_lib_constexpr_string < 201907L - typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits; -#else - template - struct _Alloc_traits_impl : __gnu_cxx::__alloc_traits<_Char_alloc_type> - { - typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Base; - - [[__gnu__::__always_inline__]] - static constexpr typename _Base::pointer - allocate(_Char_alloc_type& __a, typename _Base::size_type __n) - { - pointer __p = _Base::allocate(__a, __n); - if (std::is_constant_evaluated()) - // Begin the lifetime of characters in allocated storage. - for (size_type __i = 0; __i < __n; ++__i) - std::construct_at(__builtin_addressof(__p[__i])); - return __p; - } - }; - - template - struct _Alloc_traits_impl, _Dummy_for_PR85282> - : __gnu_cxx::__alloc_traits<_Char_alloc_type> - { - // std::char_traits begins the lifetime of characters. - }; - - using _Alloc_traits = _Alloc_traits_impl<_Traits, void>; #endif + typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits; + // Types: public: typedef _Traits traits_type; @@ -149,6 +126,22 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 #endif private: + static _GLIBCXX20_CONSTEXPR pointer + _S_allocate(_Char_alloc_type& __a, size_type __n) + { + pointer __p = _Alloc_traits::allocate(__a, __n); +#if __cpp_lib_constexpr_string >= 201907L + // std::char_traits begins the lifetime of characters, + // but custom traits might not, so do it here. + if constexpr (!is_same_v<_Traits, char_traits<_CharT>>) + if (std::__is_constant_evaluated()) + // Begin the lifetime of characters in allocated storage. + for (size_type __i = 0; __i < __n; ++__i) + std::construct_at(__builtin_addressof(__p[__i])); +#endif + return __p; + } + #if __cplusplus >= 201703L // A helper type for avoiding boiler-plate. typedef basic_string_view<_CharT, _Traits> __sv_type; @@ -1596,7 +1589,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 const auto __len = __str.size(); auto __alloc = __str._M_get_allocator(); // If this allocation throws there are no effects: - auto __ptr = _Alloc_traits::allocate(__alloc, __len + 1); + auto __ptr = _S_allocate(__alloc, __len + 1); _M_destroy(_M_allocated_capacity); _M_data(__ptr); _M_capacity(__len); diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index 99fdbeee5ad..d8a279fc9ed 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -152,7 +152,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // NB: Need an array of char_type[__capacity], plus a terminating // null char_type() element. - return _Alloc_traits::allocate(_M_get_allocator(), __capacity + 1); + return _S_allocate(_M_get_allocator(), __capacity + 1); } // NB: This is the special case for Input Iterators, used in @@ -376,8 +376,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION else if (__length < __capacity) try { - pointer __tmp - = _Alloc_traits::allocate(_M_get_allocator(), __length + 1); + pointer __tmp = _S_allocate(_M_get_allocator(), __length + 1); this->_S_copy(__tmp, _M_data(), __length + 1); _M_dispose(); _M_data(__tmp); @@ -521,8 +520,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cpp_lib_is_constant_evaluated if (std::is_constant_evaluated()) { - auto __newp = _Alloc_traits::allocate(_M_get_allocator(), - __new_size); + auto __newp = _S_allocate(_M_get_allocator(), __new_size); _S_copy(__newp, this->_M_data(), __pos); _S_copy(__newp + __pos, __s, __len2); _S_copy(__newp + __pos + __len2, __p + __len1, __how_much); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/debug.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/debug.cc index b70fbd50460..6bb46c5356c 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/debug.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/debug.cc @@ -19,7 +19,7 @@ #include -// { dg-do compile } +// { dg-do compile { target c++17_down } } // libstdc++/21770 namespace debug = __gnu_debug; diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/int.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/int.cc index 8326278b36c..4f23cc4dcef 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/int.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/int.cc @@ -20,7 +20,7 @@ #include -// { dg-do compile } +// { dg-do compile { target c++17_down } } // libstdc++/21770 template class std::basic_string, -- 2.40.1