This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Limit Debug mode impact: overload __niter_base


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]