[gcc r14-739] libstdc++: Enforce value_type consistency in strings and streams

[gcc r14-739] libstdc++: Enforce value_type consistency in strings and streams

[Jonathan Wakely]

https://gcc.gnu.org/g:c62e945492afbbd2a09896fc7b0b07f7e719a606

commit r14-739-gc62e945492afbbd2a09896fc7b0b07f7e719a606
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 10 21:30:10 2023 +0100

    libstdc++: Enforce value_type consistency in strings and streams
    
    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.

Diff:
---
 libstdc++-v3/config/abi/pre/gnu.ver                |  5 +-
 libstdc++-v3/include/bits/basic_ios.h              |  4 ++
 libstdc++-v3/include/bits/basic_string.h           | 53 ++++++++++------------
 libstdc++-v3/include/bits/basic_string.tcc         |  8 ++--
 .../requirements/explicit_instantiation/debug.cc   |  2 +-
 .../requirements/explicit_instantiation/int.cc     |  2 +-
 6 files changed, 36 insertions(+), 38 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,39 +86,16 @@ _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;
+#endif
 
-#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 +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>,