public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* niter_base simplification
@ 2015-04-22 20:10 François Dumont
  2015-04-27 11:55 ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: François Dumont @ 2015-04-22 20:10 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

Hello

     I don't know if I am missing something but I think __niter_base 
could be simplified to remove usage of _Iter_base. Additionally I 
overload it to also remove __normal_iterator layer even if behind a 
reverse_iterator or move_iterator, might help compiler to optimize code, 
no ? If not, might allow other algo optimization in the future...

     I prefered to provide a __make_reverse_iterator to allow the latter 
in C++11 and not only in C++14. Is it fine to do it this way or do you 
prefer to simply get rid of all this part ?

     * include/bits/cpp_type_traits.h (__gnu_cxx::__normal_iterator): 
Delete.
     * include/bits/stl_algobase.h (std::__niter_base): Adapt.
     * include/bits/stl_iterator.h (__make_reverse_iterator): New in C++11.
     (std::__niter_base): Overloads for std::reverse_iterator,
     __gnu_cxx::__normal_iterator and std::move_iterator.

Tested under Linux x86_64. I checked that std::copy still ends up 
calling __builtin_memmove when used on vector iterators.

François


[-- Attachment #2: niter.patch --]
[-- Type: text/x-patch, Size: 4438 bytes --]

diff --git a/libstdc++-v3/include/bits/cpp_type_traits.h b/libstdc++-v3/include/bits/cpp_type_traits.h
index 8c6bb7f..2142917 100644
--- a/libstdc++-v3/include/bits/cpp_type_traits.h
+++ b/libstdc++-v3/include/bits/cpp_type_traits.h
@@ -64,17 +64,6 @@
 // removed.
 //
 
-// Forward declaration hack, should really include this from somewhere.
-namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
-{
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
-
-  template<typename _Iterator, typename _Container>
-    class __normal_iterator;
-
-_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace
-
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -331,24 +320,6 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
     };
 
   //
-  // Normal iterator type
-  //
-  template<typename _Tp>
-    struct __is_normal_iterator
-    {
-      enum { __value = 0 };
-      typedef __false_type __type;
-    };
-
-  template<typename _Iterator, typename _Container>
-    struct __is_normal_iterator< __gnu_cxx::__normal_iterator<_Iterator,
-							      _Container> >
-    {
-      enum { __value = 1 };
-      typedef __true_type __type;
-    };
-
-  //
   // An arithmetic type is an integer type or a floating point type
   //
   template<typename _Tp>
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 0bcb133..73eea6b 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -270,17 +270,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       return __a;
     }
 
-  // If _Iterator is a __normal_iterator return its base (a plain pointer,
-  // normally) otherwise return it untouched.  See copy, fill, ... 
+  // Fallback implementation of the function used to remove the
+  // __normal_iterator wrapper. See copy, fill, ...
   template<typename _Iterator>
-    struct _Niter_base
-    : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value>
-    { };
-
-  template<typename _Iterator>
-    inline typename _Niter_base<_Iterator>::iterator_type
+    inline _Iterator
     __niter_base(_Iterator __it)
-    { return std::_Niter_base<_Iterator>::_S_base(__it); }
+    { return __it; }
 
   // Likewise, for move_iterator.
   template<typename _Iterator>
diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 4a9189e..3aad9f3 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -390,7 +390,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return __y.base() - __x.base(); }
   //@}
 
-#if __cplusplus > 201103L
+#if __cplusplus == 201103L
+  template<typename _Iterator>
+    inline reverse_iterator<_Iterator>
+    __make_reverse_iterator(_Iterator __i)
+    { return reverse_iterator<_Iterator>(__i); }
+
+# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
+  std::__make_reverse_iterator(_Iter)
+#elif __cplusplus > 201103L
 #define __cpp_lib_make_reverse_iterator 201402
 
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
@@ -400,6 +408,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline reverse_iterator<_Iterator>
     make_reverse_iterator(_Iterator __i)
     { return reverse_iterator<_Iterator>(__i); }
