libstdc++: Fix std::reverse_iterator relational operators

My recent changes to reverse_iterator's comparisons was not the version
of the code (or tests) that I meant to commit, and broke the relational
operators. This fixes them to reverse the order of the comparisons on
the base() iterators.

This also replaces the SFINAE constraints in the return type of the
reverse_iterator and move_iterator comparisons with a requires-clause.
This ensures the constrained overloads are preferred to unconstrained
ones. This means the non-standard same-type overloads can be omitted for
C++20 because they're not needed to solve the problem with std::rel_ops
or the testsuite's greedy_ops::X type.

	* include/bits/stl_iterator.h (reverse_iterator): Use requires-clause
	to constrain C++20 versions of comparison operators. Fix backwards
	logic of relational operators.
	(move_iterator): Use requires-clause to constrain comparison operators
	in C++20. Do not declare non-standard same-type overloads for C++20.
	* testsuite/24_iterators/move_iterator/rel_ops_c++20.cc: Check result
	of comparisons and check using greedy_ops type.
	* testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc: Likewise.
	* testsuite/24_iterators/move_iterator/greedy_ops.cc: Remove redundant
	main function from compile-only test.
	* testsuite/24_iterators/reverse_iterator/greedy_ops.cc: Likewise.

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 3b96b23ca8f164054681f9f3b089704ab1bb9843..a016f640df35499b407a0b7a658769a8a4418892 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,17 @@
+2020-03-28  Jonathan Wakely  <jwakely@redhat.com>
+
+       * include/bits/stl_iterator.h (reverse_iterator): Use requires-clause
+       to constrain C++20 versions of comparison operators. Fix backwards
+       logic of relational operators.
+       (move_iterator): Use requires-clause to constrain comparison operators
+       in C++20. Do not declare non-standard same-type overloads for C++20.
+       * testsuite/24_iterators/move_iterator/rel_ops_c++20.cc: Check result
+       of comparisons and check using greedy_ops type.
+       * testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc: Likewise.
+       * testsuite/24_iterators/move_iterator/greedy_ops.cc: Remove redundant
+       main function from compile-only test.
+       * testsuite/24_iterators/reverse_iterator/greedy_ops.cc: Likewise.
+
 2020-03-27  Jonathan Wakely  <jwakely@redhat.com>
 
        * include/bits/range_cmp.h (__cpp_lib_ranges): Define.

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index e68f66a2b8975383da9af7018316823078e8a16b..d7972b71998e3c33ce8966760936cf24d69973ac 100644 (file)
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -341,9 +341,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         { return __t.operator->(); }
     };
 
-  // Used in unevaluated expressions to test for implicit conversion to bool.
-  namespace __detail { bool __convbool(bool); }
-
   //@{
   /**
    *  @param  __x  A %reverse_iterator.
@@ -354,7 +351,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *  iterators.
    *
   */
-#if __cplusplus <= 201703L
+#if __cplusplus <= 201703L || ! defined __cpp_lib_concepts
   template<typename _Iterator>
     inline _GLIBCXX17_CONSTEXPR bool
     operator==(const reverse_iterator<_Iterator>& __x,
@@ -430,46 +427,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return !(__x < __y); }
 #else // C++20
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator==(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() == __y.base()))
+    requires requires { { __x.base() == __y.base() } -> convertible_to<bool>; }
     { return __x.base() == __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator!=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() != __y.base()))
+    requires requires { { __x.base() != __y.base() } -> convertible_to<bool>; }
     { return __x.base() != __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator<(const reverse_iterator<_IteratorL>& __x,
              const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() < __y.base()))
-    { return __x.base() < __y.base(); }
+    requires requires { { __x.base() > __y.base() } -> convertible_to<bool>; }
+    { return __x.base() > __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator>(const reverse_iterator<_IteratorL>& __x,
              const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() > __y.base()))
