Hi
I finally generalized this method to other debug functions, it is
more consistent and clean the implementation of the debug checks. For
4.8 branch I will limit it to just what need to be really fixed.
2013-08-30 François Dumont <fdum...@gcc.gnu.org>
PR libstdc++/58191
* include/debug/macros.h (__glibcxx_check_partitioned_lower): Add
__gnu_debug::__base calls on iterators passed to internal debug
check.
(__glibcxx_check_partitioned_lower_pred): Likewise.
(__glibcxx_check_partitioned_upper): Likewise.
(__glibcxx_check_partitioned_upper_pred): Likewise.
(__glibcxx_check_sorted): Likewise.
(__glibcxx_check_sorted_pred): Likewise.
(__glibcxx_check_sorted_set): Likewise.
(__glibcxx_check_sorted_set_pred): Likewise.
* include/debug/functions.h (__check_partitioned_lower):
Remove code to detect safe iterators.
(__check_partitioned_upper): Likewise.
(__check_sorted): Likewise.
François
On 08/27/2013 11:08 PM, Paolo Carlini wrote:
On 08/27/2013 10:57 PM, François Dumont wrote:
Hi
Here is a patch to fix the small PR 58191 regression. I don't
remember why I hadn't used the __gnu_debug::__base from the start
rather than trying to reproduce its behavior within the
__check_partitioned* methods. This way we only detect random access
safe iterator to enhance performance but do not check the iterator
category otherwise, concept checks are there for that reason.
The patch is generated from the 4.8 branch. I still need to reg
test it but if it succeeded is it ok to commit ? Just in trunk or
also in 4.8 branch ?
Thanks. Let's play safe, let's apply it to mainline and let's ask
people on the audit trail of the bug too to test it. If everything
goes well let's backport it to the branch in a few weeks.
Thanks again,
Paolo.
Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h (revision 201966)
+++ include/debug/functions.h (working copy)
@@ -336,15 +336,6 @@
return true;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence>
- inline bool
- __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- std::random_access_iterator_tag __tag)
- { return __check_sorted_aux(__first.base(), __last.base(), __tag); }
-
// Can't check if an input iterator sequence is sorted, because we can't step
// through the sequence.
template<typename _InputIterator, typename _Predicate>
@@ -371,17 +362,6 @@
return true;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence,
- typename _Predicate>
- inline bool
- __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- _Predicate __pred,
- std::random_access_iterator_tag __tag)
- { return __check_sorted_aux(__first.base(), __last.base(), __pred, __tag); }
-
// Determine if a sequence is sorted.
template<typename _InputIterator>
inline bool
@@ -470,11 +450,13 @@
return __check_sorted_set_aux(__first, __last, __pred, _SameType());
}
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 270. Binary search requirements overly strict
+ // Determine if a sequence is partitioned w.r.t. this element.
template<typename _ForwardIterator, typename _Tp>
inline bool
- __check_partitioned_lower_aux(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value,
- std::forward_iterator_tag)
+ __check_partitioned_lower(_ForwardIterator __first,
+ _ForwardIterator __last, const _Tp& __value)
{
while (__first != __last && *__first < __value)
++__first;
@@ -487,38 +469,11 @@
return __first == __last;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence, typename _Tp>
- inline bool
- __check_partitioned_lower_aux(
- const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- const _Tp& __value,
- std::random_access_iterator_tag __tag)
- {
- return __check_partitioned_lower_aux(__first.base(), __last.base(),
- __value, __tag);
- }
-
- // _GLIBCXX_RESOLVE_LIB_DEFECTS
- // 270. Binary search requirements overly strict
- // Determine if a sequence is partitioned w.r.t. this element.
template<typename _ForwardIterator, typename _Tp>
inline bool
- __check_partitioned_lower(_ForwardIterator __first,
+ __check_partitioned_upper(_ForwardIterator __first,
_ForwardIterator __last, const _Tp& __value)
{
- return __check_partitioned_lower_aux(__first, __last, __value,
- std::__iterator_category(__first));
- }
-
- template<typename _ForwardIterator, typename _Tp>
- inline bool
- __check_partitioned_upper_aux(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value,
- std::forward_iterator_tag)
- {
while (__first != __last && !(__value < *__first))
++__first;
if (__first != __last)
@@ -530,35 +485,12 @@
return __first == __last;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence, typename _Tp>
- inline bool
- __check_partitioned_upper_aux(
- const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- const _Tp& __value,
- std::random_access_iterator_tag __tag)
- {
- return __check_partitioned_upper_aux(__first.base(), __last.base(),
- __value, __tag);
- }
-
- template<typename _ForwardIterator, typename _Tp>
- inline bool
- __check_partitioned_upper(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value)
- {
- return __check_partitioned_upper_aux(__first, __last, __value,
- std::__iterator_category(__first));
- }
-
+ // Determine if a sequence is partitioned w.r.t. this element.
template<typename _ForwardIterator, typename _Tp, typename _Pred>
inline bool
- __check_partitioned_lower_aux(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value,
- _Pred __pred,
- std::forward_iterator_tag)
+ __check_partitioned_lower(_ForwardIterator __first,
+ _ForwardIterator __last, const _Tp& __value,
+ _Pred __pred)
{
while (__first != __last && bool(__pred(*__first, __value)))
++__first;
@@ -571,39 +503,12 @@
return __first == __last;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence,
- typename _Tp, typename _Pred>
- inline bool
- __check_partitioned_lower_aux(
- const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- const _Tp& __value, _Pred __pred,
- std::random_access_iterator_tag __tag)
- {
- return __check_partitioned_lower_aux(__first.base(), __last.base(),
- __value, __pred, __tag);
- }
-
- // Determine if a sequence is partitioned w.r.t. this element.
template<typename _ForwardIterator, typename _Tp, typename _Pred>
inline bool
- __check_partitioned_lower(_ForwardIterator __first,
+ __check_partitioned_upper(_ForwardIterator __first,
_ForwardIterator __last, const _Tp& __value,
_Pred __pred)
{
- return __check_partitioned_lower_aux(__first, __last, __value, __pred,
- std::__iterator_category(__first));
- }
-
- template<typename _ForwardIterator, typename _Tp, typename _Pred>
- inline bool
- __check_partitioned_upper_aux(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value,
- _Pred __pred,
- std::forward_iterator_tag)
- {
while (__first != __last && !bool(__pred(__value, *__first)))
++__first;
if (__first != __last)
@@ -615,31 +520,6 @@
return __first == __last;
}
- // For performance reason, as the iterator range has been validated, check on
- // random access safe iterators is done using the base iterator.
- template<typename _Iterator, typename _Sequence,
- typename _Tp, typename _Pred>
- inline bool
- __check_partitioned_upper_aux(
- const _Safe_iterator<_Iterator, _Sequence>& __first,
- const _Safe_iterator<_Iterator, _Sequence>& __last,
- const _Tp& __value, _Pred __pred,
- std::random_access_iterator_tag __tag)
- {
- return __check_partitioned_upper_aux(__first.base(), __last.base(),
- __value, __pred, __tag);
- }
-
- template<typename _ForwardIterator, typename _Tp, typename _Pred>
- inline bool
- __check_partitioned_upper(_ForwardIterator __first,
- _ForwardIterator __last, const _Tp& __value,
- _Pred __pred)
- {
- return __check_partitioned_upper_aux(__first, __last, __value, __pred,
- std::__iterator_category(__first));
- }
-
// Helper struct to detect random access safe iterators.
template<typename _Iterator>
struct __is_safe_random_iterator
Index: include/debug/macros.h
===================================================================
--- include/debug/macros.h (revision 201966)
+++ include/debug/macros.h (working copy)
@@ -229,7 +229,9 @@
// Verify that the iterator range [_First, _Last) is sorted
#define __glibcxx_check_sorted(_First,_Last) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last), \
+ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last)), \
_M_message(__gnu_debug::__msg_unsorted) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last))
@@ -238,7 +240,9 @@
predicate _Pred. */
#define __glibcxx_check_sorted_pred(_First,_Last,_Pred) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last), _Pred), \
_M_message(__gnu_debug::__msg_unsorted_pred) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last) \
@@ -248,7 +252,8 @@
#define __glibcxx_check_sorted_set(_First1,_Last1,_First2) \
__glibcxx_check_valid_range(_First1,_Last1); \
_GLIBCXX_DEBUG_VERIFY( \
- __gnu_debug::__check_sorted_set(_First1, _Last1, _First2), \
+ __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \
+ __gnu_debug::__base(_Last1), _First2),\
_M_message(__gnu_debug::__msg_unsorted) \
._M_iterator(_First1, #_First1) \
._M_iterator(_Last1, #_Last1))
@@ -257,7 +262,9 @@
#define __glibcxx_check_sorted_set_pred(_First1,_Last1,_First2,_Pred) \
__glibcxx_check_valid_range(_First1,_Last1); \
_GLIBCXX_DEBUG_VERIFY( \
- __gnu_debug::__check_sorted_set(_First1, _Last1, _First2, _Pred), \
+ __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \
+ __gnu_debug::__base(_Last1), \
+ _First2, _Pred), \
_M_message(__gnu_debug::__msg_unsorted_pred) \
._M_iterator(_First1, #_First1) \
._M_iterator(_Last1, #_Last1) \
@@ -267,8 +274,9 @@
w.r.t. the value _Value. */
#define __glibcxx_check_partitioned_lower(_First,_Last,_Value) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \
- _Value), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last), _Value), \
_M_message(__gnu_debug::__msg_unpartitioned) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last) \
@@ -276,8 +284,9 @@
#define __glibcxx_check_partitioned_upper(_First,_Last,_Value) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \
- _Value), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last), _Value), \
_M_message(__gnu_debug::__msg_unpartitioned) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last) \
@@ -287,8 +296,9 @@
w.r.t. the value _Value and predicate _Pred. */
#define __glibcxx_check_partitioned_lower_pred(_First,_Last,_Value,_Pred) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \
- _Value, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last), _Value, _Pred), \
_M_message(__gnu_debug::__msg_unpartitioned_pred) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last) \
@@ -299,8 +309,9 @@
w.r.t. the value _Value and predicate _Pred. */
#define __glibcxx_check_partitioned_upper_pred(_First,_Last,_Value,_Pred) \
__glibcxx_check_valid_range(_First,_Last); \
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \
- _Value, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \
+ __gnu_debug::__base(_First), \
+ __gnu_debug::__base(_Last), _Value, _Pred), \
_M_message(__gnu_debug::__msg_unpartitioned_pred) \
._M_iterator(_First, #_First) \
._M_iterator(_Last, #_Last) \