+
+# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
+  std::make_reverse_iterator(_Iter)
+#endif
+
+#if __cplusplus >= 201103L
+  template<typename _Iterator>
+    auto
+    __niter_base(reverse_iterator<_Iterator> __it)
+    -> decltype(_GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())))
+    { return _GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())); }
 #endif
 
   // 24.4.2.2.1 back_insert_iterator
@@ -979,6 +998,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  template<typename _Iterator, typename _Container>
+    _Iterator
+    __niter_base(__gnu_cxx::__normal_iterator<_Iterator, _Container> __it)
+    { return __it.base(); }
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+
 #if __cplusplus >= 201103L
 
 namespace std _GLIBCXX_VISIBILITY(default)
@@ -1212,6 +1243,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // @} group iterators
 
+  template<typename _Iterator>
+    auto
+    __niter_base(move_iterator<_Iterator> __it)
+    -> decltype(make_move_iterator(__niter_base(__it.base())))
+    { return make_move_iterator(__niter_base(__it.base())); }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: niter_base simplification
  2015-04-22 20:10 niter_base simplification François Dumont
@ 2015-04-27 11:55 ` Jonathan Wakely
  2015-04-30  9:00   ` François Dumont
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2015-04-27 11:55 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 22/04/15 22:10 +0200, François Dumont wrote:
>Hello
>
>    I don't know if I am missing something but I think __niter_base 
>could be simplified to remove usage of _Iter_base. Additionally I 
>overload it to also remove __normal_iterator layer even if behind a 
>reverse_iterator or move_iterator, might help compiler to optimize 
>code, no ? If not, might allow other algo optimization in the 
>future...
>
>    I prefered to provide a __make_reverse_iterator to allow the 
>latter in C++11 and not only in C++14. Is it fine to do it this way or 
>do you prefer to simply get rid of all this part ?

It's fine to add __make_reverse_iterator but see my comment below.

>    * include/bits/cpp_type_traits.h (__gnu_cxx::__normal_iterator): 
>Delete.

You're removing __is_normal_iterator not __normal_iterator.

>    * include/bits/stl_algobase.h (std::__niter_base): Adapt.
>    * include/bits/stl_iterator.h (__make_reverse_iterator): New in C++11.
>    (std::__niter_base): Overloads for std::reverse_iterator,
>    __gnu_cxx::__normal_iterator and std::move_iterator.
>
>Tested under Linux x86_64. I checked that std::copy still ends up 
>calling __builtin_memmove when used on vector iterators.
>
>François
>

>diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
>index 0bcb133..73eea6b 100644
>--- a/libstdc++-v3/include/bits/stl_algobase.h
>+++ b/libstdc++-v3/include/bits/stl_algobase.h
>@@ -270,17 +270,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       return __a;
>     }
> 
>-  // If _Iterator is a __normal_iterator return its base (a plain pointer,
>-  // normally) otherwise return it untouched.  See copy, fill, ... 
>+  // Fallback implementation of the function used to remove the
>+  // __normal_iterator wrapper. See copy, fill, ...

It's a bit strange to have a function with no other overloads visible
described as a fallback. It would be good to say that the other
definition is in bits/stl_iterator.h

>   template<typename _Iterator>
>-    struct _Niter_base
>-    : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value>
>-    { };
>-
>-  template<typename _Iterator>
>-    inline typename _Niter_base<_Iterator>::iterator_type
>+    inline _Iterator
>     __niter_base(_Iterator __it)
>-    { return std::_Niter_base<_Iterator>::_S_base(__it); }
>+    { return __it; }
> 
>   // Likewise, for move_iterator.

This comment no longer makes sense, because you've removed the comment
on _Niter_base that it referred to. Please restore the original text
of the _Niter_base comment for _Miter_base.

(Alternatively, could the same simplification be made for
__miter_base? Do we need _Miter_base<> or just two overloads of
__miter_base()?)


