Limit Debug mode impact: overload __niter_base

[Jonathan Wakely]

On 03/07/18 07:47 +0200, François Dumont wrote:
>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).

Please move the comment onto the other overload, since that's the one
that actually does the wrapping.

Maybe we should re-order the overloads, so the __niter_wrap<_From, _To>
overload comes first, then on the second one add a comment:

    // No need to wrap, iterator already has the right type

>+  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)); }

Please qualify this call as std::__niter_base

OK with those changes, thanks.