diff mbox

Debug functions review

Message ID 5266E4D0.9010906@gmail.com
State New
Headers show

Commit Message

François Dumont Oct. 22, 2013, 8:49 p.m. UTC 
Hi

     Here is a patch to clean up a little some debug functions. I got 
rid of the __check_singular_aux, simply playing with __check_singular 
overloads was enough. I also added the missing __check_dereferenceable 
for safe local iterators.

2013-10-22  François Dumont <fdumont@gcc.gnu.org>

     * include/debug/formatter.h (__check_singular): Add const on
     iterator reference.
     * include/debug/functions.h (__check_singular_aux): Delete.
     (__check_singular(const _Ite&)): Add const on iterator reference.
     (__check_singular(const _Safe_iterator<_Ite, _Seq>&)): Delete.
     (__check_dereferenceable(const _Ite&)): Add const on iterator
     reference.
     (__check_dereferenceable(const _Safe_local_iterator<>&)): New.
     * include/debug/safe_iterator.h (__check_singular_aux): Delete.
     (__check_singular(const _Safe_iterator_base&)): New.

Tested under Linux x86_64 debug mode.

Ok to commit ?

François

Comments

Paolo Carlini Oct. 22, 2013, 10:37 p.m. UTC | #1 
Hi,

"François Dumont" <frs.dumont@gmail.com> ha scritto:
>Hi
>
>     Here is a patch to clean up a little some debug functions. I got
>rid of the __check_singular_aux, simply playing with __check_singular
>overloads was enough. I also added the missing __check_dereferenceable
>for safe local iterators.

This is probably straightforward but I want to be sure I understand your previous message + this one: do they mean that in some cases, due to that missing 'const', we weren't catching non-dereferenceable iterators? Thus, should we also add a testcase?

Paolo
François Dumont Oct. 23, 2013, 9:22 p.m. UTC | #2 
On 10/23/2013 12:37 AM, Paolo Carlini wrote:
>
> Hi,
>
> "François Dumont" <frs.dumont@gmail.com> ha scritto:
>> Hi
>>
>>      Here is a patch to clean up a little some debug functions. I got
>> rid of the __check_singular_aux, simply playing with __check_singular
>> overloads was enough. I also added the missing __check_dereferenceable
>> for safe local iterators.
> This is probably straightforward but I want to be sure I understand your previous message + this one: do they mean that in some cases, due to that missing 'const', we weren't catching non-dereferenceable iterators? Thus, should we also add a testcase?
>
> Paolo
>
     You are right, I am preparing a test case. However you have to know 
that __check_dereferenceable is simply not used for the moment. It is 
only because I have started using it for a debug mode evolution that I 
discovered the issue.

François
Paolo Carlini Oct. 23, 2013, 10:12 p.m. UTC | #3 
On 10/23/2013 11:22 PM, François Dumont wrote:
> You are right, I am preparing a test case. However you have to know 
> that __check_dereferenceable is simply not used for the moment. It is 
> only because I have started using it for a debug mode evolution that I 
> discovered the issue.
Ok, thanks. Now however I'm curious to know the story of 
__check_dereferenceable: dates back to when Doug added debug-mode and 
never used since? Any idea?

Paolo.
diff mbox

Patch

Index: include/debug/formatter.h
===================================================================
--- include/debug/formatter.h	(revision 203909)
+++ include/debug/formatter.h	(working copy)
@@ -38,7 +38,7 @@ 
   using std::type_info;
 
   template<typename _Iterator>
-    bool __check_singular(_Iterator&);
+    bool __check_singular(const _Iterator&);
 
   class _Safe_sequence_base;
 
Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h	(revision 203909)
+++ include/debug/functions.h	(working copy)
@@ -45,20 +45,19 @@ 
   template<typename _Iterator, typename _Sequence>
     class _Safe_iterator;
 
+  template<typename _Iterator, typename _Sequence>
+    class _Safe_local_iterator;
+
   template<typename _Sequence>
     struct _Insert_range_from_self_is_safe
     { enum { __value = 0 }; };
 
-  // An arbitrary iterator pointer is not singular.
-  inline bool
-  __check_singular_aux(const void*) { return false; }
-
-  // We may have an iterator that derives from _Safe_iterator_base but isn't
-  // a _Safe_iterator.
+  /** Assume that some arbitrary iterator is not singular, because we
+      can't prove that it is. */
   template<typename _Iterator>
     inline bool
-    __check_singular(_Iterator& __x)
-    { return __check_singular_aux(&__x); }
+    __check_singular(const _Iterator& __x)
+    { return false; }
 
   /** Non-NULL pointers are nonsingular. */
   template<typename _Tp>
@@ -66,17 +65,11 @@ 
     __check_singular(const _Tp* __ptr)
     { return __ptr == 0; }
 
-  /** Safe iterators know if they are singular. */
-  template<typename _Iterator, typename _Sequence>
-    inline bool
-    __check_singular(const _Safe_iterator<_Iterator, _Sequence>& __x)
-    { return __x._M_singular(); }
-
   /** Assume that some arbitrary iterator is dereferenceable, because we
       can't prove that it isn't. */
   template<typename _Iterator>
     inline bool
-    __check_dereferenceable(_Iterator&)
+    __check_dereferenceable(const _Iterator&)
     { return true; }
 
   /** Non-NULL pointers are dereferenceable. */
@@ -85,12 +78,19 @@ 
     __check_dereferenceable(const _Tp* __ptr)
     { return __ptr; }
 
-  /** Safe iterators know if they are singular. */
+  /** Safe iterators know if they are dereferenceable. */
   template<typename _Iterator, typename _Sequence>
     inline bool
     __check_dereferenceable(const _Safe_iterator<_Iterator, _Sequence>& __x)
     { return __x._M_dereferenceable(); }
 
+  /** Safe local iterators know if they are dereferenceable. */
+  template<typename _Iterator, typename _Sequence>
+    inline bool
+    __check_dereferenceable(const _Safe_local_iterator<_Iterator,
+						       _Sequence>& __x)
+    { return __x._M_dereferenceable(); }
+
   /** If the distance between two random access iterators is
    *  nonnegative, assume the range is valid.
   */
Index: include/debug/safe_iterator.h
===================================================================
--- include/debug/safe_iterator.h	(revision 203909)
+++ include/debug/safe_iterator.h	(working copy)
@@ -56,13 +56,10 @@ 
       { return __it == __seq->_M_base().begin(); }
     };
 
-  /** Iterators that derive from _Safe_iterator_base but that aren't
-   *  _Safe_iterators can be determined singular or non-singular via
-   *  _Safe_iterator_base.
-   */
-  inline bool 
-  __check_singular_aux(const _Safe_iterator_base* __x)
-  { return __x->_M_singular(); }
+  /** _Safe_iterators can be determined singular or non-singular. */
+  inline bool
+  __check_singular(const _Safe_iterator_base& __x)
+  { return __x._M_singular(); }
 
   /** The precision to which we can calculate the distance between
    *  two iterators.