>   template<typename _Iterator>
>diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
>index 4a9189e..3aad9f3 100644
>--- a/libstdc++-v3/include/bits/stl_iterator.h
>+++ b/libstdc++-v3/include/bits/stl_iterator.h
>@@ -390,7 +390,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     { return __y.base() - __x.base(); }
>   //@}
> 
>-#if __cplusplus > 201103L
>+#if __cplusplus == 201103L
>+  template<typename _Iterator>
>+    inline reverse_iterator<_Iterator>
>+    __make_reverse_iterator(_Iterator __i)
>+    { return reverse_iterator<_Iterator>(__i); }
>+
>+# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
>+  std::__make_reverse_iterator(_Iter)
>+#elif __cplusplus > 201103L
> #define __cpp_lib_make_reverse_iterator 201402
> 
>   // _GLIBCXX_RESOLVE_LIB_DEFECTS
>@@ -400,6 +408,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     inline reverse_iterator<_Iterator>
>     make_reverse_iterator(_Iterator __i)
>     { return reverse_iterator<_Iterator>(__i); }
>+
>+# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
>+  std::make_reverse_iterator(_Iter)
>+#endif
>+
>+#if __cplusplus >= 201103L
>+  template<typename _Iterator>
>+    auto
>+    __niter_base(reverse_iterator<_Iterator> __it)
>+    -> decltype(_GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())))
>+    { return _GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())); }
> #endif
> 

It might be simpler to just add __make_reverse_iterator for >= 201103L
and then always use std::__make_reverse_iterator instead of a macro.

That's similar to what we do for std:__addressof and std:addressof.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: niter_base simplification
  2015-04-27 11:55 ` Jonathan Wakely
@ 2015-04-30  9:00   ` François Dumont
  2015-04-30 11:29     ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: François Dumont @ 2015-04-30  9:00 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4712 bytes --]

On 27/04/2015 13:55, Jonathan Wakely wrote:
> On 22/04/15 22:10 +0200, François Dumont wrote:
>> Hello
>>
>>    I don't know if I am missing something but I think __niter_base 
>> could be simplified to remove usage of _Iter_base. Additionally I 
>> overload it to also remove __normal_iterator layer even if behind a 
>> reverse_iterator or move_iterator, might help compiler to optimize 
>> code, no ? If not, might allow other algo optimization in the future...
>>
>>    I prefered to provide a __make_reverse_iterator to allow the 
>> latter in C++11 and not only in C++14. Is it fine to do it this way 
>> or do you prefer to simply get rid of all this part ?
>
> It's fine to add __make_reverse_iterator but see my comment below.
>
>>    * include/bits/cpp_type_traits.h (__gnu_cxx::__normal_iterator): 
>> Delete.
>
> You're removing __is_normal_iterator not __normal_iterator.
>
>>    * include/bits/stl_algobase.h (std::__niter_base): Adapt.
>>    * include/bits/stl_iterator.h (__make_reverse_iterator): New in 
>> C++11.
>>    (std::__niter_base): Overloads for std::reverse_iterator,
>>    __gnu_cxx::__normal_iterator and std::move_iterator.
>>
>> Tested under Linux x86_64. I checked that std::copy still ends up 
>> calling __builtin_memmove when used on vector iterators.
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/bits/stl_algobase.h 
>> b/libstdc++-v3/include/bits/stl_algobase.h
>> index 0bcb133..73eea6b 100644
>> --- a/libstdc++-v3/include/bits/stl_algobase.h
>> +++ b/libstdc++-v3/include/bits/stl_algobase.h
>> @@ -270,17 +270,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       return __a;
>>     }
>>
>> -  // If _Iterator is a __normal_iterator return its base (a plain 
>> pointer,
>> -  // normally) otherwise return it untouched.  See copy, fill, ... 
>> +  // Fallback implementation of the function used to remove the
>> +  // __normal_iterator wrapper. See copy, fill, ...
>
> It's a bit strange to have a function with no other overloads visible
> described as a fallback. It would be good to say that the other
> definition is in bits/stl_iterator.h
>
>>   template<typename _Iterator>
>> -    struct _Niter_base
>> -    : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value>
>> -    { };
>> -
>> -  template<typename _Iterator>
>> -    inline typename _Niter_base<_Iterator>::iterator_type
>> +    inline _Iterator
>>     __niter_base(_Iterator __it)
>> -    { return std::_Niter_base<_Iterator>::_S_base(__it); }
>> +    { return __it; }
>>
>>   // Likewise, for move_iterator.
>
> This comment no longer makes sense, because you've removed the comment
> on _Niter_base that it referred to. Please restore the original text
> of the _Niter_base comment for _Miter_base.
>
> (Alternatively, could the same simplification be made for
> __miter_base? Do we need _Miter_base<> or just two overloads of
> __miter_base()?)