-    { return __x.base() > __y.base(); }
+    requires requires { { __x.base() < __y.base() } -> convertible_to<bool>; }
+    { return __x.base() < __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator<=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() <= __y.base()))
-    { return __x.base() <= __y.base(); }
+    requires requires { { __x.base() >= __y.base() } -> convertible_to<bool>; }
+    { return __x.base() >= __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
-    constexpr auto
+    constexpr bool
     operator>=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    -> decltype(__detail::__convbool(__x.base() >= __y.base()))
-    { return __x.base() >= __y.base(); }
+    requires requires { { __x.base() <= __y.base() } -> convertible_to<bool>; }
+    { return __x.base() <= __y.base(); }
+
+  template<typename _IteratorL,
+          three_way_comparable_with<_IteratorL> _IteratorR>
+    constexpr compare_three_way_result_t<_IteratorL, _IteratorR>
+    operator<=>(const reverse_iterator<_IteratorL>& __x,
+               const reverse_iterator<_IteratorR>& __y)
+    { return __y.base() <=> __x.base(); }
 #endif // C++20
   //@}
 
@@ -1413,24 +1417,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // C++20
     };
 
-  // Note: See __normal_iterator operators note from Gaby to understand
-  // why there are always 2 versions for most of the move_iterator
-  // operators.
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator==(const move_iterator<_IteratorL>& __x,
               const move_iterator<_IteratorR>& __y)
 #if __cplusplus > 201703L && __cpp_lib_concepts
-    requires requires { __detail::__convbool(__x.base() == __y.base()); }
+    requires requires { { __x.base() == __y.base() } -> convertible_to<bool>; }
 #endif
     { return __x.base() == __y.base(); }
 
-  template<typename _Iterator>
-    inline _GLIBCXX17_CONSTEXPR bool
-    operator==(const move_iterator<_Iterator>& __x,
-              const move_iterator<_Iterator>& __y)
-    { return __x.base() == __y.base(); }
-
 #if __cpp_lib_three_way_comparison
   template<typename _IteratorL,
           three_way_comparable_with<_IteratorL> _IteratorR>
@@ -1444,12 +1439,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator!=(const move_iterator<_IteratorL>& __x,
               const move_iterator<_IteratorR>& __y)
     { return !(__x == __y); }
-
-  template<typename _Iterator>
-    inline _GLIBCXX17_CONSTEXPR bool
-    operator!=(const move_iterator<_Iterator>& __x,
-              const move_iterator<_Iterator>& __y)
-    { return !(__x == __y); }
 #endif
 
   template<typename _IteratorL, typename _IteratorR>
@@ -1457,60 +1446,81 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator<(const move_iterator<_IteratorL>& __x,
              const move_iterator<_IteratorR>& __y)
 #if __cplusplus > 201703L && __cpp_lib_concepts
-    requires requires { __detail::__convbool(__x.base() < __y.base()); }
+    requires requires { { __x.base() < __y.base() } -> convertible_to<bool>; }
 #endif
     { return __x.base() < __y.base(); }
 
-  template<typename _Iterator>
-    inline _GLIBCXX17_CONSTEXPR bool
-    operator<(const move_iterator<_Iterator>& __x,
-             const move_iterator<_Iterator>& __y)
-    { return __x.base() < __y.base(); }
-
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator<=(const move_iterator<_IteratorL>& __x,
               const move_iterator<_IteratorR>& __y)
 #if __cplusplus > 201703L && __cpp_lib_concepts
-    requires requires { __detail::__convbool(__y.base() < __x.base()); }
+    requires requires { { __y.base() < __x.base() } -> convertible_to<bool>; }
 #endif
     { return !(__y < __x); }
 
-  template<typename _Iterator>
-    inline _GLIBCXX17_CONSTEXPR bool
-    operator<=(const move_iterator<_Iterator>& __x,
-              const move_iterator<_Iterator>& __y)
-    { return !(__y < __x); }
-
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator>(const move_iterator<_IteratorL>& __x,
              const move_iterator<_IteratorR>& __y)
 #if __cplusplus > 201703L && __cpp_lib_concepts
-    requires requires { __detail::__convbool(__y.base() < __x.base()); }
+    requires requires { { __y.base() < __x.base() } -> convertible_to<bool>; }
 #endif
     { return __y < __x; }
 
-  template<typename _Iterator>
-    inline _GLIBCXX17_CONSTEXPR bool
-    operator>(const move_iterator<_Iterator>& __x,
-             const move_iterator<_Iterator>& __y)
-    { return __y < __x; }
-
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator>=(const move_iterator<_IteratorL>& __x,
               const move_iterator<_IteratorR>& __y)
 #if __cplusplus > 201703L && __cpp_lib_concepts
