Re: Hide move_iterator ill-form operators

2019-05-06 Thread 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
-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()))



Hide move_iterator ill-form operators

2019-05-06 Thread François Dumont

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 ?

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++); }
 
   _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--); }
 
   _GLIBCXX17_CONSTEXPR move_iterator
   operator+(difference_type __n) const
@@ -1197,51 +1189,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 { return !(__x == __y); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator<(const move_iterator<_IteratorL>& __x,
 	  const move_iterator<_IteratorR>& __y)
+-> decltype( __x.base() < __y.base() )
 { return __x.base() < __y.base(); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator<(const move_iterator<_Iterator>& __x,
 	  const move_iterator<_Iterator>& __y)
+-> decltype( __x.base() < __y.base() )
 { return __x.base() < __y.base(); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator<=(const move_iterator<_IteratorL>& __x,
 	   const move_iterator<_IteratorR>& __y)
+-> decltype( __x.base() <= __y.base() )
 { return !(__y < __x); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator<=(const move_iterator<_Iterator>& __x,
 	   const move_iterator<_Iterator>& __y)
+-> decltype( __x.base() <= __y.base() )
 { return !(__y < __x); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator>(const move_iterator<_IteratorL>& __x,
 	  const move_iterator<_IteratorR>& __y)
+-> decltype( __x.base() > __y.base() )
 { return __y < __x; }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator>(const move_iterator<_Iterator>& __x,
 	  const move_iterator<_Iterator>& __y)
+-> decltype( __x.base() > __y.base() )
 { return __y < __x; }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator>=(const move_iterator<_IteratorL>& __x,
 	   const move_iterator<_IteratorR>& __y)
+-> decltype( __x.base() >= __y.base() )
 { return !(__x < __y); }
 
   template
-inline _GLIBCXX17_CONSTEXPR bool
+inline _GLIBCXX17_CONSTEXPR auto
 operator>=(const move_iterator<_Iterator>& __x,
 	   const move_iterator<_Iterator>& __y)
+-> decltype( __x.base() >= __y.base() )
 { return !(__x < __y); }
 
   // DR 685.
diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc
new file mode 100644
index 000..7b09425358e
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc
@@ -0,0 +1,73 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any