Definitely, I already have a patch for that.

>
>
>>   template<typename _Iterator>
>> diff --git a/libstdc++-v3/include/bits/stl_iterator.h 
>> b/libstdc++-v3/include/bits/stl_iterator.h
>> index 4a9189e..3aad9f3 100644
>> --- a/libstdc++-v3/include/bits/stl_iterator.h
>> +++ b/libstdc++-v3/include/bits/stl_iterator.h
>> @@ -390,7 +390,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>     { return __y.base() - __x.base(); }
>>   //@}
>>
>> -#if __cplusplus > 201103L
>> +#if __cplusplus == 201103L
>> +  template<typename _Iterator>
>> +    inline reverse_iterator<_Iterator>
>> +    __make_reverse_iterator(_Iterator __i)
>> +    { return reverse_iterator<_Iterator>(__i); }
>> +
>> +# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
>> +  std::__make_reverse_iterator(_Iter)
>> +#elif __cplusplus > 201103L
>> #define __cpp_lib_make_reverse_iterator 201402
>>
>>   // _GLIBCXX_RESOLVE_LIB_DEFECTS
>> @@ -400,6 +408,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>     inline reverse_iterator<_Iterator>
>>     make_reverse_iterator(_Iterator __i)
>>     { return reverse_iterator<_Iterator>(__i); }
>> +
>> +# define _GLIBCXX_MAKE_REVERSE_ITERATOR(_Iter) \
>> +  std::make_reverse_iterator(_Iter)
>> +#endif
>> +
>> +#if __cplusplus >= 201103L
>> +  template<typename _Iterator>
>> +    auto
>> +    __niter_base(reverse_iterator<_Iterator> __it)
>> +    -> 
>> decltype(_GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())))
>> +    { return 
>> _GLIBCXX_MAKE_REVERSE_ITERATOR(__niter_base(__it.base())); }
>> #endif
>>
>
> It might be simpler to just add __make_reverse_iterator for >= 201103L
> and then always use std::__make_reverse_iterator instead of a macro.
>
> That's similar to what we do for std:__addressof and std:addressof.
>
Ok, attached is the patch I have plan to commit then that I am testing 
at the moment.

François


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: niter.patch --]
[-- Type: text/x-patch; name="niter.patch", Size: 5239 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 222611)
+++ ChangeLog	(working copy)
@@ -1,5 +1,14 @@
 2015-04-30  François Dumont  <fdumont@gcc.gnu.org>
 
+	* include/bits/cpp_type_traits.h
+	(__gnu_cxx::__is_normal_iterator): Delete.
+	* include/bits/stl_algobase.h (std::__niter_base): Adapt.
+	* include/bits/stl_iterator.h (__make_reverse_iterator): New in C++11.
+	(std::__niter_base): Overloads for std::reverse_iterator,
+	__gnu_cxx::__normal_iterator and std::move_iterator.
+
+2015-04-30  François Dumont  <fdumont@gcc.gnu.org>
+
 	* include/bits/hashtable_policy.h (_Prime_rehash_policy::_S_n_primes):
 	Delete.
 	* src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt):
Index: include/bits/cpp_type_traits.h
===================================================================
--- include/bits/cpp_type_traits.h	(revision 222610)
+++ include/bits/cpp_type_traits.h	(working copy)
@@ -64,17 +64,6 @@
 // removed.
 //
 
-// Forward declaration hack, should really include this from somewhere.
-namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
-{
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
-
-  template<typename _Iterator, typename _Container>
-    class __normal_iterator;
-
-_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace
-
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -331,24 +320,6 @@
     };
 
   //
-  // Normal iterator type
-  //
-  template<typename _Tp>
-    struct __is_normal_iterator
-    {
-      enum { __value = 0 };
-      typedef __false_type __type;
-    };
-
-  template<typename _Iterator, typename _Container>
-    struct __is_normal_iterator< __gnu_cxx::__normal_iterator<_Iterator,
-							      _Container> >
-    {
-      enum { __value = 1 };
-      typedef __true_type __type;
-    };
-
-  //
   // An arithmetic type is an integer type or a floating point type
   //
   template<typename _Tp>
