diff mbox

Container debug light mode

Message ID 575885E2.1050000@gmail.com
State New
Headers show

Commit Message

François Dumont June 8, 2016, 8:53 p.m. UTC 
Hi

     Here is the patch I already proposed to introduce the debug light 
mode for vector and deque containers.

     It also simplify some internal calls.

     * include/debug/debug.h
     (__glibcxx_requires_non_empty_range, __glibcxx_requires_nonempty)
     (__glibcxx_requires_subscript): Move...
     * include/debug/assertions.h: ...here and add __builtin_expect.
     (_GLIBCXX_DEBUG_ONLY): Remove ; value.
     * include/bits/stl_deque.h
     (std::deque<>::operator[]): Add __glibcxx_requires_subscript check.
     (std::deque<>::front()): Add __glibcxx_requires_nonempty check.
     (std::deque<>::back()): Likewise.
     (std::deque<>::pop_front()): Likewise.
     (std::deque<>::pop_back()): Likewise.
     (std::deque<>::swap(deque&)): Add allocator check.
     (std::deque<>::operator=): Call _M_assign_aux.
     (std::deque<>::assign(initializer_list<>)): Likewise.
     (std::deque<>::resize(size_t, const value_type&)): Call _M_fill_insert.
     (std::deque<>::insert(const_iterator, initializer_list<>)):
     Call _M_range_insert_aux.
     (std::deque<>::_M_assign_aux<It>(It, It, std::forward_iterator_tag):
     Likewise.
     (std::deque<>::_M_fill_assign): Call _M_fill_insert.
     (std::deque<>::_M_move_assign2): Call _M_assign_aux.
     * include/bits/deque.tcc
     (std::deque<>::operator=): Call _M_range_insert_aux.
     (std::deque<>::_M_assign_aux<It>(It, It, std::input_iterator_tag)):
     Likewise.
     * include/bits/stl_vector.h
     (std::vector<>::operator[]): Add __glibcxx_requires_subscript check.
     (std::vector<>::front()): Add __glibcxx_requires_nonempty check.
     (std::vector<>::back()): Likewise.
     (std::vector<>::pop_back()): Likewise.
     (std::vector<>::swap(vector&)): Add allocator check.
     (std::vector<>::operator=): Call _M_assign_aux.
     (std::vector<>::assign(initializer_list<>)): Likewise.
     (std::vector<>::resize(size_t, const value_type&)): Call 
_M_fill_insert.
     (std::vector<>::insert(const_iterator, initializer_list<>)):
     Call _M_range_insert.
     * include/bits/vector.tcc (std::vector<>::_M_assign_aux): Likewise.

Successfully run vector and deque tests under Linux x86_64 for now, will 
complete before commit.

François

Comments

Jonathan Wakely June 13, 2016, 10:21 a.m. UTC | #1 
On 08/06/16 22:53 +0200, François Dumont wrote:
>Hi
>
>    Here is the patch I already proposed to introduce the debug light 
>mode for vector and deque containers.
>
>    It also simplify some internal calls.

This looks great, and I'd like to see it on trunk, but could you split
it into two patches please? The simplifications to use
__iterator_category and replace insert() with _M_insert_* are good but
are unrelated to the debug mode parts so if there are two separate
commits it's easier to backport one piece separately, or to identify
any regressions that might be introduced.
diff mbox

Patch

Index: include/bits/deque.tcc
===================================================================
--- include/bits/deque.tcc	(revision 237180)
+++ include/bits/deque.tcc	(working copy)
@@ -119,7 +119,8 @@ 
 	    {
 	      const_iterator __mid = __x.begin() + difference_type(__len);
 	      std::copy(__x.begin(), __mid, this->_M_impl._M_start);
-	      insert(this->_M_impl._M_finish, __mid, __x.end());
+	      _M_range_insert_aux(this->_M_impl._M_finish, __mid, __x.end(),
+				  std::random_access_iterator_tag());
 	    }
 	}
       return *this;
