* Container debug light mode
@ 2016-06-08 20:54 François Dumont
2016-06-13 10:21 ` Jonathan Wakely
0 siblings, 1 reply; 4+ messages in thread
From: François Dumont @ 2016-06-08 20:54 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]
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
[-- Attachment #2: cont_debug_light.patch --]
[-- Type: text/x-patch, Size: 18142 bytes --]
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Container debug light mode
2016-06-08 20:54 Container debug light mode François Dumont
@ 2016-06-13 10:21 ` Jonathan Wakely
2016-06-16 19:28 ` François Dumont
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2016-06-13 10:21 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Container debug light mode
2016-06-13 10:21 ` Jonathan Wakely
@ 2016-06-16 19:28 ` François Dumont
2016-06-16 20:19 ` Jonathan Wakely
0 siblings, 1 reply; 4+ messages in thread
From: François Dumont @ 2016-06-16 19:28 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]
And here is the patch to only add light debug checks to vector and deque.
* 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.
* 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.
Tested under Linux x86_64.
François
On 13/06/2016 12:21, Jonathan Wakely wrote:
> 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.
>
>
[-- Attachment #2: cont_debug_light.patch --]
[-- Type: text/x-patch, Size: 10583 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
index f63ae4c..66b8da6 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -63,6 +63,8 @@
#include <initializer_list>
#endif
+#include <debug/assertions.h>
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -1365,7 +1367,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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.
@@ -1380,7 +1385,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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().
@@ -1437,7 +1445,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -1445,7 +1456,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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
@@ -1454,6 +1468,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
reference
back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1466,6 +1481,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
const_reference
back() const _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
const_iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1549,6 +1565,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_front() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
if (this->_M_impl._M_start._M_cur
!= this->_M_impl._M_start._M_last - 1)
{
@@ -1571,6 +1588,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
if (this->_M_impl._M_finish._M_cur
!= this->_M_impl._M_finish._M_first)
{
@@ -1789,6 +1807,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
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());
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 8badea3..e2c42cb 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -63,6 +63,8 @@
#include <initializer_list>
#endif
+#include <debug/assertions.h>
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -784,7 +786,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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.
@@ -799,7 +804,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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().
@@ -856,7 +864,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -864,7 +875,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read/write reference to the data at the last
@@ -872,7 +886,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
back() _GLIBCXX_NOEXCEPT
- { return *(end() - 1); }
+ {
+ __glibcxx_requires_nonempty();
+ return *(end() - 1);
+ }
/**
* Returns a read-only (constant) reference to the data at the
@@ -880,7 +897,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
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.
@@ -955,6 +975,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
--this->_M_impl._M_finish;
_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
}
@@ -1205,6 +1226,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
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());
diff --git a/libstdc++-v3/include/debug/assertions.h b/libstdc++-v3/include/debug/assertions.h
index 802645c..3708d12 100644
--- a/libstdc++-v3/include/debug/assertions.h
+++ b/libstdc++-v3/include/debug/assertions.h
@@ -33,20 +33,36 @@
# define _GLIBCXX_DEBUG_ASSERT(_Condition)
# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
-# define _GLIBCXX_DEBUG_ONLY(_Statement) ;
+# define _GLIBCXX_DEBUG_ONLY(_Statement)
-#else
-
-#define _GLIBCXX_DEBUG_ASSERT(_Condition) __glibcxx_assert(_Condition)
+#endif
-#ifdef _GLIBCXX_DEBUG_PEDANTIC
-# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+#ifndef _GLIBCXX_ASSERTIONS
+# define __glibcxx_requires_non_empty_range(_First,_Last)
+# define __glibcxx_requires_nonempty()
+# define __glibcxx_requires_subscript(_N)
#else
-# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+
+// 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
-# define _GLIBCXX_DEBUG_ONLY(_Statement) _Statement
+#ifdef _GLIBCXX_DEBUG
+# define _GLIBCXX_DEBUG_ASSERT(_Condition) __glibcxx_assert(_Condition)
+# ifdef _GLIBCXX_DEBUG_PEDANTIC
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+# else
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+# endif
+
+# define _GLIBCXX_DEBUG_ONLY(_Statement) _Statement
#endif
#endif // _GLIBCXX_DEBUG_ASSERTIONS
diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index 3d46b6d..79fe00d 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -74,24 +74,11 @@ namespace __gnu_debug
# 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>
@@ -99,8 +86,6 @@ 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_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 @@ namespace __gnu_debug
__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) \
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index 5ee33e8..f1a74eb 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -138,6 +138,7 @@ namespace __gnu_debug
return __dist.first >= 0;
}
+ // Can't tell so assume it is fine.
return true;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Container debug light mode
2016-06-16 19:28 ` François Dumont
@ 2016-06-16 20:19 ` Jonathan Wakely
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2016-06-16 20:19 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 16/06/16 21:28 +0200, François Dumont wrote:
>And here is the patch to only add light debug checks to vector and deque.
Excellent, thanks - this is OK for trunk.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-16 20:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 20:54 Container debug light mode François Dumont
2016-06-13 10:21 ` Jonathan Wakely
2016-06-16 19:28 ` François Dumont
2016-06-16 20:19 ` 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).