* [committed] libstdc++: Enforce value_type consistency in strings and streams
@ 2023-05-11 20:18 Jonathan Wakely
2023-05-11 23:29 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2023-05-11 20:18 UTC (permalink / raw)
To: libstdc++, gcc-patches
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<Iter, char_traits<Iter>, 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<A>::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<typename _CharT, typename _Traits>
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<typename _CharT, typename _Traits, typename _Alloc>
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<typename _Traits2, typename _Dummy_for_PR85282>
- 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<typename _Dummy_for_PR85282>
- struct _Alloc_traits_impl<char_traits<_CharT>, _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 <debug/string>
-// { 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 <string>
-// { dg-do compile }
+// { dg-do compile { target c++17_down } }
// libstdc++/21770
template class std::basic_string<int, std::char_traits<int>,
--
2.40.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [committed] libstdc++: Enforce value_type consistency in strings and streams
2023-05-11 20:18 [committed] libstdc++: Enforce value_type consistency in strings and streams Jonathan Wakely
@ 2023-05-11 23:29 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2023-05-11 23:29 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1.1: Type: text/plain, Size: 501 bytes --]
On Thu, 11 May 2023 at 21:20, Jonathan Wakely via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> 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.
>
>
Here's the gcc-13 backport. No new assertions, just the new exported symbol.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 6265 bytes --]
commit 0d5a359140503d26adf11325e1f9a09ba7067dfc
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed May 10 21:30:10 2023
libstdc++: Backport std::basic_string::_S_allocate from trunk
This is a backport of r14-739-gc62e945492afbb to keep the exported
symbol list consistent between trunk and gcc-13. The new assertions from
that commit are not part of this backport.
libstdc++-v3/ChangeLog:
* config/abi/pre/gnu.ver: Export basic_string::_S_allocate.
* include/bits/basic_string.h: (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.
(cherry picked from commit c62e945492afbbd2a09896fc7b0b07f7e719a606)
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_string.h b/libstdc++-v3/include/bits/basic_string.h
index b16b2898b62..870b4728928 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -89,36 +89,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
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<typename _Traits2, typename _Dummy_for_PR85282>
- 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<typename _Dummy_for_PR85282>
- struct _Alloc_traits_impl<char_traits<_CharT>, _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
// Types:
public:
@@ -149,6 +120,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 +1583,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);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-05-11 23:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-11 20:18 [committed] libstdc++: Enforce value_type consistency in strings and streams Jonathan Wakely
2023-05-11 23:29 ` 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).