@@ -280,7 +281,8 @@ 
         if (__first == __last)
           _M_erase_at_end(__cur);
         else
-          insert(end(), __first, __last);
+          _M_range_insert_aux(end(), __first, __last,
+			      std::__iterator_category(__first));
       }
 
   template <typename _Tp, typename _Alloc>
Index: include/bits/stl_deque.h
===================================================================
--- include/bits/stl_deque.h	(revision 237180)
+++ include/bits/stl_deque.h	(working copy)
@@ -63,6 +63,8 @@ 
 #include <initializer_list>
 #endif
 
+#include <debug/assertions.h>
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -1081,7 +1083,8 @@ 
       deque&
       operator=(initializer_list<value_type> __l)
       {
-	this->assign(__l.begin(), __l.end());
+	_M_assign_aux(__l.begin(), __l.end(),
+		      random_access_iterator_tag());
 	return *this;
       }
 #endif
@@ -1142,7 +1145,7 @@ 
        */
       void
       assign(initializer_list<value_type> __l)
-      { this->assign(__l.begin(), __l.end()); }
+      { _M_assign_aux(__l.begin(), __l.end(), random_access_iterator_tag()); }
 #endif
 
       /// Get a copy of the memory allocation object.
@@ -1306,7 +1309,7 @@ 
       {
 	const size_type __len = size();
 	if (__new_size > __len)
-	  insert(this->_M_impl._M_finish, __new_size - __len, __x);
+	  _M_fill_insert(this->_M_impl._M_finish, __new_size - __len, __x);
 	else if (__new_size < __len)
 	  _M_erase_at_end(this->_M_impl._M_start
 			  + difference_type(__new_size));
@@ -1328,7 +1331,7 @@ 
       {
 	const size_type __len = size();
 	if (__new_size > __len)
-	  insert(this->_M_impl._M_finish, __new_size - __len, __x);
+	  _M_fill_insert(this->_M_impl._M_finish, __new_size - __len, __x);
 	else if (__new_size < __len)
 	  _M_erase_at_end(this->_M_impl._M_start
 			  + difference_type(__new_size));
@@ -1364,7 +1367,10 @@ 
        */
       reference
       operator[](size_type __n) _GLIBCXX_NOEXCEPT
-      { return this->_M_impl._M_start[difference_type(__n)]; }
+      {
+	__glibcxx_requires_subscript(__n);
+	return this->_M_impl._M_start[difference_type(__n)];
+      }
 
       /**
        *  @brief Subscript access to the data contained in the %deque.
@@ -1379,7 +1385,10 @@ 
        */
       const_reference
       operator[](size_type __n) const _GLIBCXX_NOEXCEPT
-      { return this->_M_impl._M_start[difference_type(__n)]; }
+      {
+	__glibcxx_requires_subscript(__n);
+	return this->_M_impl._M_start[difference_type(__n)];
+      }
 
     protected:
       /// Safety check used only from at().
@@ -1436,7 +1445,10 @@ 
        */
       reference
       front() _GLIBCXX_NOEXCEPT
-      { return *begin(); }
+      {
+	__glibcxx_requires_nonempty();
+	return *begin();
+      }
 
       /**
        *  Returns a read-only (constant) reference to the data at the first
@@ -1444,7 +1456,10 @@ 
        */
       const_reference
       front() const _GLIBCXX_NOEXCEPT
-      { return *begin(); }
+      {
+	__glibcxx_requires_nonempty();
+	return *begin();
+      }
 
       /**
        *  Returns a read/write reference to the data at the last element of the
@@ -1453,6 +1468,7 @@ 
       reference
       back() _GLIBCXX_NOEXCEPT
       {
+	__glibcxx_requires_nonempty();
 	iterator __tmp = end();
 	--__tmp;
 	return *__tmp;
@@ -1465,6 +1481,7 @@ 
       const_reference
       back() const _GLIBCXX_NOEXCEPT
       {
+	__glibcxx_requires_nonempty();
 	const_iterator __tmp = end();
 	--__tmp;
 	return *__tmp;
@@ -1548,6 +1565,7 @@ 
       void
       pop_front() _GLIBCXX_NOEXCEPT
       {
+	__glibcxx_requires_nonempty();
 	if (this->_M_impl._M_start._M_cur
 	    != this->_M_impl._M_start._M_last - 1)
 	  {
@@ -1570,6 +1588,7 @@ 
       void
       pop_back() _GLIBCXX_NOEXCEPT
       {
+	__glibcxx_requires_nonempty();
 	if (this->_M_impl._M_finish._M_cur
 	    != this->_M_impl._M_finish._M_first)
 	  {
@@ -1645,7 +1664,12 @@ 
        */
       iterator
       insert(const_iterator __p, initializer_list<value_type> __l)
-      { return this->insert(__p, __l.begin(), __l.end()); }
+      {
+	auto __offset = __p - cbegin();
+	_M_range_insert_aux(__p._M_const_cast(), __l.begin(), __l.end(),
+			    std::random_access_iterator_tag());
+	return begin() + __offset;
+      }
 #endif
 
 #if __cplusplus >= 201103L
@@ -1783,6 +1807,10 @@ 
       void
       swap(deque& __x) _GLIBCXX_NOEXCEPT
       {
+#if __cplusplus >= 201103L
+	__glibcxx_assert(_Alloc_traits::propagate_on_container_swap::value
+			 || _M_get_Tp_allocator() == __x._M_get_Tp_allocator());
+#endif
 	_M_impl._M_swap_data(__x._M_impl);
 	_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
 				  __x._M_get_Tp_allocator());
@@ -1819,9 +1847,8 @@ 
         _M_initialize_dispatch(_InputIterator __first, _InputIterator __last,
 			       __false_type)
         {
-	  typedef typename std::iterator_traits<_InputIterator>::
-	    iterator_category _IterCategory;
-	  _M_range_initialize(__first, __last, _IterCategory());
+	  _M_range_initialize(__first, __last,
+			      std::__iterator_category(__first));
 	}
 
       // called by the second initialize_dispatch above
@@ -1884,11 +1911,7 @@ 
         void
         _M_assign_dispatch(_InputIterator __first, _InputIterator __last,
 			   __false_type)
-        {
-	  typedef typename std::iterator_traits<_InputIterator>::
-	    iterator_category _IterCategory;
-	  _M_assign_aux(__first, __last, _IterCategory());
-	}
+	{ _M_assign_aux(__first, __last, std::__iterator_category(__first)); }
 
       // called by the second assign_dispatch above
       template<typename _InputIterator>
@@ -1908,7 +1931,8 @@ 
 	      _ForwardIterator __mid = __first;
 	      std::advance(__mid, size());
 	      std::copy(__first, __mid, begin());
-	      insert(end(), __mid, __last);
+	      _M_range_insert_aux(end(), __mid, __last,
+				  std::__iterator_category(__first));
 	    }
 	  else
 	    _M_erase_at_end(std::copy(__first, __last, begin()));
@@ -1922,7 +1946,7 @@ 
 	if (__n > size())
 	  {
 	    std::fill(begin(), end(), __val);
-	    insert(end(), __n - size(), __val);
+	    _M_fill_insert(end(), __n - size(), __val);
 	  }
 	else
 	  {
@@ -1970,9 +1994,8 @@ 
 			   _InputIterator __first, _InputIterator __last,
 			   __false_type)
         {
-	  typedef typename std::iterator_traits<_InputIterator>::
-	    iterator_category _IterCategory;
-          _M_range_insert_aux(__pos, __first, __last, _IterCategory());
+          _M_range_insert_aux(__pos, __first, __last,
+			      std::__iterator_category(__first));
 	}
 
       // called by the second insert_dispatch above
@@ -2196,8 +2219,9 @@ 
 	  {
 	    // The rvalue's allocator cannot be moved and is not equal,
 	    // so we need to individually move each element.
-	    this->assign(std::__make_move_if_noexcept_iterator(__x.begin()),
-			 std::__make_move_if_noexcept_iterator(__x.end()));
+	    _M_assign_aux(std::__make_move_if_noexcept_iterator(__x.begin()),
+			  std::__make_move_if_noexcept_iterator(__x.end()),
+			  std::random_access_iterator_tag());
 	    __x.clear();
 	  }
       }
