libstdc++: Fix std::reverse_iterator comparisons (PR 94354)
authorJonathan Wakely <jwakely@redhat.com>
Wed, 27 May 2020 20:58:56 +0000 (21:58 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Wed, 27 May 2020 20:58:56 +0000 (21:58 +0100)
The std::reverse_iterator comparisons have always been implemented only
in terms of equality and less than. In C++98 that made no difference for
reasonable code, because when the underlying operators are the same type
they are required to support all comparisons anyway.

But since LWG 280 it's possible to compare reverse_iterator<X> and
reverse_iterator<Y>, and comparisons between X and Y might not support
the full set of equality and relational operators. This means that it
matters whether we implement operator!= as x.base() != y.base() or
!(x.base() == y.base()), and the current implementation is
non-conforming.

This was already fixed in GCC 10.1 for C++20, this change also fixes it
for all other -std modes.

PR libstdc++/94354
* include/bits/stl_iterator.h (reverse_iterator): Fix comparison
operators to use the correct operations on the underlying
iterators.
* testsuite/24_iterators/reverse_iterator/rel_ops.cc: New test.

libstdc++-v3/include/bits/stl_iterator.h
libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc [new file with mode: 0644]

index 19b1d53..b0f4549 100644 (file)
@@ -393,6 +393,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // DR 280. Comparison of reverse_iterator to const reverse_iterator.
+
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator==(const reverse_iterator<_IteratorL>& __x,
@@ -403,31 +404,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline _GLIBCXX17_CONSTEXPR bool
     operator<(const reverse_iterator<_IteratorL>& __x,
              const reverse_iterator<_IteratorR>& __y)
-    { return __y.base() < __x.base(); }
+    { return __x.base() > __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator!=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    { return !(__x == __y); }
+    { return __x.base() != __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator>(const reverse_iterator<_IteratorL>& __x,
              const reverse_iterator<_IteratorR>& __y)
-    { return __y < __x; }
+    { return __x.base() < __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator<=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    { return !(__y < __x); }
+    { return __x.base() >= __y.base(); }
 
   template<typename _IteratorL, typename _IteratorR>
     inline _GLIBCXX17_CONSTEXPR bool
     operator>=(const reverse_iterator<_IteratorL>& __x,
               const reverse_iterator<_IteratorR>& __y)
-    { return !(__x < __y); }
+    { return __x.base() <= __y.base(); }
 #else // C++20
   template<typename _IteratorL, typename _IteratorR>
     constexpr bool
diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc
new file mode 100644 (file)
index 0000000..4f2675f
--- /dev/null
@@ -0,0 +1,99 @@
+// Copyright (C) 2020 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 later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile }
+
+#include <iterator>
+
+template<int>
+struct Iter
+{
+  typedef std::random_access_iterator_tag iterator_category;
+  typedef int value_type;
+  typedef int* pointer;
+  typedef int& reference;
+  typedef std::ptrdiff_t difference_type;
+
+  Iter();
+
+  Iter& operator++();
+  Iter operator++(int);
+  Iter& operator--();
+  Iter operator--(int);
+  int& operator*() const;
+  int* operator->() const;
+
+  int& operator[](difference_type) const;
+
+  Iter& operator+=(difference_type);
+  Iter& operator-=(difference_type);
+
+  template<int N> friend Iter operator+(Iter<N>, difference_type);
+  template<int N> friend Iter operator+(difference_type, Iter<N>);
+  template<int N> friend Iter operator-(Iter<N>, difference_type);
+  template<int N> friend difference_type operator-(Iter<N>, Iter<N>);
+
+  // Define the full set of operators for same-type comparisons
+  template<int N> friend bool operator==(Iter<N>, Iter<N>); // synthesizes !=
+  template<int N> friend bool operator<(Iter<N>, Iter<N>);
+  template<int N> friend bool operator>(Iter<N>, Iter<N>);
+  template<int N> friend bool operator<=(Iter<N>, Iter<N>);
+  template<int N> friend bool operator>=(Iter<N>, Iter<N>);
+};
+
+// Define a single kind of mixed-type comparison for each specialization.
+int   operator==(Iter<0>, long*);
+void* operator!=(Iter<1>, long*);
+bool& operator< (Iter<2>, long*);
+int   operator> (Iter<3>, long*);
+void* operator<=(Iter<4>, long*);
+bool& operator>=(Iter<5>, long*);
+
+using std::reverse_iterator;
+
+reverse_iterator< Iter<0> > l0;
+reverse_iterator< Iter<1> > l1;
+reverse_iterator< Iter<2> > l2;
+reverse_iterator< Iter<3> > l3;
+reverse_iterator< Iter<4> > l4;
+reverse_iterator< Iter<5> > l5;
+reverse_iterator<long*> r;
+
+// PR libstdc++/94354
+// Check that these operators use the correct operator on the
+// underlying iterator types.
+bool b0 = l0 == r;
+bool b1 = l1 != r;
+bool b2 = l2 > r;
+bool b3 = l3 < r;
+bool b4 = l4 >= r;
+bool b5 = l5 <= r;
+
+#if __cplusplus >= 201703L
+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 );
+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 );
+#endif // C++17