Re: Limit Debug mode impact: overload __niter_base

[François Dumont <frs dot dumont at gmail dot com>]

Here is the updated patch.

    * include/bits/stl_algobase.h (__niter_wrap): New.
    (__copy_move_a2(_II, _II, _OI)): Use latter.
    (__copy_move_backward_a2(_BI1, _BI1, _BI2)): Likewise.
    (fill_n(_OI, _Size, const _Tp&)): Likewise.
    (equal(_II1, _II1, _II2)): Use __glibcxx_requires_can_increment.
    * include/debug/stl_iterator.h
    (std::__niter_base(const __gnu_cxx::_Safe_iterator<
    __gnu_cxx::__normal_iterator<>, _Sequence>&)): New declaration.
    * include/debug/vector (__niter_base(const __gnu_cxx::_Safe_iterator<
    __gnu_cxx::__normal_iterator<>, _Sequence>&)): New.

Ok to commit ?


On 02/07/2018 13:57, Jonathan Wakely wrote:
> On 01/07/18 21:20 +0200, François Dumont wrote:
> >     Here is a new proposal between yours and mine.
> >
> >     It is still adding a function to wrap what __niter_base unwrap, I 
> > called it __nwrap_iter for this reason. But it takes advantage of 
>
> Since "niter" refers to __normal_iterator I think a name based on
> "niter" would be better than nsomething_iter.
>
> __niter_wrap
> __niter_rewrap
> __niter_lift (misuse of functional programming term?)
> __niter_raise (misuse of linear algebra term?)
> __make_niter
> __remake_niter
>
>
> > knowing that __niter_base will only unwrap random access iterator to 
> > use an expression to that will do the right thing, no matter the 
> > original iterator type.
>
> OK, since __niter_base only transforms types based on 
> __normal_iterator that seems safe to assume (in theory we could use
> __normal_iterator with non-random access iterators, but we don't).
>
> Could you please add a comment to the __nwrap_iter saying something
> like:
>
>  // Reverse the __niter_base transformation to get a
>  // __normal_iterator back again (this assumes that __normal_iterator
>  // is only used to wrap random access iterators, like pointers).
>
>
> >     I eventually found no issue in the testsuite, despite the 
> > std::equal change. I might have had a local invalid test.
>
> Yes, I *did* test it already :-)
>
>
>
>
> > diff --git a/libstdc++-v3/include/bits/stl_algobase.h 
> > b/libstdc++-v3/include/bits/stl_algobase.h
> > index d429943..003ae8d 100644
> > --- a/libstdc++-v3/include/bits/stl_algobase.h
> > +++ b/libstdc++-v3/include/bits/stl_algobase.h
> > @@ -277,6 +277,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >     __niter_base(_Iterator __it)
> >     { return __it; }
> >
> > +  // Convert an iterator of type _From to an iterator of type _To.
> > +  // e.g. from int* to __normal_iterator<int*, Seq>.
> > +  template<typename _Iterator>
> > +    inline _Iterator
> > +    __nwrap_iter(_Iterator, _Iterator, _Iterator __res)
> > +    { return __res; }
> > +
> > +  template<typename _From, typename _To>
> > +    inline _From
> > +    __nwrap_iter(_From __from, _To __to, _To __res)
> > +    { return __from + (__res - __to); }
>
> Every time you call this function you pass it:
>
>  __nwrap_iter(x, __niter_base(x), y)
>
> So can the __niter_base(x) call happen inside __nwrap_iter?
>
> i.e.
>
>  template<typename _From, typename _To>
>    inline _From
>    __nwrap_iter(_From __from, _To __res)
>    { return __from + (__res - __niter_base(__from)); }

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index d429943..e5e7d15 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -277,6 +277,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __niter_base(_Iterator __it)
     { return __it; }
 
+  // Reverse the __niter_base transformation to get a
+  // __normal_iterator back again (this assumes that __normal_iterator
+  // is only used to wrap random access iterators, like pointers).
+  template<typename _Iterator>
+    inline _Iterator
+    __niter_wrap(_Iterator, _Iterator __res)
+    { return __res; }
+
+  template<typename _From, typename _To>
+    inline _From
+    __niter_wrap(_From __from, _To __res)
+    { return __from + (__res - __niter_base(__from)); }
+
   // 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.)
@@ -419,9 +432,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline _OI
     __copy_move_a2(_II __first, _II __last, _OI __result)
     {
-      return _OI(std::__copy_move_a<_IsMove>(std::__niter_base(__first),
-					     std::__niter_base(__last),
-					     std::__niter_base(__result)));
+      return std::__niter_wrap(__result,
+		std::__copy_move_a<_IsMove>(std::__niter_base(__first),
+					    std::__niter_base(__last),
+					    std::__niter_base(__result)));
     }
 
   /**
@@ -593,7 +607,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline _BI2
     __copy_move_backward_a2(_BI1 __first, _BI1 __last, _BI2 __result)
     {
-      return _BI2(std::__copy_move_backward_a<_IsMove>
+      return std::__niter_wrap(__result,
+		std::__copy_move_backward_a<_IsMove>
 		  (std::__niter_base(__first), std::__niter_base(__last),
 		   std::__niter_base(__result)));
     }
@@ -804,7 +819,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_function_requires(_OutputIteratorConcept<_OI, _Tp>)
       __glibcxx_requires_can_increment(__first, __n);
 
-      return _OI(std::__fill_n_a(std::__niter_base(__first), __n, __value));
+      return std::__niter_wrap(__first,
+		std::__fill_n_a(std::__niter_base(__first), __n, __value));
     }
 
   template<bool _BoolType>
@@ -1062,7 +1078,7 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
       __glibcxx_function_requires(_EqualOpConcept<
 	    typename iterator_traits<_II1>::value_type,
 	    typename iterator_traits<_II2>::value_type>)
-      __glibcxx_requires_valid_range(__first1, __last1);
+      __glibcxx_requires_can_increment_range(__first1, __last1, __first2);
 
       return std::__equal_aux(std::__niter_base(__first1),
 			      std::__niter_base(__last1),
diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a76..f20b000 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -120,4 +120,17 @@ namespace __gnu_debug
 #endif
 }
 
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  template<typename _Iterator, typename _Container, typename _Sequence>
+    _Iterator
+    __niter_base(const __gnu_debug::_Safe_iterator<
+		 __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+		 _Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+
 #endif
diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index 802f4fd..ced5520 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -785,6 +785,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { return std::hash<_GLIBCXX_STD_C::vector<bool, _Alloc>>()(__b); }
     };
 
+ template<typename _Iterator, typename _Container, typename _Sequence>
+    _Iterator
+    __niter_base(const __gnu_debug::_Safe_iterator<
+		 __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+		 _Sequence>& __it)
+    { return std::__niter_base(__it.base()); }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 #endif