Index: include/bits/stl_vector.h
===================================================================
--- include/bits/stl_vector.h	(revision 237180)
+++ include/bits/stl_vector.h	(working copy)
@@ -63,6 +63,8 @@ 
 #include <initializer_list>
 #endif
 
+#include <debug/assertions.h>
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -320,7 +322,8 @@ 
       vector(const vector& __x)
       : _Base(__x.size(),
         _Alloc_traits::_S_select_on_copy(__x._M_get_Tp_allocator()))
-      { this->_M_impl._M_finish =
+      {
+	this->_M_impl._M_finish =
 	  std::__uninitialized_copy_a(__x.begin(), __x.end(),
 				      this->_M_impl._M_start,
 				      _M_get_Tp_allocator());
@@ -340,7 +343,8 @@ 
       /// Copy constructor with alternative allocator
       vector(const vector& __x, const allocator_type& __a)
       : _Base(__x.size(), __a)
-      { this->_M_impl._M_finish =
+      {
+	this->_M_impl._M_finish =
 	  std::__uninitialized_copy_a(__x.begin(), __x.end(),
 				      this->_M_impl._M_start,
 				      _M_get_Tp_allocator());
@@ -470,7 +474,8 @@ 
       vector&
       operator=(initializer_list<value_type> __l)
       {
-	this->assign(__l.begin(), __l.end());
+	this->_M_assign_aux(__l.begin(), __l.end(),
+			    random_access_iterator_tag());
 	return *this;
       }
 #endif
@@ -532,7 +537,10 @@ 
        */
       void
       assign(initializer_list<value_type> __l)
-      { this->assign(__l.begin(), __l.end()); }
+      {
+	this->_M_assign_aux(__l.begin(), __l.end(),
+			    random_access_iterator_tag());
+      }
 #endif
 
       /// Get a copy of the memory allocation object.
@@ -694,7 +702,7 @@ 
       resize(size_type __new_size, const value_type& __x)
       {
 	if (__new_size > size())
-	  insert(end(), __new_size - size(), __x);
+	  _M_fill_insert(end(), __new_size - size(), __x);
 	else if (__new_size < size())
 	  _M_erase_at_end(this->_M_impl._M_start + __new_size);
       }
@@ -714,7 +722,7 @@ 
       resize(size_type __new_size, value_type __x = value_type())
       {
 	if (__new_size > size())
-	  insert(end(), __new_size - size(), __x);
+	  _M_fill_insert(end(), __new_size - size(), __x);
 	else if (__new_size < size())
 	  _M_erase_at_end(this->_M_impl._M_start + __new_size);
       }
@@ -778,7 +786,10 @@ 
        */
       reference
       operator[](size_type __n) _GLIBCXX_NOEXCEPT
-      { return *(this->_M_impl._M_start + __n); }
+      {
+	__glibcxx_requires_subscript(__n);
+	return *(this->_M_impl._M_start + __n);
+      }
 
       /**
        *  @brief  Subscript access to the data contained in the %vector.
@@ -793,7 +804,10 @@ 
        */
       const_reference
       operator[](size_type __n) const _GLIBCXX_NOEXCEPT
-      { return *(this->_M_impl._M_start + __n); }
+      {
+	__glibcxx_requires_subscript(__n);
+	return *(this->_M_impl._M_start + __n);
+      }
 
     protected:
       /// Safety check used only from at().
@@ -850,7 +864,10 @@ 
        */
       reference
       front() _GLIBCXX_NOEXCEPT
-      { return *begin(); }
+      {
+	__glibcxx_requires_nonempty();
+	return *begin();
+      }
 
       /**
        *  Returns a read-only (constant) reference to the data at the first
@@ -858,7 +875,10 @@ 
        */
       const_reference
       front() const _GLIBCXX_NOEXCEPT
-      { return *begin(); }
+      {
+	__glibcxx_requires_nonempty();
+	return *begin();
+      }
 
       /**
        *  Returns a read/write reference to the data at the last
@@ -866,7 +886,10 @@ 
        */
       reference
       back() _GLIBCXX_NOEXCEPT
-      { return *(end() - 1); }
+      {
+	__glibcxx_requires_nonempty();
+	return *(end() - 1);
+      }
       
       /**
        *  Returns a read-only (constant) reference to the data at the
@@ -874,7 +897,10 @@ 
        */
       const_reference
       back() const _GLIBCXX_NOEXCEPT
-      { return *(end() - 1); }
+      {
+	__glibcxx_requires_nonempty();
+	return *(end() - 1);
+      }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // DR 464. Suggestion for new member functions in standard containers.
@@ -949,6 +975,7 @@ 
       void
       pop_back() _GLIBCXX_NOEXCEPT
       {
+	__glibcxx_requires_nonempty();
 	--this->_M_impl._M_finish;
 	_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
       }
@@ -1030,7 +1057,12 @@ 
        */
       iterator
       insert(const_iterator __position, initializer_list<value_type> __l)
-      { return this->insert(__position, __l.begin(), __l.end()); }
+      {
+	auto __offset = __position - cbegin();
+	_M_range_insert(begin() + __offset, __l.begin(), __l.end(),
+			std::random_access_iterator_tag());
+	return begin() + __offset;
+      }
 #endif
 
 #if __cplusplus >= 201103L
@@ -1194,6 +1226,10 @@ 
       void
       swap(vector& __x) _GLIBCXX_NOEXCEPT
       {
+#if __cplusplus >= 201103L
+	__glibcxx_assert(_Alloc_traits::propagate_on_container_swap::value
+			 || _M_get_Tp_allocator() == __x._M_get_Tp_allocator());
+#endif
 	this->_M_impl._M_swap_data(__x._M_impl);
 	_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
 	                          __x._M_get_Tp_allocator());
@@ -1328,11 +1364,7 @@ 
         void
         _M_assign_dispatch(_InputIterator __first, _InputIterator __last,
 			   __false_type)
-        {
-	  typedef typename std::iterator_traits<_InputIterator>::
-	    iterator_category _IterCategory;
-	  _M_assign_aux(__first, __last, _IterCategory());
-	}
+	{ _M_assign_aux(__first, __last, std::__iterator_category(__first)); }
 
       // Called by the second assign_dispatch above
       template<typename _InputIterator>
@@ -1351,7 +1383,6 @@ 
       void
       _M_fill_assign(size_type __n, const value_type& __val);
 
-
       // Internal insert functions follow.
 
       // Called by the range insert to implement [23.1.1]/9
@@ -1370,9 +1401,8 @@ 
         _M_insert_dispatch(iterator __pos, _InputIterator __first,
 			   _InputIterator __last, __false_type)
         {
-	  typedef typename std::iterator_traits<_InputIterator>::
-	    iterator_category _IterCategory;
-	  _M_range_insert(__pos, __first, __last, _IterCategory());
+	  _M_range_insert(__pos, __first, __last,
+			  std::__iterator_category(__first));
 	}
 
       // Called by the second insert_dispatch above
Index: include/bits/vector.tcc
===================================================================
--- include/bits/vector.tcc	(revision 237180)
+++ include/bits/vector.tcc	(working copy)
@@ -256,7 +256,8 @@ 
 	if (__first == __last)
 	  _M_erase_at_end(__cur);
 	else
-	  insert(end(), __first, __last);
+	  _M_range_insert(end(), __first, __last,
+			  std::__iterator_category(__first));
       }
 
   template<typename _Tp, typename _Alloc>
Index: include/debug/assertions.h
===================================================================
--- include/debug/assertions.h	(revision 237180)
+++ include/debug/assertions.h	(working copy)
@@ -33,10 +33,27 @@ 
 
 # define _GLIBCXX_DEBUG_ASSERT(_Condition)
 # define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
-# define _GLIBCXX_DEBUG_ONLY(_Statement) ;
+# define _GLIBCXX_DEBUG_ONLY(_Statement)
 
+#endif
+
+#ifndef _GLIBCXX_ASSERTIONS
+# define __glibcxx_requires_non_empty_range(_First,_Last)
+# define __glibcxx_requires_nonempty()
+# define __glibcxx_requires_subscript(_N)
 #else
 
+// Verify that [_First, _Last) forms a non-empty iterator range.
+# define __glibcxx_requires_non_empty_range(_First,_Last)	\
+  __glibcxx_assert(__builtin_expect(_First != _Last, true))
+# define __glibcxx_requires_subscript(_N)	\
+  __glibcxx_assert(__builtin_expect(_N < this->size(), true))
+// Verify that the container is nonempty
+# define __glibcxx_requires_nonempty()		\
+  __glibcxx_assert(__builtin_expect(!this->empty(), true))
+#endif
+
+#ifdef _GLIBCXX_DEBUG
 #define _GLIBCXX_DEBUG_ASSERT(_Condition) __glibcxx_assert(_Condition)
 
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
@@ -46,7 +63,6 @@ 
 #endif
 
 # define _GLIBCXX_DEBUG_ONLY(_Statement) _Statement
-
 #endif
 
 #endif // _GLIBCXX_DEBUG_ASSERTIONS
Index: include/debug/debug.h
===================================================================
--- include/debug/debug.h	(revision 237180)
+++ include/debug/debug.h	(working copy)
@@ -74,33 +74,18 @@ 
 # define __glibcxx_requires_heap_pred(_First,_Last,_Pred)
 # define __glibcxx_requires_string(_String)
 # define __glibcxx_requires_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N)
 # define __glibcxx_requires_irreflexive(_First,_Last)
 # define __glibcxx_requires_irreflexive2(_First,_Last)
 # define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred)
 # define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred)
 
-#ifdef _GLIBCXX_ASSERTIONS
-// Verify that [_First, _Last) forms a non-empty iterator range.
-# define __glibcxx_requires_non_empty_range(_First,_Last) \
-  __glibcxx_assert(_First != _Last)
-// Verify that the container is nonempty
-# define __glibcxx_requires_nonempty() \
-  __glibcxx_assert(! this->empty())
 #else
-# define __glibcxx_requires_non_empty_range(_First,_Last)
-# define __glibcxx_requires_nonempty()
-#endif
 
-#else
-
 # include <debug/macros.h>
 
 # 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_non_empty_range(_First,_Last)	\
-  __glibcxx_check_non_empty_range(_First,_Last)
 # define __glibcxx_requires_sorted(_First,_Last)	\
   __glibcxx_check_sorted(_First,_Last)
 # define __glibcxx_requires_sorted_pred(_First,_Last,_Pred)	\
@@ -121,11 +106,9 @@ 
   __glibcxx_check_heap(_First,_Last)
 # define __glibcxx_requires_heap_pred(_First,_Last,_Pred)	\
   __glibcxx_check_heap_pred(_First,_Last,_Pred)
-# define __glibcxx_requires_nonempty() __glibcxx_check_nonempty()
 # define __glibcxx_requires_string(_String) __glibcxx_check_string(_String)
 # define __glibcxx_requires_string_len(_String,_Len)	\
   __glibcxx_check_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N) __glibcxx_check_subscript(_N)
 # define __glibcxx_requires_irreflexive(_First,_Last)	\
   __glibcxx_check_irreflexive(_First,_Last)
 # define __glibcxx_requires_irreflexive2(_First,_Last)	\
Index: include/debug/helper_functions.h
===================================================================
--- include/debug/helper_functions.h	(revision 237180)
+++ include/debug/helper_functions.h	(working copy)
@@ -138,6 +138,7 @@ 
 	  return __dist.first >= 0;
 	}
 
+      // Can't tell so assume it is fine.
       return true;
     }