Hide move_iterator ill-form operators

[Jonathan Wakely]

On 06/05/19 19:36 +0200, François Dumont wrote:
>Hi
>
>Â Â Â  This is another attempt to make adapter iterator types operators 
>undefined when underlying iterator type doesn't support it. For the 
>move_iterator it is rather easy and even already done for the 
>operator- so I just generalize it to comparison operators. It doesn't 
>cover all operators of course but it is still better than current 
>situation.
>
>Â Â Â  * include/bits/stl_iterator.h (move_iterator<>::operator++(int)):
>Â Â Â  Simplify implementation using underlying iterator type same
>Â Â Â  post-increment operator.
>Â Â Â  (move_iterator<>::operator--(int)):
>Â Â Â  Simplify implementation using underlying iterator type same
>Â Â Â  post-decrement operator.
>Â Â Â  (move_iterator<>::operator<(const move_iterator<>&,
>Â Â Â  const move_iterator<>&): Define return type as return type of the same
>Â Â Â  expression on underlying iterator type.
>Â Â Â  (move_iterator<>::operator<=(const move_iterator<>&,
>Â Â Â  const move_iterator<>&): Likewise.
>Â Â Â  (move_iterator<>::operator>(const move_iterator<>&,
>Â Â Â  const move_iterator<>&): Likewise.
>Â Â Â  (move_iterator<>::operator>=(const move_iterator<>&,
>Â Â Â  const move_iterator<>&): Likewise.
>Â Â Â  * testsuite/24_iterators/move_iterator/operator_neg.cc: New.
>
>Â Â Â  Ok to commit or should the Standard be amended first ?

Not OK.

The C++2a draft already solves the same problem, but differently.
Please follow the draft standard, instead of inventing something
different.

>François

>diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
>index 47be1a9dbcd..c1bbc75ca43 100644
>--- a/libstdc++-v3/include/bits/stl_iterator.h
>+++ b/libstdc++-v3/include/bits/stl_iterator.h
>@@ -1121,11 +1121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
>       _GLIBCXX17_CONSTEXPR move_iterator
>       operator++(int)
>-      {
>-	move_iterator __tmp = *this;
>-	++_M_current;
>-	return __tmp;
>-      }
>+      { return move_iterator(_M_current++); }

This is not what C++2a says.

> 
>       _GLIBCXX17_CONSTEXPR move_iterator&
>       operator--()
>@@ -1136,11 +1132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
>       _GLIBCXX17_CONSTEXPR move_iterator
>       operator--(int)
>-      {
>-	move_iterator __tmp = *this;
>-	--_M_current;
>-	return __tmp;
>-      }
>+      { return move_iterator(_M_current--); }

This is not what C++2a says.

>       _GLIBCXX17_CONSTEXPR move_iterator
>       operator+(difference_type __n) const
>@@ -1197,51 +1189,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     { return !(__x == __y); }
> 
>   template<typename _IteratorL, typename _IteratorR>
>-    inline _GLIBCXX17_CONSTEXPR bool
>+    inline _GLIBCXX17_CONSTEXPR auto
>     operator<(const move_iterator<_IteratorL>& __x,
> 	      const move_iterator<_IteratorR>& __y)
>+    -> decltype( __x.base() < __y.base() )

This is wrong, it needs to return bool, e.g.

    -> decltype(bool(__x.base() < __y.base()))