[committed] libstdc++: Fix std::reverse_iterator comparisons (PR 94354)

Message ID 20200527210847.GA3030029@redhat.com
State New
Headers show
Series
  • [committed] libstdc++: Fix std::reverse_iterator comparisons (PR 94354)
Related show

Commit Message

Kees Cook via Gcc-patches May 27, 2020, 9:08 p.m.
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.

Tested powerpc64le-linux, committed to master.
commit 979e89a9a94f66241fa8355e2b2e8f4a680c83e1
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 27 21:58:56 2020 +0100

    libstdc++: Fix std::reverse_iterator comparisons (PR 94354)
    
    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.

Patch

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 19b1d53f781..b0f45499aec 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -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
index 00000000000..4f2675f471b
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc
@@ -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