Hi

   libstdc++: [_GLIBCXX_DEBUG] Improve COW basic_string checks

    Put the __glibcxx_requires_valid_range checks at public member level
    so that diagnostics are giving directly the invoked member name.

    Add the std::_RequireInputIter constraints on members taking template
    iterator range to avoid the "do the right thing" ambiguity without the help of __is_integer.

    libstdc++v3/ChangeLog:

            * include/bits/cow_string.h
            (basic_string::_M_replace): New.
            (basic_string(_InputIterator, _InputIterator, const _Alloc&): Add             std::_RequireInputIter constraint and __glibcxx_requires_valid_constructor_range
            check.
            (basic_string::append(_InputIte, _InputIte): Add std::_RequireInputIter             constraint and __glibcxx_requires_valid_range check. Call _M_replace.
            (basic_string::assign(_InputIte, _InputIte): Likewise.
            (basic_string::insert(iterator, _InputIte, _InputIte)): Likewise.             (basic_string::replace(iterator, iterator, _InputIte, _InputIte)): Likewise.
            [__cplusplus >= 201103L](basic_string::_S_construct): New.
            (basic_string::_M_replace_dispatch(iterator, iterator, _InputIte, _InputIte,
            __false_type)): Remove __glibcxx_requires_valid_range check.
            * include/debug/debug.h (__glibcxx_requires_valid_constructor_range): New.

Tested under Linux x86_64 with _GLIBCXX_USE_CXX11_ABI=0 and _GLIBCXX_DEBUG.

Note that there is a double requires-valid-range check when _M_replace is being called because of the temporary basic_string instantiation.

Ok to commit ?

François
diff --git a/libstdc++-v3/include/bits/cow_string.h 
b/libstdc++-v3/include/bits/cow_string.h
index 7c945f6b998..eb8e80d06db 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -721,10 +721,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  @param  __end  End of range.
        *  @param  __a  Allocator to use (default is default allocator).
        */
-      template<class _InputIterator>
+#if __cplusplus >= 201103L
+      template<typename _InputIterator,
+              typename = std::_RequireInputIter<_InputIterator>>
+#else
+      template<typename _InputIterator>
+#endif
        basic_string(_InputIterator __beg, _InputIterator __end,
                     const _Alloc& __a = _Alloc())
-       : _M_dataplus(_S_construct(__beg, __end, __a), __a)
+       : _M_dataplus(_S_construct(
+         __glibcxx_requires_valid_constructor_range(__beg, __end),
+         __end, __a), __a)
        { }
 
 #ifdef __glibcxx_string_view // >= C++17
@@ -1391,10 +1398,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *
        *  Appends characters in the range [__first,__last) to this string.
        */
-      template<class _InputIterator>
+#if __cplusplus >= 201103L
+      template<typename _InputIterator,
+              typename = std::_RequireInputIter<_InputIterator>>
+#else
+      template<typename _InputIterator>
+#endif
        basic_string&
        append(_InputIterator __first, _InputIterator __last)
-       { return this->replace(_M_iend(), _M_iend(), __first, __last); }
+       {
+         __glibcxx_requires_valid_range(__first, __last);
+         return _M_replace(_M_iend(), _M_iend(), __first, __last);
+       }
 
 #ifdef __glibcxx_string_view // >= C++17
       /**
@@ -1538,10 +1553,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *
        *  Sets value of string to characters in the range [__first,__last).
       */
-      template<class _InputIterator>
+#if __cplusplus >= 201103L
+      template<typename _InputIterator,
+              typename = std::_RequireInputIter<_InputIterator>>
+#else
+      template<typename _InputIterator>
+#endif
        basic_string&
        assign(_InputIterator __first, _InputIterator __last)
-       { return this->replace(_M_ibegin(), _M_iend(), __first, __last); }
+       {
+         __glibcxx_requires_valid_range(__first, __last);
+         return _M_replace(_M_ibegin(), _M_iend(), __first, __last);
+       }
 
 #if __glibcxx_containers_ranges // C++ >= 23
       /**
@@ -1631,10 +1654,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  length_error is thrown.  The value of the string doesn't
        *  change if an error is thrown.
       */
-      template<class _InputIterator>
+#if __cplusplus >= 201103L
+      template<typename _InputIterator,
+              typename = std::_RequireInputIter<_InputIterator>>
+#else
+      template<typename _InputIterator>
+#endif
        void
        insert(iterator __p, _InputIterator __beg, _InputIterator __end)
-       { this->replace(__p, __p, __beg, __end); }
+       {
+         _GLIBCXX_DEBUG_PEDASSERT(_M_ibegin() <= __p && __p <= _M_iend());
+         __glibcxx_requires_valid_range(__beg, __end);
+         _M_replace(__p, __p, __beg, __end);
+       }
 
 #if __glibcxx_containers_ranges // C++ >= 23
       /**
@@ -2117,15 +2149,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  The value of the string doesn't change if an error is
        *  thrown.
       */
-      template<class _InputIterator>
+#if __cplusplus >= 201103L
+      template<typename _InputIterator,
+              typename = std::_RequireInputIter<_InputIterator>>
+#else
+      template<typename _InputIterator>
+#endif
        basic_string&
        replace(iterator __i1, iterator __i2,
                _InputIterator __k1, _InputIterator __k2)
        {
          _GLIBCXX_DEBUG_PEDASSERT(_M_ibegin() <= __i1 && __i1 <= __i2
                                   && __i2 <= _M_iend());
-         typedef typename std::__is_integer<_InputIterator>::__type _Integral;
-         return _M_replace_dispatch(__i1, __i2, __k1, __k2, _Integral());
+         __glibcxx_requires_valid_range(__k1, __k2);
+         return _M_replace(__i1, __i2, __k1, __k2);
        }
 
       // Specializations for the common case of pointer and iterator:
@@ -2269,11 +2306,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // C++17
 
     private:
+      template<class _InputIterator>
+       basic_string&
+       _M_replace(iterator __i1, iterator __i2,
+                  _InputIterator __k1, _InputIterator __k2)
+       {
+#if __cplusplus < 201103L
+         typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+#else
+         using _Integral = std::__false_type;
+#endif
+         return _M_replace_dispatch(__i1, __i2, __k1, __k2, _Integral());
+       }
+
+#if __cplusplus < 201103L
       template<class _Integer>
        basic_string&
        _M_replace_dispatch(iterator __i1, iterator __i2, _Integer __n,
                            _Integer __val, __true_type)
        { return _M_replace_aux(__i1 - _M_ibegin(), __i2 - __i1, __n, __val); }
+#endif
 
       template<class _InputIterator>
        basic_string&
@@ -2288,6 +2340,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_replace_safe(size_type __pos1, size_type __n1, const _CharT* __s,
                      size_type __n2);
 
+#if __cplusplus < 201103L
       // _S_construct_aux is used to implement the 21.3.1 para 15 which
       // requires special behaviour if _InIter is an integral type
       template<class _InIterator>
@@ -2319,6 +2372,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
          typedef typename std::__is_integer<_InIterator>::__type _Integral;
          return _S_construct_aux(__beg, __end, __a, _Integral());
        }
+#else
+      template<typename _InIterator>
+       static _CharT*
+       _S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a)
+       {
+         return _S_construct(__beg, __end, __a,
+                             std::__iterator_category(__beg));
+       }
+#endif
 
       // For Input Iterators, used in istreambuf_iterators, etc.
       template<class _InIterator>
@@ -3832,7 +3894,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_replace_dispatch(iterator __i1, iterator __i2, _InputIterator __k1,
                          _InputIterator __k2, __false_type)
       {
-       __glibcxx_requires_valid_range(__k1, __k2);
        const basic_string __s(__k1, __k2);
        const size_type __n1 = __i2 - __i1;
        _M_check_length(__n1, __s.size(), "basic_string::_M_replace_dispatch");
diff --git a/libstdc++-v3/include/debug/debug.h 
b/libstdc++-v3/include/debug/debug.h
index 5604f98e4d3..aabb9af2271 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -65,6 +65,8 @@ namespace __gnu_debug
 
 # define __glibcxx_requires_cond(_Cond,_Msg)
 # define __glibcxx_requires_valid_range(_First,_Last)
+# define __glibcxx_requires_valid_constructor_range(_First,_Last) \
+  _First
 # define __glibcxx_requires_can_increment(_First,_Size)
 # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2)
 # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2)
@@ -92,6 +94,8 @@ namespace __gnu_debug
 # define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg)
 # define __glibcxx_requires_valid_range(_First,_Last)  \
   __glibcxx_check_valid_range(_First,_Last)
+# define __glibcxx_requires_valid_constructor_range(_First,_Last) \
+  __glibcxx_check_valid_constructor_range(_First,_Last)
 # define __glibcxx_requires_can_increment(_First,_Size)        \
   __glibcxx_check_can_increment(_First,_Size)
 # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2)        
\

Reply via email to