-    requires requires { __detail::__convbool(__x.base() < __y.base()); }
+    requires requires { { __x.base() < __y.base() } -> convertible_to<bool>; }
 #endif
     { return !(__x < __y); }
 
+#if ! (__cplusplus > 201703L && __cpp_lib_concepts)
+  // Note: See __normal_iterator operators note from Gaby to understand
+  // why we have these extra overloads for some move_iterator operators.
+
+  // These extra overloads are not needed in C++20, because the ones above
+  // are constrained with a requires-clause and so overload resolution will
+  // prefer them to greedy unconstrained function templates.
+
+  template<typename _Iterator>
+    inline _GLIBCXX17_CONSTEXPR bool
+    operator==(const move_iterator<_Iterator>& __x,
+              const move_iterator<_Iterator>& __y)
+    { return __x.base() == __y.base(); }
+
+  template<typename _Iterator>
+    inline _GLIBCXX17_CONSTEXPR bool
+    operator!=(const move_iterator<_Iterator>& __x,
+              const move_iterator<_Iterator>& __y)
+    { return !(__x == __y); }
+
+  template<typename _Iterator>
+    inline _GLIBCXX17_CONSTEXPR bool
+    operator<(const move_iterator<_Iterator>& __x,
+             const move_iterator<_Iterator>& __y)
+    { return __x.base() < __y.base(); }
+
+  template<typename _Iterator>
+    inline _GLIBCXX17_CONSTEXPR bool
+    operator<=(const move_iterator<_Iterator>& __x,
+              const move_iterator<_Iterator>& __y)
+    { return !(__y < __x); }
+
+  template<typename _Iterator>
+    inline _GLIBCXX17_CONSTEXPR bool
+    operator>(const move_iterator<_Iterator>& __x,
+             const move_iterator<_Iterator>& __y)
+    { return __y < __x; }
+
   template<typename _Iterator>
     inline _GLIBCXX17_CONSTEXPR bool
     operator>=(const move_iterator<_Iterator>& __x,
               const move_iterator<_Iterator>& __y)
     { return !(__x < __y); }
+#endif // ! C++20
 
   // DR 685.
   template<typename _IteratorL, typename _IteratorR>

diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc
index 66da0a4cff972f40efd323f2f8ae484085431184..0e6d598cd43f10a5caa3d180704da1d3318aba9d 100644 (file)
--- a/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc
@@ -24,7 +24,7 @@ void test01()
   typedef std::move_iterator<greedy_ops::X*> iterator_type;
 
   iterator_type it(nullptr);
-  
+
   it == it;
   it != it;
   it < it;
@@ -35,9 +35,3 @@ void test01()
   1 + it;
   it + 1;
 }
-
-int main() 
-{ 
-  test01();
-  return 0;
-}

diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/rel_ops_c++20.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/rel_ops_c++20.cc
index 4e7b9d01e15ff2f933a0561e40c3c4d5f169ebbd..a530923f8ad3a40cbf845488853846337ebb1bca 100644 (file)
--- a/libstdc++-v3/testsuite/24_iterators/move_iterator/rel_ops_c++20.cc
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/rel_ops_c++20.cc
@@ -127,3 +127,37 @@ static_assert( has_le<2> );
 static_assert( ! has_ge<0> );
 static_assert( has_ge<1> );
 static_assert( ! has_ge<2> );
+
+int arr[3] = { 1, 2, 3 };
+constexpr std::move_iterator<int*> beg{std::begin(arr)};
+constexpr std::move_iterator<const int*> cbeg{std::cbegin(arr)};
+static_assert( beg == cbeg );
+static_assert( beg <= cbeg );
+static_assert( beg >= cbeg );
+static_assert( std::is_eq(beg <=> cbeg) );
+constexpr std::move_iterator<const int*> cend{std::cend(arr)};
+static_assert( beg != cend );
+static_assert( beg < cend );
+static_assert( cend > beg );
+static_assert( beg <= cend );
+static_assert( cend >= beg );
+static_assert( std::is_lt(beg <=> cend) );
+
+#include <testsuite_greedy_ops.h>
+
+void test01()
+{
+  typedef std::move_iterator<greedy_ops::X*> iterator_type;
+
+  iterator_type it(nullptr);
+
+  it == it;
+  it != it;
+  it < it;
+  it <= it;
+  it > it;
+  it >= it;
+  // it - it;  // See PR libstdc++/71771
+  1 + it;
+  it + 1;
+}

diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/greedy_ops.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/greedy_ops.cc
index 60d4611e690b05b0bcb8a189c99330d0d9a72970..a5ea452460452a47adf8ceed02a85720c2c8a02f 100644 (file)
--- a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/greedy_ops.cc
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/greedy_ops.cc
@@ -24,7 +24,7 @@ void test01()
   typedef std::reverse_iterator<greedy_ops::X*> iterator_type;
 
   iterator_type it;
-  
+
   it == it;
   it != it;
   it < it;
@@ -37,9 +37,3 @@ void test01()
   1 + it;
   it + 1;
 }
-
-int main() 
-{ 
-  test01();
-  return 0;
-}

diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc
index 3e91a0396fe09346be332b1a7cc4b0860d43b82d..a382ae524832decebf3aecfbd5235fa997120bc1 100644 (file)
--- a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops_c++20.cc
@@ -78,10 +78,10 @@ reverse_iterator<long*> r{nullptr};
 
 bool b0 = l0 == r;
 bool b1 = l1 != r;
-bool b2 = l2 < r;
-bool b3 = l3 > r;
-bool b4 = l4 <= r;
-bool b5 = l5 >= r;
+bool b2 = l2 > r;
+bool b3 = l3 < r;
+bool b4 = l4 >= r;
+bool b5 = l5 <= r;
 
 template<int N>
   concept has_eq
@@ -129,15 +129,15 @@ static_assert( ! has_ne<5> );
 
 static_assert( ! has_lt<0> );
 static_assert( ! has_lt<1> );
-static_assert( has_lt<2> );
-static_assert( ! has_lt<3> );
+static_assert( ! has_lt<2> );
+static_assert( has_lt<3> );
 static_assert( ! has_lt<4> );
 static_assert( ! has_lt<5> );
 
 static_assert( ! has_gt<0> );
 static_assert( ! has_gt<1> );
-static_assert( ! has_gt<2> );
-static_assert( has_gt<3> );
+static_assert( has_gt<2> );
+static_assert( ! has_gt<3> );
 static_assert( ! has_gt<4> );
 static_assert( ! has_gt<5> );
 
@@ -145,12 +145,49 @@ static_assert( ! has_le<0> );
 static_assert( ! has_le<1> );
 static_assert( ! has_le<2> );
 static_assert( ! has_le<3> );
-static_assert( has_le<4> );
-static_assert( ! has_le<5> );
+static_assert( ! has_le<4> );
+static_assert( has_le<5> );
 
 static_assert( ! has_ge<0> );
 static_assert( ! has_ge<1> );
 static_assert( ! has_ge<2> );
 static_assert( ! has_ge<3> );
-static_assert( ! has_ge<4> );
-static_assert( has_ge<5> );
+static_assert( has_ge<4> );
+static_assert( ! has_ge<5> );
+
+int arr[3] = { 1, 2, 3 };
+constexpr std::reverse_iterator<int*> rbeg = std::rbegin(arr);
+constexpr std::reverse_iterator<const int*> crbeg = std::crbegin(arr);
+static_assert( rbeg == crbeg );
+static_assert( rbeg <= crbeg );
+static_assert( rbeg >= crbeg );
+static_assert( std::is_eq(rbeg <=> crbeg) );
+constexpr std::reverse_iterator<const int*> crend = std::crend(arr);
+static_assert( rbeg != crend );
+static_assert( rbeg < crend );
+static_assert( crend > rbeg );
+static_assert( rbeg <= crend );
+static_assert( crend >= rbeg );
+static_assert( std::is_lt(rbeg <=> crend) );
+
+#include <testsuite_greedy_ops.h>
+
+// copied from 24_iterators/reverse_iterator/greedy_ops.cc
+void test01()
+{
+  typedef std::reverse_iterator<greedy_ops::X*> iterator_type;
+
+  iterator_type it;
+
+  it == it;
+  it != it;
+  it < it;
+  it <= it;
+  it > it;
+  it >= it;
+#if __cplusplus < 201103L
+  it - it; // See PR libstdc++/71771
+#endif
+  1 + it;
+  it + 1;
+}