Re: unsafe STL patch

[François Dumont <francois dot cppdevs at free dot fr>]

Hi

   Here is my new proposition, is it what you were expecting ?

   However it do not compile. When I run tests g++ complains about not 
knowing the __iter_base struct defined in bits/stl_algobase.h. I might 
be missing something obvious, do you see it ? I wonder if I have not a 
problem of pch. Is there any when building tests ? How to regenerate 
them or disable this feature ? It would also help me if you could tell 
me how to build/run tests from the testsuite_files I have generated 
without running the abi check ?

   Do you have a link to the FSF form ?

Bests

Paolo Carlini wrote:
> Hi,
>   
> > Hello
> >
> >    This is a patch proposition to:
> > - Limit impact on STL safe mode on performance
> > - Limit template method instanciation
> >
> >    My point is that once an iterator range has been validated thanks
> > to the check_valid_range or any similar function it is useless to keep
> > the safe wrapper when passing it to the normal container
> > implementation. Of course the safe wrapper can only be removed if the
> > check_valid_range has been able to validate that begin is before end
> > which is only possible for random access iterator.
> >     
> The idea makes sense.
>
> I think it's becoming rather clear that a mechanism like your __unsafe
> is rather useful in general: in stl_algobase.h I'm guilty to have
> duplicated it two times in __miter_base and __niter_base. Are you
> willing to figure out first an abstraction for that?
>   
> >    The patch is limited to the impact on the list safe implementation.
> > If you find an interest in this patch I will generalize it to other
> > safe containers, complete ChangeLog and perhaps fill a FSF form.
> >     
> The conditional seems a bit strange to me: if the premise holds, then
> the consequent should not have that <perhaps> in it ;)
>
> Paolo.
>   

Index: debug/functions.h
===================================================================
--- debug/functions.h	(révision 156331)
+++ debug/functions.h	(copie de travail)
@@ -34,6 +34,7 @@
 #include <cstddef>                       // for ptrdiff_t
 #include <bits/stl_iterator_base_types.h> // for iterator_traits, categories
 #include <bits/cpp_type_traits.h>         // for __is_integer
+#include <bits/stl_algobase.h>            // for __iter_base
 
 namespace __gnu_debug
 {
@@ -378,6 +379,24 @@
 	++__first;
       return __first == __last;
     }
+
+  template<typename _Iterator>
+    struct __is_safe_random_iterator
+    {
+      enum { __value = 0 };
+      typedef std::__false_type __type;
+    };
+
+  template<typename _Iterator, typename _Sequence>
+    struct __is_safe_random_iterator<_Safe_iterator<_Iterator, _Sequence> >
+      : std::__are_same<std::random_access_iterator_tag,
+                        typename std::iterator_traits<_Iterator>::iterator_category>
+    {};
+
+  template<typename _Iterator>
+    struct __siter_base
+      : std::__iter_base<_Iterator, __is_safe_random_iterator<_Iterator>::__value>
+    {};
 } // namespace __gnu_debug
 
 #endif
Index: debug/list
===================================================================
--- debug/list	(révision 156331)
+++ debug/list	(copie de travail)
@@ -78,7 +78,9 @@
       template<class _InputIterator>
       list(_InputIterator __first, _InputIterator __last,
 	   const _Allocator& __a = _Allocator())
-      : _Base(__gnu_debug::__check_valid_range(__first, __last), __last, __a)
+      : _Base(__gnu_debug::__siter_base<_InputIterator>::__b(
+                __gnu_debug::__check_valid_range(__first, __last)),
+              __gnu_debug::__siter_base<_InputIterator>::__b(__last), __a)
       { }
 
 
@@ -139,8 +141,9 @@
         void
         assign(_InputIterator __first, _InputIterator __last)
         {
-	  __glibcxx_check_valid_range(__first, __last);
-	  _Base::assign(__first, __last);
+          __glibcxx_check_valid_range(__first, __last);
+          _Base::assign(__gnu_debug::__siter_base<_InputIterator>::__b(__first),
+                        __gnu_debug::__siter_base<_InputIterator>::__b(__last));
 	  this->_M_invalidate_all();
 	}
 
@@ -341,9 +344,11 @@
         insert(iterator __position, _InputIterator __first,
 	       _InputIterator __last)
         {
-	  __glibcxx_check_insert_range(__position, __first, __last);
-	  _Base::insert(__position.base(), __first, __last);
-	}
+          __glibcxx_check_insert_range(__position, __first, __last);
+          _Base::insert(__position.base(),
+                        __gnu_debug::__siter_base<_InputIterator>::__b(__first),
+                        __gnu_debug::__siter_base<_InputIterator>::__b(__last));
+        }
 
       iterator
       erase(iterator __position)
Index: bits/stl_algobase.h
===================================================================
--- bits/stl_algobase.h	(révision 156331)
+++ bits/stl_algobase.h	(copie de travail)
@@ -259,11 +259,10 @@
     }
 
 
-  // If _Iterator is a __normal_iterator return its base (a plain pointer,
-  // normally) otherwise return it untouched.  See copy, fill, ... 
-  template<typename _Iterator,
-	   bool _IsNormal = __is_normal_iterator<_Iterator>::__value>
-    struct __niter_base
+  // If _Iterator has a base returns it otherwise _Iterator is returned
+  // untouched
+  template<typename _Iterator, bool _HasBase>
+    struct __iter_base
     {
       static _Iterator
       __b(_Iterator __it)
@@ -271,31 +270,26 @@
     };
 
   template<typename _Iterator>
-    struct __niter_base<_Iterator, true>
+    struct __iter_base<_Iterator, true>
     {
       static typename _Iterator::iterator_type
       __b(_Iterator __it)
       { return __it.base(); }
     };
 
+  // If _Iterator is a __normal_iterator return its base (a plain pointer,
+  // normally) otherwise return it untouched.  See copy, fill, ... 
+  template<typename _Iterator>
+    struct __niter_base
+      : __iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value>
+    {};
+
   // Likewise, for move_iterator.
-  template<typename _Iterator,
-	   bool _IsMove = __is_move_iterator<_Iterator>::__value>
+  template<typename _Iterator>
     struct __miter_base
-    {
-      static _Iterator
-      __b(_Iterator __it)
-      { return __it; }
-    };
+      : __iter_base<_Iterator, __is_move_iterator<_Iterator>::__value>
+    {};
 
-  template<typename _Iterator>
-    struct __miter_base<_Iterator, true>
-    {
-      static typename _Iterator::iterator_type
-      __b(_Iterator __it)
-      { return __it.base(); }
-    };
-
   // All of these auxiliary structs serve two purposes.  (1) Replace
   // calls to copy with memmove whenever possible.  (Memmove, not memcpy,
   // because the input and output ranges are permitted to overlap.)