On 14/09/15 20:27 +0200, François Dumont wrote:
Hi
Here is what I had in mind when talking about moving debug checks to
the lightweight debug checks.
Ah yes, I hadn't thought about removing reundant checks from the
__gnu_debug containers, but that makes sense.
Sometimes the checks have been simply moved resulting in a simpler
debug vector implementation (front, back...). Sometimes I copy the
checks in a simpler form and kept the debug one too to make sure
execution of the debug code is fine.
I plan to do the same for other containers.
I still need to run tests, ok if tests are fine ?
François
diff --git a/libstdc++-v3/include/bits/stl_vector.h
b/libstdc++-v3/include/bits/stl_vector.h
index 305d446..89a9aec 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -449,6 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
vector&
operator=(vector&& __x) noexcept(_Alloc_traits::_S_nothrow_move())
{
+ __glibcxx_assert(this != &__x);
Please don't do this, it fails in valid programs. The standard needs
to be fixed in this regard.
constexpr bool __move_storage =
_Alloc_traits::_S_propagate_on_move_assign()
|| _Alloc_traits::_S_always_equal();
@@ -778,7 +779,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
operator[](size_type __n) _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_assert(__n < size());
+ return *(this->_M_impl._M_start + __n);
+ }
This could use __glibcxx_requires_subscript(__n), see the attached
patch.
/**
* @brief Subscript access to the data contained in the %vector.
@@ -793,7 +797,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
operator[](size_type __n) const _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_assert(__n < size());
+ return *(this->_M_impl._M_start + __n);
+ }
protected:
/// Safety check used only from at().
@@ -850,7 +857,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_assert(!empty());
This is __glibcxx_requires_nonempty(), already defined for
_GLIBCXX_ASSERTIONS.
@@ -1051,6 +1071,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
insert(const_iterator __position, size_type __n, const value_type& __x)
{
+ __glibcxx_assert(__position >= cbegin() && __position <= cend());
difference_type __offset = __position - cbegin();
_M_fill_insert(begin() + __offset, __n, __x);
return begin() + __offset;
This is undefined behaviour, so I'd rather not add this check (I know
it's on the google branch, but it's still undefined behaviour).
We could do it with std::less, I suppose.
I've attached the simplest checks I thought we should add for vector
and deque.
commit c2b5d263b7553074c82a721dc59b71a2a4a84436
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Thu Sep 10 14:23:43 2015 +0100
Add cheap assertions to std::vector and std::deque.
PR libstdc++/56109
* include/bits/stl_deque.h (deque::operator[], deque::front,
deque::back, deque::pop_front, deque::pop_back, deque::swap): Assert
preconditions.
* include/bits/stl_vector.h (vector::operator[], vector::front,
vector::back, vector::pop_back, vector::swap): Likewise.
* include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define
__glibcxx_requires_subscript.
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index e4fa6e3..fd38af9 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,14 @@
+2015-09-10 Jonathan Wakely <jwak...@redhat.com>
+
+ PR libstdc++/56109
+ * include/bits/stl_deque.h (deque::operator[], deque::front,
+ deque::back, deque::pop_front, deque::pop_back, deque::swap): Assert
+ preconditions.
+ * include/bits/stl_vector.h (vector::operator[], vector::front,
+ vector::back, vector::pop_back, vector::swap): Likewise.
+ * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define
+ __glibcxx_requires_subscript.
+
2015-09-09 Jonathan Wakely <jwak...@redhat.com>
* doc/xml/manual/using.xml (_GLIBCXX_ASSERTIONS): Document.
diff --git a/libstdc++-v3/include/bits/stl_deque.h
b/libstdc++-v3/include/bits/stl_deque.h
index f674245..3c8bb2e 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -1362,7 +1362,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.
@@ -1377,7 +1380,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().
@@ -1434,7 +1440,11 @@ _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
@@ -1442,7 +1452,11 @@ _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
@@ -1451,6 +1465,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
reference
back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
+
iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1463,6 +1479,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
const_reference
back() const _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
+
const_iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1546,6 +1564,8 @@ _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)
{
@@ -1568,6 +1588,8 @@ _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)
{
@@ -1781,6 +1803,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 305d446..9cffcb9 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -778,7 +778,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.
@@ -793,7 +796,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().
@@ -850,7 +856,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
@@ -858,7 +867,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
@@ -866,7 +878,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
@@ -874,7 +889,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.
@@ -949,6 +967,8 @@ _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);
}
@@ -1194,6 +1214,11 @@ _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/debug.h
b/libstdc++-v3/include/debug/debug.h
index b5935fe..d4ed4dc 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -84,12 +84,16 @@ namespace __gnu_debug
// 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
+// Verify that the container is nonempty.
# define __glibcxx_requires_nonempty() \
__glibcxx_assert(! this->empty())
+// Verify a subscript is in range.
+# define __glibcxx_requires_subscript(_N) \
+ __glibcxx_assert(_N < this->size())
#else
# define __glibcxx_requires_non_empty_range(_First,_Last)
# define __glibcxx_requires_nonempty()
+# define __glibcxx_requires_subscript(_N)
#endif
#else