Index: include/bits/stl_algobase.h
===================================================================
--- include/bits/stl_algobase.h	(revision 222610)
+++ include/bits/stl_algobase.h	(working copy)
@@ -270,19 +270,15 @@
       return __a;
     }
 
-  // If _Iterator is a __normal_iterator return its base (a plain pointer,
-  // normally) otherwise return it untouched.  See copy, fill, ... 
+  // Fallback implementation of the function in bits/stl_iterator.h used to
+  // remove the __normal_iterator wrapper. See copy, fill, ...
   template<typename _Iterator>
-    struct _Niter_base
-    : _Iter_base<_Iterator, __is_normal_iterator<_Iterator>::__value>
-    { };
-
-  template<typename _Iterator>
-    inline typename _Niter_base<_Iterator>::iterator_type
+    inline _Iterator
     __niter_base(_Iterator __it)
-    { return std::_Niter_base<_Iterator>::_S_base(__it); }
+    { return __it; }
 
-  // Likewise, for move_iterator.
+  // If _Iterator is a move_iterator return its base otherwise return it
+  // untouched.  See copy, fill, ...
   template<typename _Iterator>
     struct _Miter_base
     : _Iter_base<_Iterator, __is_move_iterator<_Iterator>::__value>
Index: include/bits/stl_iterator.h
===================================================================
--- include/bits/stl_iterator.h	(revision 222610)
+++ include/bits/stl_iterator.h	(working copy)
@@ -388,9 +388,16 @@
     { return __y.base() - __x.base(); }
   //@}
 
-#if __cplusplus > 201103L
-#define __cpp_lib_make_reverse_iterator 201402
+#if __cplusplus >= 201103L
+  // Same as C++14 make_reverse_iterator but used in C++03 mode too.
+  template<typename _Iterator>
+    inline reverse_iterator<_Iterator>
+    __make_reverse_iterator(_Iterator __i)
+    { return reverse_iterator<_Iterator>(__i); }
 
+# if __cplusplus > 201103L
+#  define __cpp_lib_make_reverse_iterator 201402
+
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // DR 2285. make_reverse_iterator
   /// Generator function for reverse_iterator.
@@ -398,8 +405,17 @@
     inline reverse_iterator<_Iterator>
     make_reverse_iterator(_Iterator __i)
     { return reverse_iterator<_Iterator>(__i); }
+# endif
 #endif
 
+#if __cplusplus >= 201103L
+  template<typename _Iterator>
+    auto
+    __niter_base(reverse_iterator<_Iterator> __it)
+    -> decltype(__make_reverse_iterator(__niter_base(__it.base())))
+    { return __make_reverse_iterator(__niter_base(__it.base())); }
+#endif
+
   // 24.4.2.2.1 back_insert_iterator
   /**
    *  @brief  Turns assignment into insertion.
@@ -935,6 +951,18 @@
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  template<typename _Iterator, typename _Container>
+    _Iterator
+    __niter_base(__gnu_cxx::__normal_iterator<_Iterator, _Container> __it)
+    { return __it.base(); }
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+
 #if __cplusplus >= 201103L
 
 namespace std _GLIBCXX_VISIBILITY(default)
@@ -1168,6 +1196,12 @@
 
   // @} group iterators
 
+  template<typename _Iterator>
+    auto
+    __niter_base(move_iterator<_Iterator> __it)
+    -> decltype(make_move_iterator(__niter_base(__it.base())))
+    { return make_move_iterator(__niter_base(__it.base())); }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: niter_base simplification
  2015-04-30  9:00   ` François Dumont
@ 2015-04-30 11:29     ` Jonathan Wakely
  2015-05-03 20:19       ` miter_base simplification François Dumont
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2015-04-30 11:29 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 30/04/15 10:40 +0200, François Dumont wrote:
>On 27/04/2015 13:55, Jonathan Wakely wrote:
>>(Alternatively, could the same simplification be made for
>>__miter_base? Do we need _Miter_base<> or just two overloads of
>>__miter_base()?)
>
>Definitely, I already have a patch for that.

Great :-)

>>It might be simpler to just add __make_reverse_iterator for >= 201103L
>>and then always use std::__make_reverse_iterator instead of a macro.
>>
>>That's similar to what we do for std:__addressof and std:addressof.
>>
>Ok, attached is the patch I have plan to commit then that I am testing 
>at the moment.

Looks good, OK for trunk assuming the tests pass.

Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* miter_base simplification
  2015-04-30 11:29     ` Jonathan Wakely
@ 2015-05-03 20:19       ` François Dumont
  2015-05-19 20:47         ` François Dumont
  2015-05-20 10:04         ` Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: François Dumont @ 2015-05-03 20:19 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On 30/04/2015 13:18, Jonathan Wakely wrote:
> On 30/04/15 10:40 +0200, François Dumont wrote:
>> On 27/04/2015 13:55, Jonathan Wakely wrote:
>>> (Alternatively, could the same simplification be made for
>>> __miter_base? Do we need _Miter_base<> or just two overloads of
>>> __miter_base()?)
>>
>> Definitely, I already have a patch for that.
>
> Great :-)

And here is the patch for this part.

I have implemented it in such a way that it will also remove several 
layers of move_iterator.

2015-05-04  François Dumont  <fdumont@gcc.gnu.org>

     * include/bits/cpp_type_traits.h
     (std::move_iterator): Delete declaration.
     (std::__is_move_iterator<move_iterator>): Move partial 
specialization...
     * include/bits/stl_iterator.h: ... here.
     (std::__miter_base): Overloads for std::reverse_iterator and
     std::move_iterator.
     * include/bits/stl_algobase.h (std::__miter_base): Provide default
     implementation.

Tested under Linux x86_64.

Ok to commit ?

François


[-- Attachment #2: miter.patch --]
[-- Type: text/x-patch, Size: 2921 bytes --]

Index: include/bits/cpp_type_traits.h
===================================================================
--- include/bits/cpp_type_traits.h	(revision 222751)
+++ include/bits/cpp_type_traits.h	(working copy)
@@ -399,18 +399,6 @@
       typedef __false_type __type;
     };
 
-#if __cplusplus >= 201103L
-  template<typename _Iterator>
-    class move_iterator;
-
-  template<typename _Iterator>
-    struct __is_move_iterator< move_iterator<_Iterator> >
-    {
-      enum { __value = 1 };
-      typedef __true_type __type;
-    };
-#endif
-
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
Index: include/bits/stl_algobase.h
===================================================================
--- include/bits/stl_algobase.h	(revision 222751)
+++ include/bits/stl_algobase.h	(working copy)
@@ -277,17 +277,11 @@
     __niter_base(_Iterator __it)
     { return __it; }
 
-  // If _Iterator is a move_iterator return its base otherwise return it
-  // untouched.  See copy, fill, ...
+  // Likewise for move_iterator.
   template<typename _Iterator>
-    struct _Miter_base
-    : _Iter_base<_Iterator, __is_move_iterator<_Iterator>::__value>
-    { };
-
-  template<typename _Iterator>
-    inline typename _Miter_base<_Iterator>::iterator_type
+    inline _Iterator
     __miter_base(_Iterator __it)
-    { return std::_Miter_base<_Iterator>::_S_base(__it); }
+    { return __it; }
 
   // All of these auxiliary structs serve two purposes.  (1) Replace
   // calls to copy with memmove whenever possible.  (Memmove, not memcpy,
Index: include/bits/stl_iterator.h
===================================================================
--- include/bits/stl_iterator.h	(revision 222751)
+++ include/bits/stl_iterator.h	(working copy)
@@ -414,6 +414,17 @@
     __niter_base(reverse_iterator<_Iterator> __it)
     -> decltype(__make_reverse_iterator(__niter_base(__it.base())))
     { return __make_reverse_iterator(__niter_base(__it.base())); }
+
+  template<typename _Iterator>
+    struct __is_move_iterator<reverse_iterator<_Iterator> >
+      : __is_move_iterator<_Iterator>
+    { };
+
+  template<typename _Iterator>
+    auto
+    __miter_base(reverse_iterator<_Iterator> __it)
+    -> decltype(__make_reverse_iterator(__miter_base(__it.base())))
+    { return __make_reverse_iterator(__miter_base(__it.base())); }
 #endif
 
   // 24.4.2.2.1 back_insert_iterator
@@ -1202,6 +1213,19 @@
     -> decltype(make_move_iterator(__niter_base(__it.base())))
     { return make_move_iterator(__niter_base(__it.base())); }
 
+  template<typename _Iterator>
+    struct __is_move_iterator<move_iterator<_Iterator> >
+    {
+      enum { __value = 1 };
+      typedef __true_type __type;
+    };
+
+  template<typename _Iterator>
+    auto
+    __miter_base(move_iterator<_Iterator> __it)
+    -> decltype(__miter_base(__it.base()))
+    { return __miter_base(__it.base()); }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: miter_base simplification
  2015-05-03 20:19       ` miter_base simplification François Dumont
@ 2015-05-19 20:47         ` François Dumont
  2015-05-20 10:04         ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: François Dumont @ 2015-05-19 20:47 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 03/05/2015 22:19, François Dumont wrote:
> On 30/04/2015 13:18, Jonathan Wakely wrote:
>> On 30/04/15 10:40 +0200, François Dumont wrote:
>>> On 27/04/2015 13:55, Jonathan Wakely wrote:
>>>> (Alternatively, could the same simplification be made for
>>>> __miter_base? Do we need _Miter_base<> or just two overloads of
>>>> __miter_base()?)
>>>
>>> Definitely, I already have a patch for that.
>>
>> Great :-)
>
> And here is the patch for this part.
>
> I have implemented it in such a way that it will also remove several 
> layers of move_iterator.
>
> 2015-05-04  François Dumont  <fdumont@gcc.gnu.org>
>
>     * include/bits/cpp_type_traits.h
>     (std::move_iterator): Delete declaration.
>     (std::__is_move_iterator<move_iterator>): Move partial 
> specialization...
>     * include/bits/stl_iterator.h: ... here.
>     (std::__miter_base): Overloads for std::reverse_iterator and
>     std::move_iterator.
>     * include/bits/stl_algobase.h (std::__miter_base): Provide default
>     implementation.
>
> Tested under Linux x86_64.
>
> Ok to commit ?

Is it ok ?

François

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: miter_base simplification
  2015-05-03 20:19       ` miter_base simplification François Dumont
  2015-05-19 20:47         ` François Dumont
@ 2015-05-20 10:04         ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2015-05-20 10:04 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 03/05/15 22:19 +0200, François Dumont wrote:
>On 30/04/2015 13:18, Jonathan Wakely wrote:
>>On 30/04/15 10:40 +0200, François Dumont wrote:
>>>On 27/04/2015 13:55, Jonathan Wakely wrote:
>>>>(Alternatively, could the same simplification be made for
>>>>__miter_base? Do we need _Miter_base<> or just two overloads of
>>>>__miter_base()?)
>>>
>>>Definitely, I already have a patch for that.
>>
>>Great :-)
>
>And here is the patch for this part.
>
>I have implemented it in such a way that it will also remove several 
>layers of move_iterator.
>
>2015-05-04  François Dumont  <fdumont@gcc.gnu.org>
>
>    * include/bits/cpp_type_traits.h
>    (std::move_iterator): Delete declaration.
>    (std::__is_move_iterator<move_iterator>): Move partial 
>specialization...
>    * include/bits/stl_iterator.h: ... here.
>    (std::__miter_base): Overloads for std::reverse_iterator and
>    std::move_iterator.
>    * include/bits/stl_algobase.h (std::__miter_base): Provide default
>    implementation.
>
>Tested under Linux x86_64.
>
>Ok to commit ?

Yes OK, thanks (sorry for forgetting about this patch).

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-05-20 10:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-22 20:10 niter_base simplification François Dumont
2015-04-27 11:55 ` Jonathan Wakely
2015-04-30  9:00   ` François Dumont
2015-04-30 11:29     ` Jonathan Wakely
2015-05-03 20:19       ` miter_base simplification François Dumont
2015-05-19 20:47         ` François Dumont
2015-05-20 10:04         ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).